graphql-tools icon indicating copy to clipboard operation
graphql-tools copied to clipboard

Inline fragments are only evaluated against the first subschema containing the merge type

Open postsa opened this issue 2 years ago • 3 comments
trafficstars

Issue workflow progress

Progress of the issue based on the Contributor Workflow

  • [X] 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

    Make sure to fork this template and run yarn generate in the terminal.

    Please make sure the GraphQL Tools package versions under package.json matches yours.

  • [X] 2. A failing test has been provided
  • [ ] 3. A local solution has been provided
  • [ ] 4. A pull request is pending review

Describe the bug

See unit tests in PR https://github.com/ardatan/graphql-tools/pull/4965

When we provide the query

  query {
    student(id: "12314241") {
      id
      name
      major {
        ... on MajorSuccess {
          id
          name
        }
        ... on MajorError {
          id
          code
        }
      }
    }

to the stitched schema, it first executes the Student type against the student subschema. The major type that is returned only contains an ID. The presence of a major ID on its own doesn't give us enough information to pick an inline fragment to evaluate. My understanding is that selecting a fragment is the work of either 1) a provided __typename, an __isTypeOf resolver, or the __resolveType resolver. Regardless of which strategy is used, the type is compared to the inline fragments after the student schema returns its response. The student schema does not have enough information to determine whether the major schema will decide that the type is a MajorSuccess or a MajorError. As such, GraphQL fails to match an inline fragment and determines that the major field is completed. Since the major field is completed, no request to the major schema is made.

To Reproduce Steps to reproduce the behavior:

See unit tests in PR https://github.com/ardatan/graphql-tools/pull/4965

Expected behavior I think I expect the selection set containing the inline fragments to be evaluated by subsequent subgraphs containing merge type definitions for the union type. Although I understand that this might not be supported or that I might be missing a configuration option that enables this behavior.

Environment:

  • OS:
  • @graphql-tools/...:
  • NodeJS:

Additional context

postsa avatar Jan 12 '23 18:01 postsa

Hi 👋, we have experienced a similar issue with stitching...

Please see a replication of this issue here https://github.com/metallicOxide/type-merging-interfaces Interestingly, the issue was not present on v7.1 of @graphql-tools/stitch but became present from v8 onwards...

We discovered that with the type merging approach described here the inline fragments were not evaluated and returning null. https://the-guild.dev/graphql/stitching/docs/approaches/type-merging#:~:text=Type%20merging%20allows%20partial%20definitions,type%20in%20the%20gateway%20schema.

We were able to get around this issue by using schema-extensions instead however this is not ideal as we had to create stub types which we extended. The approach is described here -> https://the-guild.dev/graphql/stitching/docs/approaches/schema-extensions#basic-example

metallicOxide avatar Aug 08 '23 02:08 metallicOxide

One possible workaround is to avoid merging union/interface types between two schemas, and instead add the fields that refer to them to the stitched schema, along with a resolver that delegates to the appropriate subschema. Something like this:

let studentSchema = makeExecutableSchema({
  typeDefs: /* GraphQL */ `
    type Query {
      student(id: String!): Student
    }
    type Student {
      id: String!
      name: String!
      # major: Major ## Add this later, in stitching
    }
  `,
});

let majorsSchema = makeExecutableSchema({
  typeDefs: /* GraphQL */ `
    type Query {
      major(id: String!): Major @merge(keyField: "id")
    }
    union Major = MajorError | MajorSuccess # Allow for different return values
    type MajorError {
      id: String!
      code: String!
    }
    type MajorSuccess {
      id: String!
      name: String!
    }
  `,
});

stitchSchemas({
  subschemas: [studentSchema, majorsSchema],
  typeDefs: `extend type Student { major: Major}`,
  resolvers: {
    Student: {
      major: {
        selectionSet: forwardArgsToSelectionSet('{ id }'),
        resolve: (
          selections: { id: string },
          args,
          context,
          info,
        ) => delegateToSchema({
          schema: majorsSchema,
          operation: OperationTypeNode.QUERY,
          fieldName: 'major',
          args: {
            id: selections.id,
          },
          context,
          info,
        }),
      },
    },
  },
});

hedgepigdaniel avatar Aug 09 '23 05:08 hedgepigdaniel

One of the things that also worked for me was making a direct call to delegateToSchema in the resolve key in merge.

let studentSchema = makeExecutableSchema({
  typeDefs: /* GraphQL */ `
    type Query {
      student(id: String!): Student
    }
    type Student {
      id: String!
      name: String!
      major: Major 
    }
  `,
});

let majorsSchema = makeExecutableSchema({
  typeDefs: /* GraphQL */ `
    type Query {
      major(id: String!): Major
    }
    union Major = MajorError | MajorSuccess # Allow for different return values
    type MajorError {
      id: String!
      code: String!
    }
    type MajorSuccess {
      id: String!
      name: String!
    }
  `,
  merge: {
    Major:  {
      selectionSet: '{id}',
      resolve(model, context, info, subschema) {
          return delegateToSchema({
            schema: subschema,
            operation: OperationTypeNode.QUERY,
            fieldName: 'major',
            args: {id: model.id},
            context,
            info,
            skipTypeMerging: true,
          });
      }
    }
  }
});

Strange enough, passing in the 5th argument in resolve (selectionSet) into batchDelegateToSchema actually breaks this...

metallicOxide avatar Aug 09 '23 06:08 metallicOxide