graphql icon indicating copy to clipboard operation
graphql copied to clipboard

Inline type comparison fragments fail sometimes when using createUnionType resolveType with classes and transformSchema

Open PiDelport opened this issue 5 years ago • 2 comments

I'm submitting a...

[x] Bug report

Current behavior

When defining a union type with createUnionType and supplying a resolveType that returns classes (not string references), and when transforming the schema (for example with graphql-middleware and graphql-shield), the type sometimes gets resolved to a different type instance than what the GraphQL execution engine uses, which causes inline type comparison fragments on the union type to silently fail and return nothing.

Simplified example

(See below for regression test in our project.)

  • GraphQLModule module definition, with transformSchema to apply middleware:

      GraphQLModule.forRoot({
        ...,
        transformSchema: schema => applyMiddleware(schema, shield(...)),
      })
    
  • Union type definition:

    export const AmountUnion = createUnionType({
      name: 'Amount',
      types: () => [Absolute, Percentage],
      resolveType(value) {
        if (value.absolute) return Absolute;
        if (value.percentage) return Percentage;
        return null;
      },
    });
    
  • Example query snippet:

      ... {
        amount {
          __typename
          ... on Absolute {
            absolute
          }
          ... on Percentage {
            percentage
          }
        }
      }
    

Result (with any middleware applied):

  ... [
    {
      amount: {
        __typename: 'Percentage',
      },
    },
    {
      amount: {
        __typename: 'Absolute',
      },
    },
  ],

Commenting out the transformSchema yields the correct result:

  ... [
    {
      amount: {
        __typename: 'Percentage',
        percentage: 50,
      },
    },
    {
      amount: {
        __typename: 'Absolute',
        absolute: 10,
      },
    },
  ],

Expected behavior

The query's inline type comparison fragments should match correctly even when the schema is transformed.

Root cause

From my investigation so far, I think this is an instance of the following bug:

  • https://github.com/graphql/graphql-js/issues/1093 (extendSchema breaks resolution of fragments on interfaces / unions)
  • https://github.com/MichalLytek/type-graphql/issues/583 (Union type doesn't work when running two schemas together)
  • https://github.com/MichalLytek/type-graphql/issues/605 (Reference mismatch for resolveType with classes)

The fix is likely similar to the fixes implemented for type-graphql here:

  • https://github.com/MichalLytek/type-graphql/commit/7bf668265071f6efeeb01cc9fbbc810f5cbe5ba8 (fix: properly transform result of resolveType option)
  • https://github.com/MichalLytek/type-graphql/commit/bb530662a979d88f378468454d4e388101b9b940 (fix(unions): allow sharing union type between schemas)

(In resolve-type.factory.ts and/or union-definition.factory.ts?)

Minimal reproduction of the problem with instructions

Regression test and workaround in this PR: https://github.com/registreerocks/registree-core/pull/373

  • https://github.com/registreerocks/registree-core/pull/373#issuecomment-779103360 for test failure output
  • https://github.com/registreerocks/registree-core/commit/29c47e82a7e004b1dd48e2d7e4ab86818f23c115 for union type definition and workaround

Environment

Nest version:

  • @nestjs/graphql 7.9.8

Other:

  • Node version: v15.8.0
  • Platform: Ubuntu 20.04.2 LTS

PiDelport avatar Feb 15 '21 12:02 PiDelport

Would you like to create a PR for this issue?

kamilmysliwiec avatar Feb 24 '21 11:02 kamilmysliwiec

Well, I took a stab at making a reproducing test case in the nestjs/graphql tests, but it doesn't seem entirely trivial: I'll have to dig deeper when I can make some time for it.

PiDelport avatar Mar 04 '21 14:03 PiDelport