-
Notifications
You must be signed in to change notification settings - Fork 117
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
Option to use pointers for optional fields #178
Comments
Thanks for raising this. I think now that we're well beyond v0.0.1 it's time for me to admit that using pointers for optionality the best thing for some people, even if I personally dislike it :-) . I think a Do you want this on input types, or just output? I can see reasons either way (consistency, vs. the fact that you can instead use omitempty there). Possibly eventually we'll have two options, but I think it's fine to just implement whichever version you want for now. Speaking of which, ideally we'd want to avoid a proliferation of config options, especially if in the future we want to add things like "use omitempty automatically for (all/struct/optional) input types" or "use pointers_for_optional: true
pointers_for_optional: output_only # or, eventually: input_only, always, never, ...
pointers: output_optional # or, eventually: structs, structs_or_optional, always, never, ...
pointers:
output: optional
# input: never
optional:
output: pointer # or, eventually: Optional[T], presence_field, ...
# input: omitempty I think I like the first or last of those the most; the first is at least simple and the last seems like it might be extensible? Anyway, we can discuss this now or later, since it Otherwise, I think this will mostly just work the obvious way, so I don't think there's too much more to discuss. I'm happy for you to have a go at implementing it! Let me know if you run into any trouble implementing; I'm guessing it will be fairly straightforward but I haven't thought it through very far. |
Awesome! I'll find some time to look at this in the near future. Of the options, I quite like the last one. As you say, it leaves it open for setting specific types or for particular sentinel values. If I go that route, I'd be thinking of something like: optionals:
output: pointer # or "value" for the current behaviour Also toying with |
This implements the approach suggested in #178 (comment). See the added documentation for the full behavior.
Fixed in #198, forgot to close this earlier! |
Is your feature request related to a problem? Please describe.
I'm working on a Go service that needs to speak to a number of 'backend' GraphQL APIs. I'm trying to get to a setup where both sides of the communication use generated types, in order to bring integration issues to compile-time.
I'm using
gqlgen
for the server, which defaults to using pointers to describe optional fields. For example, given a GraphQL type such as:gqlgen
will generate a server-side model like:E.g., the
AvatarURL
field is a*string
since it is optional in the schema.However, when I use
genqlient
I get a model like:I'm sure reasonable people could disagree about how optional fields should be encoded in Go, but I'm personally a fan of pointers for this purpose, at least as a default (I'd rather assume
""
can be a valid, presentstring
than treat it as<none>
by default). In this case, it's particularly jarring since the server-side models (worked on by the same engineers) use a different paradigm.From the looks of things, annotating optional fields explicitly with
pointer: true
would generate the models I desired, but for me this undermines the advantages of using code generation to ensure consistency between the client and server – it's something that can be forgotten.Describe the solution you'd like
I'd like a configuration for
genqlient.yaml
that will lead to using pointers for optional fields in generated types. E.g. I'd likegenqlient
to generate the following, without additional annotations:After starting to write this I found the note on Optionality and pointers in
DESIGN.md
, and while it's a fine take, it would be really useful to be able to configure this 'globally' when working with different design decisions.Describe alternatives you've considered
Annotating all optional fields is an obvious "functional" solution, but it undermines some of the goals I have for using code generation.
I'm also considering forking, to change the default for our use, but it would be far preferable to have upstream support. I may be able to work on a PR, if one would be considered.
The text was updated successfully, but these errors were encountered: