Skip to content
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

Bizarre Cannot query field "settings" on type "...". error #652

Open
josefsabl opened this issue Feb 1, 2024 · 3 comments
Open

Bizarre Cannot query field "settings" on type "...". error #652

josefsabl opened this issue Feb 1, 2024 · 3 comments

Comments

@josefsabl
Copy link

josefsabl commented Feb 1, 2024

Turns out you cannot query for field starting with set when function is declared like this:

#[Graph\Field]
public function settings(): string

You get Cannot query field "settings" on type "...". error.

Same applies for fields starting with get. function getawayId() ends up in field field awayId.

Obviously there is flaw in how the function names are parsed to match the query fields. Maybe the camel case could be taken into consideration

@oojacoboo
Copy link
Collaborator

@josefsabl thanks for reporting. GraphQLite does attempt to parse getters and setters. Overall, I think it does a pretty good job here, but there are exceptions and it's not always transparent. That's an unfortunate aspect of the design. That said, you can specify the field name argument on the Field attribute to override this.

See docs:
https://graphqlite.thecodingmachine.io/docs/annotations-reference#field

@josefsabl
Copy link
Author

I understand and also understand fixing it would introduce a BC break.

Please note I spent good amount of time trying to figure out what is going on (Double, triple, quadruple checked there is no typo. Double, triple, quadruple checked I am using correct class. Checked whether 'settings'[ is not some sort of reserved word in GraphQL. Invited a colleague to see it and exclude possibility I am crazy. Do a hopeless xdebug code tracing. Then it struck me.) The point is this may happen to more people.

Maybe in some major update I would think about introducing a deprecation for 'setsomething' while 'setSomething' would still work. Maybe add option for the 'setsomething' to continue work.

Also there are not many words except "settings" that make sense in this context and 'tings' is not a word so maybe just adding "settings" on a blacklist would do the trick. But it is little stupid, I guess :-)

Workaround I used is to rename the function from settings() to getSettings().

Anyway, thanks for great library! Have a nice day.

@oojacoboo
Copy link
Collaborator

@josefsabl if you'd like to submit a PR on this, we can put it into the next release. We're targeting 7.0 for the next release, as there is a lot in it. I agree that matching for case would be ideal here and should be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants