genqlient icon indicating copy to clipboard operation
genqlient copied to clipboard

Option to use pointers for optional fields

Open connec opened this issue 2 years ago • 2 comments

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:

type User {
    id: ID!
    name: String!
    avatarUrl: String
}

gqlgen will generate a server-side model like:

type User struct {
	AvatarURL *string `json:"avatarUrl"`
	ID string `json:"id"`
	Name string `json:"name"`
}

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:

type userGetByIDUser struct {
  AvatarUrl string `json:"avatarUrl"`
  Id string `json:"id"`
  Name string `json:"name"`
}

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, present string 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 like genqlient to generate the following, without additional annotations:

type userGetByIDUser struct {
  AvatarUrl *string `json:"avatarUrl"`
  Id string `json:"id"`
  Name string `json:"name"`
}

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.

connec avatar Mar 17 '22 16:03 connec

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 genqlient.yaml option to say to automatically use pointers is a fine way.

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 myutils.Optional[T] for optional types" in addition to this and #155. But honestly I'm having trouble seeing far enough into the future to know how to do that, so let's just do the best we can and we can always add synonyms later. Sketching a few options:

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.

benjaminjkraft avatar Mar 23 '22 18:03 benjaminjkraft

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 optional, optional_fields, optional_types, ...

connec avatar Mar 24 '22 12:03 connec

Fixed in #198, forgot to close this earlier!

benjaminjkraft avatar Aug 20 '22 18:08 benjaminjkraft