Supporting backward compatible schemas

[GFriedrich]

Is your feature request related to a problem? Please describe.
When making changes over time to a GraphQL schema, one of them might be adding a new type to an interface.
According to the GraphQL spec this is not considered a breaking change (see https://spec.graphql.org/October2021/#sec-Validation.Type-system-evolution):

Any change that can cause a previously valid request to become invalid is considered a breaking change.

As only a new type was added, all previous queries will still work.
graphql-js in turn "only" registers this as a "potentially dangerous change" (see https://github.com/graphql/graphql-js/blob/bb87e73067865207e19924297cabff45686f2a76/src/utilities/findBreakingChanges.ts#L62).

This all brings me to the conclusion that adding a new type to an existing interface should work also for genqlient.

Describe the solution you'd like
It would be great to allow the deserialization of responses using unknown types.
That could be achieved by using a default struct that just implements the common values of the interface and that is used as a fallback in case the type is unknown.
Optionally this behavior could be enabled/disabled depending on the use-case of the implementation.

Describe alternatives you've considered
The only other option would be to force people updating their applications which may become impossible/very cumbersome if there are a large number of service affected.

Additional context
None

[benjaminjkraft]

This should work -- genqlient allows unknown keys in the JSON and upon regeneration Go similarly considers adding a struct field to be a non-breaking change. If you're running into trouble, could you be more specific about what you expect to work, and what error you're seeing when the server adds a field?

[GFriedrich]

Hey @benjaminjkraft,
thanks for giving this a check.

I'm not talking about a situation where a new field is added (this would anyways not be returned when not queried by the client).
What I'm talking about is if you add a new type to an interface.
Imagine the following setup:

type ParentType {
    myObjects: [MyInterface!]!
}

interface MyInterface {
    id: ID!
}

type TypeA implements MyInterface {
    id: ID!
    someValue: Int!
}

Now a new type gets added to the interface like:

type TypeB implements MyInterface {
    id: ID!
    someOther: Int!
}

As the new type is part of the list it will be returned even on existing queries, but only with the shared fields.
Due to the fact that the implementation checks the __typename it will run into the error of receiving an unknown type and fails on deserialization - see here:

genqlient/generate/unmarshal_helper.go.tmpl

Line 34 in 6309a6e

`unexpected concrete type for {{.GoName}}: "%v"`, tn.TypeName)

Or do you think differently?

[benjaminjkraft]

Ah, thanks for explaining, I misread. I agree this doesn't work today, and should. I'm surprised it hasn't come up before!

What I'm less sure of is how it should work. We can't just throw the data in a map, because it needs to implement the interface. I guess what you're suggesting is we could, for each interface, generate a type MyInterfaceUnknown struct ..., with the fields of the interface (equivalent-ish to what we do for struct: true), plus the usual methods, and then destructure into that? (And I guess this probably already works right for struct: true fields.) That seems plausible, although I'm not totally sure if we'd run into trouble somewhere along the way!

Not totally sure if this should be behind a flag. Probably the new behavior is the better default, but maybe we need to keep the old behavior available, in case. It's hard for me to see exactly why anyone might care, but it's probably not hard to keep a flag in. (If they don't want to handle unknown they can always return their own error when they get to it, but maybe they want to know they can handle it before they start doing so?)

Possibly we ought to do the same thing for unions, although in that case perhaps it's just an empty struct (or a typename field, I guess). (And I don't think any action is needed for enums, since the Go doesn't validate that they're an "expected" value.)

[GFriedrich]

@benjaminjkraft: Yes, exactly. Everything you wrote was my thinking as well.