TypeScript output shouldn't treat interface as a union of implementing types

[twavv]

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

The current treatment of interfaces treats an interface as a union of its types. This means it's actually not safe for backwards compatible usage (i.e., the server should be able to add new interface implementations in a backwards compatible way, but graphql-code-generator breaks that contract). It's not the end of the world, but I didn't see anything else about this.

Describe the solution you'd like

It's not possible to do something like

interface IAnimal { __typename: string; name: string; }
interface Dog extends IAnimal { __typename: "Dog"; favoriteDogTreatType: string; }
interface Cat extends IAnimal { __typename: "Cat"; meowVolume: number; }

since the resulting codegen would be something like this:

type Animal = IAnimal | Dog | Cat;

and this would be broken:

const animal: IAnimal = ...;
if (animal.__typename === "Dog") {
  // This is an error since this type guard only narrows to (IAnimal | Dog)
  // since it's technically valid for IAnimal to have __typename "Dog" in
  // TypeScript's world -- even though that's not possible in GraphQL world
  console.log(animal.favoriteDogTreatType);
}

One solution to this could be to use a sentinel value:

type Animal = (IAnimal & __typename: "_interface_sentinel_do_not_check_for_me_or_you_will_be_fired_") | Dog | Cat;

Describe alternatives you've considered
Status quo isn't the end of the world! :)

Additional context

The actual issues this can cause is essentially forgetting to write fallback code which then breaks when new interface implementations are added on the server side.

// This type-checks in the current version
function greetAnimal(animal: Animal): string {
  switch (animal.__typename) {
  case "Dog":
    return `Hello, puppy!`;
  case "Cat":
    return `Meow meow.`;
  }
}

This breaks when the server adds an Iguana implementation because there's no default case.

[twavv]

Also, as an aside, the current codegen seems to generate every case for every single interface even when they're not given. I'm not sure that this is desirable behavior.

Ostensibly it's meant to handle the case where a field on the interface is actually a more specific type (e.g., Animal has a field owner: Owner, and Doghas a fieldowner: DogOwnerwhich is an implementation of theOwner` type), but it also means that when we do

animal {
  name
}

the resulting codegen looks like

type MyFragment = ({ __typename: "Dog" } & Pick<Dog, "name">) | ({ __typename: "Cat" } & Pick<Dog, "name">)

(and it grows linearly with the number of interface implementations) whereas it's arguably more reasonably to do

type MyFragment = { name: string };

I'd definitely push for only generating the first thing when you explicitly use those fragments (e.g., animal { name ... on Dog { owner } } still picks name from Dog rather than using the interface version).

[charlypoly]

@travigd

We just merged a solution proposal from @asmundg (#6826) in @graphql-codegen/typescript@2.4.11 📦
Let us know if it fits your use case.

[twavv]

Thanks @charlypoly! Probably won't have time to test this with my own codebase in the near future, but scrolling through the linked PR, and it looks like it doesn't address the original issue of the PR (i.e., falsely pretending like an interface is equivalent to a union of its know implementations). So the switch statement in my example above would still type-check successfully, even though it should not (*this would be a breaking change, so probably behind a config flag, but imo still important).