Skip to content
This repository has been archived by the owner on Sep 11, 2019. It is now read-only.

Stitching causes errors when using graphql@0.12.x #50

Closed
jlengstorf opened this issue Dec 19, 2017 · 5 comments · Fixed by #53
Closed

Stitching causes errors when using graphql@0.12.x #50

jlengstorf opened this issue Dec 19, 2017 · 5 comments · Fixed by #53

Comments

@jlengstorf
Copy link
Member

I noticed an issue with graphql-tools when we tried to upgrade to graphql@0.12.x: ardatan/graphql-tools#537

The issues are caused in our tests, and after a little digging I figured out it's happening in our stitching handler.

Here's one of the two offending tests:

    it('warns for use of schema', () => {
      console.warn = jest.genMockFn();
      const dataSources = [
        {
          namespace: 'Baz',
          schema: 'type User { name: String } type Query { me: User }',
          context: req => ({ getUser: () => ({ name: 'Test user' }) }),
          resolvers: { Query: { me: (_, __, context) => context.getUser() } },
          stitching: {
            linkTypeDefs: 'extend type User { age: Int }',
            resolvers: mergeInfo => ({
              User: {
                age: () => 40,
              },
            }),
          },
        },
      ];

      gramps({ dataSources });

      return expect(console.warn).toBeCalled();
    });

Run as-is, we get the following error:

$ npm run lint --silent && npm run test:unit --silent
 FAIL  test/gramps.test.js
  ● GrAMPS › gramps() › warns for use of schema

    TypeError: Cannot read property 'getFields' of null

      143 |         typeof source.context === 'function'
      144 |           ? source.context(req)
    > 145 |           : source.context;
      146 |
      147 |       return {
      148 |         ...allContext,

      at node_modules/graphql-tools/src/stitching/mergeSchemas.ts:114:27
          at Array.forEach (<anonymous>)
      at mergeSchemas (node_modules/graphql-tools/src/stitching/mergeSchemas.ts:85:17)
      at gramps (src/gramps.js:145:1430)
      at Object.it.only (test/gramps.test.js:41:25)

But if I remove the stitching prop and run the test like this:

    it.only('warns for use of schema', () => {
      console.warn = jest.genMockFn();
      const dataSources = [
        {
          namespace: 'Baz',
          schema: 'type User { name: String } type Query { me: User }',
          context: req => ({ getUser: () => ({ name: 'Test user' }) }),
          resolvers: { Query: { me: (_, __, context) => context.getUser() } },
        },
      ];

      gramps({ dataSources });

      return expect(console.warn).toBeCalled();
    });

It passes as expected.

Nothing looks out-of-place in https://github.com/gramps-graphql/gramps/blob/master/src/lib/combineStitchingResolvers.js

I don't see anything at first glance that looks obviously weird in the execution, either:

https://github.com/gramps-graphql/gramps/blob/master/src/gramps.js#L129-L138

We need to dig into this a little deeper and figure out what's going wrong.

@ecwyne
Copy link
Collaborator

ecwyne commented Dec 19, 2017

ardatan/graphql-tools#537 (comment)

The issue is actually in graphql-tools. mergeSchemas() should take an Array<GraphQLSchema | string> but using graphql@0.12.x breaks this somehow

@jlengstorf
Copy link
Member Author

Ah, okay. So this should work itself out once the fix goes into graphql-tools.

Thanks for putting that test case together!

@ecwyne
Copy link
Collaborator

ecwyne commented Dec 19, 2017

I believe so, though it looks like there may be a bit of a new api coming down the pipe.

It looks like this is published on npm as graphql-tools@2.15.0-alpha.1 but it still doesn't work in my test case.

@freiksenet
Copy link

freiksenet commented Dec 20, 2017

Will be fixed soon. ardatan/graphql-tools#547

@ecwyne
Copy link
Collaborator

ecwyne commented Dec 20, 2017

Thanks so much @freiksenet!

@jlengstorf it looks like graphql-tools@0.14.1 has been published which fixes the issue with graphql@0.12.x

jlengstorf added a commit that referenced this issue Dec 20, 2017
jlengstorf added a commit that referenced this issue Dec 20, 2017
* chore: upgrade graphql and graphql-tools

close #50

* fix: pin graphql-tools to compatible version

2.14.1 is required for operation with graphql@0.12.x
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants