Skip to content
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

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

Open
lennyburdette opened this issue Oct 28, 2022 · 1 comment
Labels
core Related to codegen core/cli

Comments

@lennyburdette
Copy link

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
Copy link

glasser commented Oct 28, 2022

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:

  • encapsulates the as any to be something you only have to write once
  • keeps the runtime behavior that a new implementation gets the console.log (or whatever you want it to be)
  • at build time, if a new implementation is added, then next time you run codegen it will fail to build (because pet is not actually never), which I think is what you want? (If it’s not what you want then just declare x: any instead and drop the as any)

@charlypoly charlypoly added the core Related to codegen core/cli label Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to codegen core/cli
Projects
None yet
Development

No branches or pull requests

3 participants