Suggestion: Prefer TypeScript interfaces over type intersections

[fubhy]

Is your feature request related to a problem? Please describe.
There is a significant performance downside to generating complex type intersections that inherit and mutate around each other instead of statically generating more verbose, self-contained interfaces. The negative performance impact is further amplified by using Omit / Pick excessively. In addition to the performance impact, I'd also argue that these constructs are harder to read & reason about, hence are harder to untangle when debugging a type error.

The performance impact of "abusing" complex type declarations, especially in large projects, has also been documented by the typescript core team (e.g. here: https://twitter.com/drosenwasser/status/1319205169918144513) and this issue also has its own section in the typescript documentation: https://github.com/microsoft/TypeScript/wiki/Performance#preferring-interfaces-over-intersections.

In mid-to-large size applications, including ours, the negative performance impact of these type intersections is currently the most significant (albeit granted not the only) contributor to IDE lag when interpreting types for our React application.

Describe the solution you'd like
I'd like to propose to shift to a less "crafty" approach when generating TypeScript type definitions for queries. Instead of generating types and type intersections for pretty much everything, I'd like to see a shift / preference of interfaces wherever possible. Naturally, there's particularly one case where type intersections actually do make sense: modeling graphql union types. For all other cases, however, I don't see a technical reason why types should be preferred given their negative performance impact and worse developer experience / readability.

[ardatan]

Sounds interesting. We'd love to accept any PRs

[fubhy]

I saw that there was an issue for this in 2019 already: #1716 that was closed. I'd like to bring this up again regardless because the original creator did not raise the performance issue in his post.

I ran a codemod on our codebase's generated types to refactor some (not all) of the generated types to interfaces yesterday and that brought tsc --noEmit down from ~30 seconds to ~5 seconds consistently.

[ardatan]

We have two different configuration options that might help; declarationKind: interface and preResolveType: true

[fubhy]

We have two different configuration options that might help; declarationKind: interface and preResolveType: true

Ah yeah I experimented with those settings a bit but what I would love to get rid of are structure like this:

export type AddTrackedAssetsTradeFragment = { __typename: 'AddTrackedAssetsTrade' } & Pick<
  Types.AddTrackedAssetsTrade,
  'id' | 'method' | 'timestamp'
> & {
    adapter: { __typename: 'IntegrationAdapter' } & Pick<Types.IntegrationAdapter, 'id' | 'identifier'>;
  } & AddTrackedAssetsTradeFragment;

Instead of deriving the types for the hook return values for a specific query from these "base" types of the schema, I'd love to see graphql-codegen produce these as fixed interfaces too. They are generated after all so there's no benefit (e.g. avoiding repetition if you'd manually write these) of inheriting and deriving the types of these from the base schema types. I might be msising something here though ;-)

For instance, the output for this could be:

export interface AddTrackedAssetsTradeFragment {
    __typename: 'AddTrackedAssetsTrade';
    id: string;
    method: string;
    timestamp: number;
    adapter: {
      __typename: 'IntegrationAdapter';
      id: string;
      identifier: string;
    }
    incomingAssetAmounts: Array<{
      __typename: 'AssetAmount'
      id: string;
      asset: {
        __typename: 'Asset'
        id: string;
        name: string;
        symbol: string;
        decimals: number;
      };
      price?: {
        __typename: 'AssetPrice'
        price: number;
        timestamp: number;
      };
    }
  }

[fubhy]

I understand that the more verbose output seems a bit off putting at first and it certainly is a bit redundant. Especially considering that you'd likely still want to generate dedicated interfaces also for each individual fragment type. But instead of then producing a bunch of intersection types from these I'd rather have the full query return types declared as flat interfaces too. Both for readability, coherent typescript error output, etc. but also (and especially) for IDE and tooling performance.

Let me know if that's something that would make sense for graphql-codegen from your perspective

[ardatan]

@fubhy In the following example you see codegen can generate types without 'Fragment' and 'Pick'.
I think the only missing thing you expect is interface instead of type.
https://codesandbox.io/s/inspiring-black-7czks?file=/types.ts
But we'd love to accept a PR that improves preResolveTypes behavior

[fubhy]

Sweet. Yes, that's exactly what I meant (generate types without Fragment & Pick). Whether it's type or interface is a secondary concern (type simply invites you to do these crazy Pick & Omit monsters which is why I tell my team usually to prefer interface but in this case there's no practical difference as long as those are avoided 💯 )

Great, that looks like it might work for my use-case. I'll try a bit further later this week to see if I can get it to work for us (just dropping that config in fails with Unknown fragment ... errors so I might need to adjust some other config too or fix a bug if it turns out that this is one).

Thanks!

I'll report back once we've got this working to see if there's any room for tuning this further.

[fubhy]

Turns out that there appears to already be an issue for this bug: dotansimha/graphql-code-generator-community#112

It works without the near-operations-file preset but that's not an option because of the sheer number of queries we have (~50.000 LOC if we dump it all into one file).