-
Notifications
You must be signed in to change notification settings - Fork 114
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
typename
doesn't work on basic types.
#130
Comments
I think the real issue is that directives apply not only to a node but also its children. So if you have:
then it tries to apply the I don't understand how this doesn't cause problems elsewhere, like if you had:
wouldn't it cause the type for |
OK, I think it is a bug in genqlient. WIll file separately. |
I think this can't be implemented until #131 is resolved. But once it is, this diff might be sufficient:
(plus changing the tests to test for it: I changed it liks this:
) |
This lets you write code like: ``` query x { # @genqlient(typename: "MyString") someStringField } ``` and genqlient will do ``` typename MyString string type x struct { someStringField MyString } ``` This was not difficult to implement, though it required introducing a new identifier type. The main difficulty I had was weird test failures, that it turns out was due to the tests putting a bunch of fields on the same line, so that the genqlient directive on the previous line applied to all of them, accidentally. This became a problem when `typename` suddenly started being respected for builtin types! I fixed it by just spreading out the queries a bit. Fixes #130
This lets you write code like: ``` query x { # @genqlient(typename: "MyString") someStringField } ``` and genqlient will do ``` typename MyString string type x struct { someStringField MyString } ``` This was not difficult to implement, though it required introducing a new identifier type. The main difficulty I had was weird test failures, that it turns out was due to the tests putting a bunch of fields on the same line, so that the genqlient directive on the previous line applied to all of them, accidentally. This became a problem when `typename` suddenly started being respected for builtin types! I fixed it by just spreading out the queries a bit. Fixes #130
## Summary: This lets you write code like: ``` query x { # @genqlient(typename: "MyString") someStringField } ``` and genqlient will do ``` typename MyString string type x struct { someStringField MyString } ``` This was not difficult to implement, though it required introducing a new identifier type. The main difficulty I had was weird test failures, that it turns out was due to the tests putting a bunch of fields on the same line, so that the genqlient directive on the previous line applied to all of them, accidentally. This became a problem when `typename` suddenly started being respected for builtin types! I fixed it by just spreading out the queries a bit. Fixes #130 ## Test plan: make check Author: csilvers Reviewers: dnerdy, StevenACoffman, benjaminjkraft Required Reviewers: Approved By: StevenACoffman Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint Pull Request URL: #133
It seems like the
typename
directive only works on structs. But it would be great if it worked on basic types likestring
, so you could do:where
renderType
is a string in the graphql schema, and the generated code would be:The text was updated successfully, but these errors were encountered: