To Reproduce
https://codesandbox.io/s/graphql-codegen-issue-template-ttfo0
scalar IPV4
scalar IPV6
type IPV4Route {
address: IPV4
gateway: IPV4
}
type IPV6Route {
address: IPV6
gateway: IPV6
}
union RouteUnion = IPV4Route | IPV6Route
type Query {
routes: [RouteUnion!]!
}
fragment NetRoute on RouteUnion {
__typename
... on IPV4Route {
ipv4Address: address
ipv4Gateway: gateway
}
... on IPV6Route {
ipv6Address: address
ipv6Gateway: gateway
}
}
query QQ {
routes {
...NetRoute
}
}
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.0Additional context
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'}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 馃殌
Most helpful comment
The duplicate
__typenamecauses TypeScript to match the typename to the empty object, instead to the actual object (the one if the fragment.This will work:
While this will cause an error:
I will try to reproduce this in a failing test and then fix it. I think we shouldn't add
__typenamein case of union/interface where fragments are preset, because those fragments will already have__typenameset with the correct fields set.Thanks @whtsky!
Btw, @n1ru4l , we can add unit tests and run
validateAndCompileto 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.