Schema extensions with interfaces fails in v7

[Grmiade]

Description

As you can see in the tests suite, schema extensions with interfaces fails in v7 when we use complexe fragments.
I wrote this regression test "Schema extensions with interfaces -> should resolve with 2 fragments" to show you a failure example.
I also added you some successful examples with simple queries with interfaces, complexe queries without interface and type merging.
I hope this will help you ;)

Maybe related to #2157?

Tests suite

import { batchDelegateToSchema } from '@graphql-tools/batch-delegate'
import { makeExecutableSchema } from '@graphql-tools/schema'
import { stitchSchemas } from '@graphql-tools/stitch'
import { graphql } from 'graphql'

const expectedData = [
  {
    network: { id: '57', domains: [{ id: '60', name: 'network57.com' }] },
  },
]

describe('Schema extensions with interfaces', () => {
  const networkSchema = makeExecutableSchema({
    typeDefs: `
      interface Domain {
        id: ID!
        name: String!
      }
      type Domain1 implements Domain {
        id: ID!
        name: String!
      }
      type Domain2 implements Domain {
        id: ID!
        name: String!
        extra: String!
      }
      type Network {
        id: ID!
        domains: [Domain!]!
      }
      type Query {
        networks(ids: [ID!]!): [Network!]!
      }
    `,
    resolvers: {
      Domain: {
        __resolveType() {
          return 'Domain1'
        },
      },
      Query: {
        networks: (root, { ids }) =>
          ids.map((id) => ({ id, domains: [{ id: Number(id) + 3, name: `network${id}.com` }] })),
      },
    },
  })

  const postsSchema = makeExecutableSchema({
    typeDefs: `
      type Network {
        id: ID!
      }
      type Post {
        id: ID!
        network: Network!
      }
      type Query {
        posts(ids: [ID!]!): [Post]!
      }
    `,
    resolvers: {
      Query: {
        posts: (root, { ids }) =>
          ids.map((id) => ({
            id,
            network: { id: Number(id) + 2 },
          })),
      },
    },
  })

  const gatewaySchema = stitchSchemas({
    subschemas: [{ schema: networkSchema }, { schema: postsSchema }],
    resolvers: {
      Post: {
        network: {
          selectionSet: '{ network { id } }',
          resolve(parent, _args, context, info) {
            return batchDelegateToSchema({
              key: parent.network.id,
              argsFromKeys: (ids) => ({ ids }),
              context,
              fieldName: 'networks',
              info,
              operation: 'query',
              schema: networkSchema,
            })
          },
        },
      },
    },
  })

  it('should resolve with no fragments', async () => {
    const { data } = await graphql(
      gatewaySchema,
      `
        query {
          posts(ids: [55]) {
            network {
              id
              domains {
                id
                name
              }
            }
          }
        }
      `,
    )

    expect(data.posts).toEqual(expectedData)
  })

  it('should resolve with 1 fragment', async () => {
    const { data } = await graphql(
      gatewaySchema,
      `
        query {
          posts(ids: [55]) {
            network {
              domains {
                name
              }
            }
            ...F2
          }
        }

        fragment F2 on Post {
          network {
            id
            domains {
              id
            }
          }
        }
      `,
    )

    expect(data.posts).toEqual(expectedData)
  })

  // Fails with `Cannot return null for non-nullable field Domain1.id` because batchDelegateToSchema returns
  // { domains: [{ name: 'network57.com', __typename: 'Domain1' }], id: '57' }
  it('should resolve with 2 fragments', async () => {
    const { data } = await graphql(
      gatewaySchema,
      `
        query {
          posts(ids: [55]) {
            network {
              domains {
                name
              }
            }
            ...F2
          }
        }

        fragment F2 on Post {
          network {
            ...F3
          }
        }

        fragment F3 on Network {
          id
          domains {
            id
          }
        }
      `,
    )

    expect(data.posts).toEqual(expectedData)
  })
})

describe('Schema extensions with no interfaces', () => {
  const networkSchema = makeExecutableSchema({
    typeDefs: `
      type Domain {
        id: ID!
        name: String!
      }
      type Network {
        id: ID!
        domains: [Domain!]!
      }
      type Query {
        networks(ids: [ID!]!): [Network!]!
      }
    `,
    resolvers: {
      Query: {
        networks: (root, { ids }) =>
          ids.map((id) => ({ id, domains: [{ id: Number(id) + 3, name: `network${id}.com` }] })),
      },
    },
  })

  const postsSchema = makeExecutableSchema({
    typeDefs: `
      type Network {
        id: ID!
      }
      type Post {
        id: ID!
        network: Network!
      }
      type Query {
        posts(ids: [ID!]!): [Post]!
      }
    `,
    resolvers: {
      Query: {
        posts: (root, { ids }) =>
          ids.map((id) => ({
            id,
            network: { id: Number(id) + 2 },
          })),
      },
    },
  })

  const gatewaySchema = stitchSchemas({
    subschemas: [{ schema: networkSchema }, { schema: postsSchema }],
    resolvers: {
      Post: {
        network: {
          selectionSet: '{ network { id } }',
          resolve(parent, _args, context, info) {
            return batchDelegateToSchema({
              key: parent.network.id,
              argsFromKeys: (ids) => ({ ids }),
              context,
              fieldName: 'networks',
              info,
              operation: 'query',
              schema: networkSchema,
            })
          },
        },
      },
    },
  })

  it('should resolve with 2 fragments', async () => {
    const { data } = await graphql(
      gatewaySchema,
      `
        query {
          posts(ids: [55]) {
            network {
              domains {
                name
              }
            }
            ...F2
          }
        }

        fragment F2 on Post {
          network {
            ...F3
          }
        }

        fragment F3 on Network {
          id
          domains {
            id
          }
        }
      `,
    )

    expect(data.posts).toEqual(expectedData)
  })
})

describe('Type merging with interfaces', () => {
  const networkSchema = makeExecutableSchema({
    typeDefs: `
      interface Domain {
        id: ID!
        name: String!
      }
      type Domain1 implements Domain {
        id: ID!
        name: String!
      }
      type Domain2 implements Domain {
        id: ID!
        name: String!
        extra: String!
      }
      type Network {
        id: ID!
        domains: [Domain!]!
      }
      type Query {
        networks(ids: [ID!]!): [Network!]!
      }
    `,
    resolvers: {
      Domain: {
        __resolveType() {
          return 'Domain1'
        },
      },
      Query: {
        networks: (root, { ids }) =>
          ids.map((id) => ({ id, domains: [{ id: Number(id) + 3, name: `network${id}.com` }] })),
      },
    },
  })

  const postsSchema = makeExecutableSchema({
    typeDefs: `
      type Network {
        id: ID!
      }
      type Post {
        id: ID!
        network: Network!
      }
      type Query {
        posts(ids: [ID!]!): [Post]!
      }
    `,
    resolvers: {
      Query: {
        posts: (root, { ids }) =>
          ids.map((id) => ({
            id,
            network: { id: Number(id) + 2 },
          })),
      },
    },
  })

  const gatewaySchema = stitchSchemas({
    subschemas: [
      {
        schema: networkSchema,
        merge: {
          Network: {
            selectionSet: '{ id }',
            fieldName: 'networks',
            key: ({ id }) => id,
            argsFromKeys: (ids) => ({ ids }),
          },
        },
      },
      {
        schema: postsSchema,
      },
    ],
  })

  it('should resolve with 2 fragments', async () => {
    const { data } = await graphql(
      gatewaySchema,
      `
        query {
          posts(ids: [55]) {
            network {
              domains {
                name
              }
            }
            ...F2
          }
        }

        fragment F2 on Post {
          network {
            ...F3
          }
        }

        fragment F3 on Network {
          id
          domains {
            id
          }
        }
      `,
    )

    expect(data.posts).toEqual(expectedData)
  })
})

[gmac]

FWIW, I had experienced some quirks with schema extensions that I cleared up by converting them all to merged types. I’ve been much happier without any extensions in the mix. That’s not to imply your issue here isn’t relevant, only that I highly recommend making the switch!

[gmac]

One other question: were you using merged types before? (mergeTypes:true) That option is now enabled by default in v7. If your stitching setup is expecting to run without merging, you might try adding mergeTypes:false as a stitching option.

[yaacovCR]

Just looking this over quickly, I think in the first example, the postsSchema has a resolver on the gateway, but instead of extending the schema on the gateway with the Network type and the new network field on the Post type, it just includes this type and field within the postsSchema. That may give unexpected behavior. Resolvers on the gateway should really usually match new fields on the gateway...

[yaacovCR]

Not sure if that's the cause per se will have time to dig in hopefully later this week

[Grmiade]

One other question: were you using merged types before? (mergeTypes:true) That option is now enabled by default in v7. If your stitching setup is expecting to run without merging, you might try adding mergeTypes:false as a stitching option.

@gmac We only use schema extensions on the impacted project. I tried, setting mergeTypes to false didn't change anything about this issue ;)

[yaacovCR]

Not sure what you mean about only using extensions as I do not see any types added on the gateway. Was able to fix your example using type merging settings.

If you really need, I can give you some feedback on how to implement this with new fields on the gateway, but I am not sure if that's what you really want...

The new suggested pattern is the merging pattern with fix as in associated PR...

[Grmiade]

When I speak about type merging or schema extensions, I make a reference to these documentations:
Type merging and Schema Extensions ;)
We use typeDefs and resolvers to define our gateway without using the merge API.

So if we want to use the v7, we will have to migrate to the new merging pattern (Type merging)?

[yaacovCR]

I don't believe so -- but your failing test did not properly use typeDefs/resolvers, see above. Can you fix it, and then I can further troubleshoot once I have a minimal reproduction that reflects your setup...

Sorry about this bug! Must be very frustrating. Hopeful fix will be forthcoming soon. Just need to better understand your setup so that I can expedite

[Grmiade]

@yaacovCR This failing test is ok for you?

import { batchDelegateToSchema } from '@graphql-tools/batch-delegate'
import { makeExecutableSchema } from '@graphql-tools/schema'
import { stitchSchemas } from '@graphql-tools/stitch'
import { graphql } from 'graphql'

const networkSchema = makeExecutableSchema({
  typeDefs: `
      interface Domain {
        id: ID!
        name: String!
      }
      type Domain1 implements Domain {
        id: ID!
        name: String!
      }
      type Domain2 implements Domain {
        id: ID!
        name: String!
        extra: String!
      }
      type Network {
        id: ID!
        domains: [Domain!]!
      }
      type Query {
        networks(ids: [ID!]!): [Network!]!
      }
    `,
  resolvers: {
    Domain: {
      __resolveType() {
        return 'Domain1'
      },
    },
    Query: {
      networks: (root, { ids }) =>
        ids.map((id) => ({ id, domains: [{ id: Number(id) + 3, name: `network${id}.com` }] })),
    },
  },
})

const postsSchema = makeExecutableSchema({
  typeDefs: `
      type Post {
        id: ID!
        networkId: ID!
      }
      type Query {
        posts(ids: [ID!]!): [Post]!
      }
    `,
  resolvers: {
    Query: {
      posts: (root, { ids }) =>
        ids.map((id) => ({
          id,
          networkId: Number(id) + 2,
        })),
    },
  },
})

const gatewaySchema = stitchSchemas({
  subschemas: [networkSchema, postsSchema],
  typeDefs: `
    extend type Post {
      network: Network!
    }
  `,
  resolvers: {
    Post: {
      network: {
        selectionSet: '{ networkId }',
        resolve(parent, _args, context, info) {
          return batchDelegateToSchema({
            key: parent.networkId,
            argsFromKeys: (ids) => ({ ids }),
            context,
            fieldName: 'networks',
            info,
            operation: 'query',
            schema: networkSchema,
          })
        },
      },
    },
  },
})

const expectedData = [
  {
    network: { id: '57', domains: [{ id: '60', name: 'network57.com' }] },
  },
]

// Passed
it('should resolve with no fragments', async () => {
  const { data } = await graphql(
    gatewaySchema,
    `
      query {
        posts(ids: [55]) {
          network {
            id
            domains {
              id
              name
            }
          }
        }
      }
    `,
  )

  expect(data.posts).toEqual(expectedData)
})

// Passed
it('should resolve with 1 fragment', async () => {
  const { data } = await graphql(
    gatewaySchema,
    `
      query {
        posts(ids: [55]) {
          network {
            domains {
              name
            }
          }
          ...F1
        }
      }

      fragment F1 on Post {
        network {
          id
          domains {
            id
          }
        }
      }
    `,
  )

  expect(data.posts).toEqual(expectedData)
})

// Failed
// `Cannot return null for non-nullable field Domain1.id`
// batchDelegateToSchema returns { domains: [{ name: 'network57.com', __typename: 'Domain1' }], id: '57' }
it('should resolve with 2 fragments', async () => {
  const { data, errors } = await graphql(
    gatewaySchema,
    `
      query {
        posts(ids: [55]) {
          network {
            domains {
              name
            }
          }
          ...F1
        }
      }

      fragment F1 on Post {
        network {
          ...F2
        }
      }

      fragment F2 on Network {
        id
        domains {
          id
        }
      }
    `,
  )

  if (errors) console.log(errors)
  expect(data.posts).toEqual(expectedData)
})

[yaacovCR]

Now we are talking. Do me a favor and check if it works with inline fragment.

[Grmiade]

Like this?

it('should resolve with 2 inline fragments', async () => {
  const { data } = await graphql(
    gatewaySchema,
    `
      query {
        posts(ids: [55]) {
          network {
            domains {
              name
            }
          }
          ... on Post {
            network {
              ... on Network {
                id
                domains {
                  id
                }
              }
            }
          }
        }
      }
    `,
  )

  expect(data.posts).toEqual(expectedData)
})

It works with this syntax!

[yaacovCR]

Ok! Bug is within the Visit selection sets transformer

Here: https://github.com/ardatan/graphql-tools/blob/master/packages/delegate/src/transforms/VisitSelectionSets.ts#L129

typeInfo is initialized with Post for root fields, but for fragments should be initialized to individual fragment type

[yaacovCR]

At least I think. I would have thought visitWithTypeInfo handles this without any special processing, but can dig in hopefully later today....

[yaacovCR]

actually, error completely elsewhere, visitWithTypeInfo behaves as expected, this was a regression introduced by WrapConcreteTypes refactoring in which all fragments were being processed (incorrectly) when only fragments on root types should have been processed.

[Grmiade]

Thanks @yaacovCR, our issue is fixed in the latest versions 🎉