dotansimha / dotansimha/graphql-code-generator

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

Open
#8,538 1 comment 2 reactions 0 assignees View on GitHub
core
Dominant language
TypeScript
Stars
11.3k
Forks
1.4k
Avg merge
1d 1h
Merged PRs (30d)
23

Description

**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:

```graphql
query {
pet {
name
... on Cat {
meow
}
... on Dog {
bark
}
}
}
```

```ts
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:

```ts
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:

```ts
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:

```ts
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):

```ts
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](https://typescript-eslint.io/rules/switch-exhaustiveness-check/), but it only works on union types. I played around with something like

```ts
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!

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.