-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
InputTypeGenerator::mapFactoryMethod doesn't call field middleware. #439
Comments
Would you accept a PR to fix this? I may have some time next weekend. |
@withinboredom Can you please describe this issue in more detail. What field middleware are you referring to, and in your example, what is it you're expecting to happen? |
This field middleware: https://graphqlite.thecodingmachine.io/docs/field-middlewares I'd expect the input type's fields to recognize my custom attribute and apply whatever customization to the field. |
@withinboredom You're going to have to do a better job in explaining if you'd like some input on this. I still don't know what you're trying to do really. |
For example, I have the following attribute: <?php
namespace Attributes;
use Attribute;
use TheCodingMachine\GraphQLite\Annotations\MiddlewareAnnotationInterface;
#[Attribute(Attribute::TARGET_FUNCTION |
Attribute::TARGET_CLASS |
Attribute::TARGET_PARAMETER |
Attribute::TARGET_METHOD)]
class Description implements MiddlewareAnnotationInterface
{
public function __construct(public readonly string $description)
{
}
} and the following middleware: <?php
namespace Middlewares;
use GraphQL\Type\Definition\FieldDefinition;
use Attributes\Description;
use TheCodingMachine\GraphQLite\Middlewares\FieldHandlerInterface;
use TheCodingMachine\GraphQLite\Middlewares\FieldMiddlewareInterface;
use TheCodingMachine\GraphQLite\QueryFieldDescriptor;
class DescriptionMiddleware implements FieldMiddlewareInterface
{
public function process(
QueryFieldDescriptor $queryFieldDescriptor,
FieldHandlerInterface $fieldHandler
): ?FieldDefinition {
$annotations = $queryFieldDescriptor->getMiddlewareAnnotations();
/**
* @var Description|null $description
*/
$description = $annotations->getAnnotationByType(Description::class);
if (null === $description) {
return $fieldHandler->handle($queryFieldDescriptor);
}
$queryFieldDescriptor->setComment($description->description);
return $fieldHandler->handle($queryFieldDescriptor);
}
} Fields allow you to do something like:
But I also wanted them on things like controllers: #[Query]
#[Logged]
#[FailWith(value: null)]
#[Description('This is everything about you.')]
public function me(#[InjectUser] User $user): User
{
return $user;
} or on the fields in a mutation #[Mutation]
#[Logged]
#[Right(Rights::SubscriptionValue)]
#[Description('Delete a contact')]
public function deleteContact(
#[Description('The contact to delete')]
#[UseInputType(ContactRepository::TYPE_SELECTOR_REQ)] Contact $contact
): Contact {} However, in factories, it doesn't work because the field middleware doesn't appear to be called at all for the generated fields: #[Factory(name: self::TYPE_INPUT, default: false)]
public function createContact(
#[Description('a contacts name')]
string $name
): Contact {} |
Ah, thanks for the explanation. This is indeed poorly implemented at the moment. And not just in this scenario. There are multiple scenarios where the description isn't consistent across the API. For example, the PHPDoc is used if a A more comprehensive approach to this needs to be taken. I already have concerns about there being too many attributes and things not being as cohesive as they should. As for a PR, absolutely. This would be great. If you'd like to review the API and put forward some further suggestions on this that'd be a good place to start. If not, I may have some time over this weekend to review this. Personally, I think relying on the PHPDoc, while nice, is a bad approach that could expose internals. It also fragments the API/use of this lib and makes it confusing. IMO, we should just have a |
@withinboredom have a look at this PR #466 this would be possible to implement when afterwards. |
When generating fields from a factory method, the field middleware appears to never be called for the generated fields. I expected to be able to do something like:
For now, the workaround appears to be to manually specify an input type instead of a factory?
The text was updated successfully, but these errors were encountered: