Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 of 4 tasks
Tracked by #5201 ...
postsa opened this issue Jan 12, 2023 · 3 comments
Open
2 of 4 tasks
Tracked by #5201 ...

Comments

@postsa
Copy link

postsa commented Jan 12, 2023

Issue workflow progress

Progress of the issue based on the Contributor Workflow

  • 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.

  • 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 #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 #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

@metallicOxide
Copy link

metallicOxide commented Aug 8, 2023

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

@hedgepigdaniel
Copy link

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,
        }),
      },
    },
  },
});

@metallicOxide
Copy link

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants