gqlgen: 0.17.14 generate getters for interfaces problem

What happened?

When upgrading to 0.17.14 my code generation started to fail due to the new generation of getters for interfaces. It’s due to interfaces returning other interfaces I think.

What did you expect?

Code generation to result in valid code

Minimal graphql.schema and models to reproduce

Reproduction repo: https://github.com/argoyle/gqlgen-implements

versions

  • go run github.com/99designs/gqlgen version? 0.17.14
  • go version? 1.19

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 1
  • Comments: 24 (16 by maintainers)

Most upvoted comments

This is an interesting one. Looking at the schema (https://github.com/argoyle/gqlgen-implements/blob/main/graph/schema.graphqls), we have UsersConnection implements Connection, but… it doesn’t look like it actually does. The edges field on UsersConnection is [UsersEdge!]!, but the edges field in the Connection interface is [Edge!]!.

Even though UsersEdge implements Edge is true, I’m pretty sure [UsersEdge!]! implements [Edge!]! isn’t, i.e., even if every element in edges is a UsersEdge, I’m fairly positive the type on edges still needs to be [Edge!]!.

I can see if the GraphQL spec calls this out.

EDIT: It does https://spec.graphql.org/October2021/#sec-Objects. What you have here @argoyle is valid GraphQL. I can modify the getter generation logic for this scenario.

One last note: this should have been more prominent in the release notes because it is a breaking change. Also probably should have justified a minor version bump.

Thanks! I cut an interim release v0.17.15 that includes the latest refinement, and by the time you get a config option landed I can then cut a v0.17.16. I’m going to leave this issue open until then.

I asked some co-workers the same question and they added these:

  • GraphQL only has Int (=int32) and Float (=float64), notably there is officially no int64/uint64
  • Fragments aren’t a part of GraphQL’s type system per se, but are sort of related and don’t have any equivalent in Go; and the fragment embedding rules (legal any time the two types share an implementation) are a bit surprising and different from the Go embedding rules (the embedder necessarily implements the embeddee)
  • GraphQL types aren’t really just one type, because in different contexts you have different fields of that type. So while both GraphQL schemas and Go are both nominally typed, GraphQL response types kinda feel a bit less nominally typed in that there are many kinds of “User” depending on which fields you asked for. This is especially relevant when it comes to idiomatic usage of the types; in code using GraphQL it’s idiomatic to use row polymorphism (a.k.a. inexact types, e.g. using a function of {id: Int} on a value of type {id: Int, <other fields>}) such that you can add some fields to a query and existing code doesn’t care. But Go has no way to do row polymorphism (at least not without introducing interfaces everywhere), and so you have to do things a bit differently especially if you have multiple overlapping queries. (Ed.: this isn’t a good explanation, if it doesn’t make sense let me know and I can take another stab where I actually properly explain my terms.) One way to explain the difference in interfaces is that Go interfaces are effectively structurally typed (there are subtle cases where this isn’t true, but mostly) whereas GraphQL interfaces are nominally typed, and so we emulate nominally-typed interfaces via sentinel methods (this is probably only a useful explanation for people who already know what those words mean though).

Do you mind listing some of the things you stumbled on?

Not at all:

  • Unions exist in GraphQL, but not Go, so they are somewhat mocked through Go interfaces
    • The Is{UnionName}() signpost functions leverage the Go compiler to help alleviate some of the confusion here
    • Without the signposts, a programmer implementing a query resolver would need likely need to jump to the GraphQL schema to figure out what types are included in the union
  • Interfaces in GraphQL differ from Go interfaces in two key ways
    • A GraphQL interface definition includes only fields, where Go interfaces contain methods
      • Getter functions are generated to preserve the fields defined in the GraphQL interface
    • GraphQL types must explicitly declare that they’re implementing an interface to be used as that interface type, where Go does this implicitly, i.e. if a type has the same methods that an interface defines, it can be used as that interface type
      • The signpost functions prevent Go types from inadvertently implementing GraphQL interfaces
        • As an example, imagine two different GraphQL interfaces that only contain ID: ID!
  • GraphQL makes you use special input types if you need structured input to a query or mutation
    • input types can’t be used as returns, solely as inputs
    • Go’s type system has no analog, and no great way to enforce this
  • GraphQL types have the notion of “required” fields, e.g. Int!, where Go only has pointers
    • For primitives, it is somewhat convenient to just use the type directly for required fields, and pointers for optional fields
    • For structures, this becomes a hard decision, since using a structure type directly in Go will force copies to be created
      • The compromise here is that structure fields are always generated as pointer in Go, and, if they’re marked as required in the GraphQL schema, the resolver has to enforce that the field in the Go structure is non-nil at runtime

That’s what I have off the top of my head. I haven’t done anything too terribly exotic with gqlgen though (other than filling in schema.resolvers.go, it’s just using whatever gqlgen generates), so I’m sure others have stumbled on bigger footguns.

@MichaelMure I re-opened this issue because it looks like there may be some more rough edges to iron out.

If we can’t easily make the new generating getters for interfaces transparently painless for existing users, I’d like to default it to off and make a config option to allow people to opt-in to this new behavior while we continue to refine it. What do you think of that @neptoess ?