dotansimha / graphql-code-generator

A tool for generating code based on a GraphQL schema and GraphQL operations (query/mutation/subscription), with flexible support for custom plugins.
https://the-guild.dev/graphql/codegen/
MIT License
10.79k stars 1.31k forks source link

Future-proofed union types for interfaces and unions in typescript-operations #8538

Open lennyburdette opened 1 year ago

lennyburdette commented 1 year ago

Is your feature request related to a problem? Please describe.

Adding implements InterfaceName to a type is not a breaking change (according to GraphQL Inspector, and I think community consensus) but it does mean that client behavior could change. I usually recommend that clients program defensively with a default: branch it their switch statements, like this:

query {
  pet {
    name
    ... on Cat {
      meow
    }
    ... on Dog {
      bark
    }
  }
}
switch (pet.__typename) {
  case 'Cat':
    console.log(`Cat meows ${pet.meow}`);
    break;
  case 'Dog':
    console.log(`Dog barks ${pet.bark}`);
    break;
  default:
    console.log(`Unknown animal ${pet.__typename}`);
}

So that when type Fish implements Pet is added to the schema, the UI has some way of handling it.

However, the typescript-operations plugin generates types like this:

export type PetQuery = {
  __typename: "Query";
  pet: 
    | { __typename: 'Cat'; meow?: string | null; name?: string | null; }
    | { __typename: 'Dog'; bark?: string | null; name?: string | null; }
};

Which results in a TypeScript error:

default:
  console.log(`Unknown pet type ${pet.__typename}`);
//                                    ^^^^^^^^^^
// Property '__typename' does not exist on type 'never'

Describe the solution you'd like

I'd like an option to the typescript-operations plugin that adds an additional type to the union type like this:

export type PetQuery = {
  __typename: "Query";
  pet: 
    | { __typename: 'Cat'; meow?: string | null; name?: string | null; }
    | { __typename: 'Dog'; bark?: string | null; name?: string | null; }
    | { __typename: string, name?: string | null }
};

The option would be disabled by default so that users could opt into this change.

Describe alternatives you've considered

You can always add // @ts-ignore in the default: clause, or cast it to any (thought this might trigger other linters):

console.log(`Unknown pet type ${(pet as any).__typename}`);

Additional context

Ideally we could enforce adding the default: class with the switch-exhaustiveness-check eslint rule, but it only works on union types. I played around with something like

enum PetQuery_PetTypename {
  Cat = "Cat",
  Dog = "Dog",
  __unknown = ""
}

export type PetQuery = {
  __typename: "Query";
  pet: 
    | { __typename: PetQuery_PetTypename.Cat; meow?: string | null; name?: string | null; }
    | { __typename: PetQuery_PetTypename.Dog; bark?: string | null; name?: string | null; }
    | { __typename: PetQuery_PetTypename.__unknown, name?: string | null }
};

which did cause the exhaustiveness check to occur, but it feels hacky. But I bet someone with more TypeScript knowledge knows how to do this!

glasser commented 1 year ago

One approach:

function unknownAbstract(typeName: string, x: never) {
    console.log(`Unknown ${typeName} ${(x as any).__typename}`);
}

function switcheroo(q: PetQuery) {
    const { pet } = q;
    switch (pet.__typename) {
        case 'Cat':
            console.log(`Cat meows ${pet.meow}`);
            break;
        case 'Dog':
            console.log(`Dog barks ${pet.bark}`);
            break;
        default:
            unknownAbstract('Animal', pet);
    }
}

What this does is: