Graphql-code-generator: TypeScript: Fragment + Union gives duplicated (wrong) __typename

Created on 4 Sep 2019  路  7Comments  路  Source: dotansimha/graphql-code-generator

To Reproduce
https://codesandbox.io/s/graphql-codegen-issue-template-ttfo0

  1. My GraphQL schema:
  scalar IPV4
  scalar IPV6
  type IPV4Route {
    address: IPV4
    gateway: IPV4
  }
  type IPV6Route {
    address: IPV6
    gateway: IPV6
  }
  union RouteUnion = IPV4Route | IPV6Route
  type Query {
    routes: [RouteUnion!]!
  }
  1. My GraphQL operations:

  fragment NetRoute on RouteUnion {
    __typename
    ... on IPV4Route {
      ipv4Address: address
      ipv4Gateway: gateway
    }
    ... on IPV6Route {
      ipv6Address: address
      ipv6Gateway: gateway
    }
  }
  query QQ {
    routes {
      ...NetRoute
    }
  }
  1. Output:
type NetRouteFragment = (
  { __typename: 'IPV4Route' }
  & { ipv4Address: Ipv4Route['address'], ipv4Gateway: Ipv4Route['gateway'] }
) | (
  { __typename: 'IPV6Route' }
  & { ipv6Address: Ipv6Route['address'], ipv6Gateway: Ipv6Route['gateway'] }
);

type QqQuery = (
  { __typename?: 'Query' }
  & { routes: Array<({ __typename?: 'IPV4Route' } | { __typename?: 'IPV6Route' })
    & NetRouteFragment
  > }
);

Expected output

type NetRouteFragment = (
  { __typename: 'IPV4Route' }
  & { ipv4Address: Ipv4Route['address'], ipv4Gateway: Ipv4Route['gateway'] }
) | (
  { __typename: 'IPV6Route' }
  & { ipv6Address: Ipv6Route['address'], ipv6Gateway: Ipv6Route['gateway'] }
);

type QqQuery = (
  { __typename?: 'Query' }
  & { routes: Array<NetRouteFragment> }
);

Environment:

  • @graphql-codegen/typescript-operations: 1.7.0

Additional context

bug plugins waiting-for-release

Most helpful comment

