overblog / GraphQLBundle

This bundle provides tools to build a complete GraphQL API server in your Symfony App.
MIT License
780 stars 223 forks source link

Merge guessed argument properties and args described with GQL\Arg #1151

Closed mathroc closed 4 months ago

mathroc commented 6 months ago
Q A
Bug fix? yes/no
New feature? yes/no
BC breaks? yes/no
Deprecations? no
Tests pass? yes/no
Documented? no
Fixed tickets #...
License MIT

When a single argument is described with #[GQL\Arg] all other are not guessed anymore

This commit starts by guessing all arguments and override the ones that have #[GQL\Arg] attributes describing them

I'm not sure of a few things:

in the future it would be nice if we could add the attribute on the argument directly instead of the method so we could make the name & type optional there (or forbidden?)

Vincz commented 6 months ago

@mathroc to answer your questions:

You are right, we could also explore the possibility to set attributes directly on the arguments.

mathroc commented 6 months ago

and for the last question, I can answer it by looking at the tests: yes sometimes the guessing can't work at all, e.g.: arrays

:thinking:

Vincz commented 6 months ago

Yes, and we want an error when we don't have a corresponding #[GQL\Arg] and the auto-guess failed.
To make this work, you first need to get arguments having a corresponding#[GQL\Arg] and auto-guess only the remaining ones (by passing the list of already resolved to the auto-guesser for example).

mathroc commented 6 months ago

How should the arguments be ordered? if it has any importance)

Vincz commented 6 months ago

@mathroc The GraphQL arguments order doesn't matter as they are named. But we need them in the right order when the PHP method is called.

mathroc commented 6 months ago

ok, I think the PR is fine then, let me know if there's anything to do. (should I squash the commits?)