The duplicate __typename causes TypeScript to match the typename to the empty object, instead to the actual object (the one if the fragment.

This will work:

type QqQuery = (
  { __typename?: 'Query' }
  & { routes: Array<NetRouteFragment> }
);

While this will cause an error:

type QqQuery = (
  { __typename?: 'Query' }
  & { routes: Array<({ __typename?: 'IPV4Route' } | { __typename?: 'IPV6Route' })
    & NetRouteFragment
  > }
);

I will try to reproduce this in a failing test and then fix it. I think we shouldn't add __typename in case of union/interface where fragments are preset, because those fragments will already have __typename set with the correct fields set.

Thanks @whtsky!

Btw, @n1ru4l , we can add unit tests and run validateAndCompile to fail the error in case of an invalid TypeScript, you can also add usage and the same errors you are getting in TS Playground. I'll add a test like that and share it with you.

All 7 comments

Hi @whtsky !

Can you please explain why is that an issue?

In GraphQL, interface and union doesn't really have a __typename field, even if you specify in over the interface or over the union, the final value will be a type (because interface and union are abstract).

In your case, the RouteUnion union can return either IPV4Route or IPV6Route, and the types generates reflects this exact case - it can return either an object with with { __typename: 'IPV4Route' } or { __typename: 'IPV6Route' }.

The duplication is there because we first need to declare the abstract type and the possible types of that, and then the fragment specifies fields per each possible type and overrides the __typename.

Am I missing something? Does it breaks for you or you have any usage issues?

Hi @dotansimha.

The duplicated __typename makes the type incorrect.

Array<({ __typename?: 'IPV4Route' } | { __typename?: 'IPV6Route' })
    & NetRouteFragment
  >

means the element can be

  • {__typename: undefined}
  • {__typename: 'IPV4Route'}
  • {__typename: 'IPV6Route'}
  • or NetRouteFragment:
(
  { __typename: 'IPV4Route' }
  & { ipv4Address: Ipv4Route['address'], ipv4Gateway: Ipv4Route['gateway'] }
) | (
  { __typename: 'IPV6Route' }
  & { ipv6Address: Ipv6Route['address'], ipv6Gateway: Ipv6Route['gateway'] }
)

Does it breaks for you or you have any usage issues?

we have code like

if (route.__typename === 'IPV4Route') {
    return validateIPV4(route.ipv4Gateway)
} else {
    return validateIPV6(route.ipv6Gateway)
}

when the route.__typename is IPV4Route, route's type is

(
  { __typename: 'IPV4Route' }
  & { ipv4Address: Ipv4Route['address'], ipv4Gateway: Ipv4Route['gateway'] }
) | { __typename?: 'IPV4Route' }

... which is incorrect and produces type error.

TS Playground:

https://www.typescriptlang.org/play/#code/C4TwDgpgBAkmBuAWASgewK7GgXigbwCgpioBDAE3ICcIBnWgLiluCoEsA7AcyJK9KwB3UiCYt23AgF8CBUJFgIAbGkw58vYhWp1GzVpx4ko-ISLEHJM2fOgA5CMFVYAYlVJcAthA7AouAApePCgAfVDbDlJvJgByGAAFADUUDCxYqBliADJ8KDYERABBShp6JjgkZwgAbVjtMtpYgF0AGnzCgHEBCGFRRSq02tjTXpEWzIIASigAHygg4hDwyOiIOMSklSGMrKhckIL4JRKdcoHttTqG3Rb2o6Vus37Ky6w60b6JmSmAbhtwNAAIoARyB6AgVBASVI7FIACMADZ0fz4KT-AEKUHgyEgVGLPIrQFRbwAfjiOKhu14BygVCGeiKVHcIAAPAFlhFiWtyVB4slUmpdnNCVzICSILz+Vtqrsppp9lAHE4hm4PN5fLwAHyTP6yOSAqDVVHYiFQur0tRNZo1DjoTzwyHNWQAM3QHAAxsA2KgOFB4KREWxyD0nmMQAFLVgmNUZoRjGwXQsoxAAHRE8VrfzYXDSwXpOMK4we320VDI1OI1BcSNDVNHRBhvry4xSKAQRG0aDx4wkEscMsVqs1lP15RNkQtkgyKRAA

Can we have some tests for checking whether the properties can be accessed?

The duplicate __typename causes TypeScript to match the typename to the empty object, instead to the actual object (the one if the fragment.

This will work:

type QqQuery = (
  { __typename?: 'Query' }
  & { routes: Array<NetRouteFragment> }
);

While this will cause an error:

type QqQuery = (
  { __typename?: 'Query' }
  & { routes: Array<({ __typename?: 'IPV4Route' } | { __typename?: 'IPV6Route' })
    & NetRouteFragment
  > }
);

I will try to reproduce this in a failing test and then fix it. I think we shouldn't add __typename in case of union/interface where fragments are preset, because those fragments will already have __typename set with the correct fields set.

Thanks @whtsky!

Btw, @n1ru4l , we can add unit tests and run validateAndCompile to fail the error in case of an invalid TypeScript, you can also add usage and the same errors you are getting in TS Playground. I'll add a test like that and share it with you.

I think I managed to reproduce and fix it in: https://github.com/dotansimha/graphql-code-generator/pull/2540
also there was an issues with the wrapping ( when there are inline fragments and fragments spread together.
Now it will add __typename only where required, without applying it on the usage of fragments.

@whtsky can you please try 1.7.1-alpha-812c162b.42?
@n1ru4l can you please review?

@dotansimha 1.7.1-alpha-812c162b.42 looks good, thanks

Fixed in 1.8.0 馃殌

Was this page helpful?
0 / 5 - 0 ratings