gramps-graphql / gramps--legacy

The core data source combination engine of GrAMPS.
https://gramps.js.org
MIT License
197 stars 13 forks source link

Class methods are lost when spreading over context #76

Open vjpr opened 6 years ago

vjpr commented 6 years ago
query {
  getLatestComic {
    transcript
    year
    month
    day
    link
    news
  }
}
{
  "data": {
    "getLatestComic": null
  },
  "errors": [
    {
      "message": "context.getLatestComic is not a function",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "getLatestComic"
      ]
    }
  ]
}

Server log:

Error: context.getComicById is not a function
    at Object.checkResultAndHandleErrors xxx/packages/xxx-graphql/node_modules/.registry.npmjs.org/graphql-tools/2.18.0/node_modules/graphql-tools/src/stitching/errors.ts:84:7)
    at Object.<anonymous> xxx/packages/xxx-graphql/node_modules/.registry.npmjs.org/graphql-tools/2.18.0/node_modules/graphql-tools/src/stitching/delegateToSchema.ts:97:14)
    at step xxx/packages/xxx-graphql/node_modules/.registry.npmjs.org/graphql-tools/2.18.0/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:40:23)
    at Object.next xxx/packages/xxx-graphql/node_modules/.registry.npmjs.org/graphql-tools/2.18.0/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:21:53)
    at fulfilled xxx/packages/xxx-graphql/node_modules/.registry.npmjs.org/graphql-tools/2.18.0/node_modules/graphql-tools/dist/stitching/delegateToSchema.js:12:58)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

I tried this:

  const XKCD = require('@gramps/data-source-xkcd').default

  const gramps = require('@gramps/gramps').default
  const GraphQLOptions = gramps({
    dataSources: [XKCD],
  })

  router.post(
    '/graphql-gramps',
    koaBody(),
    graphqlKoa(ctx => {
      console.log({ctx})
      return GraphQLOptions(ctx.req)
    }),
  )

I can see all the functions in graphiql, but running them causes this error.

ecwyne commented 6 years ago

I tried this out, and it is was also an issue on apollo-server-express (ie. not a koa-specific issue)

I believe this is because we are spreading over the source context here

getLatestComic is implemented as a class method which are not enumerable

Here is a babel example to show that the class methods are not included in the newly created object {...sourceContext}

@jlengstorf I'm not sure if this means we should change the implementation in data-source-xkcd or somehow change how @gramps/gramps spreads over context

vjpr commented 6 years ago

@ecwyne Good find, and thanks for the detailed explanation!

I'm not sure if this means we should change the implementation in data-source-xkcd or somehow change how @gramps/gramps spreads over context

I would suggest playing it safe, and change how gramps/gramps spreads over context.

jlengstorf commented 6 years ago

@ecwyne @vjpr First: sorry I'm so late to respond here. 😨

The problem of spreading over classes was fixed in the XKCD data source by moving the class under an object property:

https://github.com/gramps-graphql/data-source-xkcd/blob/accebd3840ef5c02365e1f5380d38ad096052c4b/src/index.js#L13

Are you using the latest version of the data source?

I'd be happy to accept a PR that addressed the problem of spreading over classes. I'll update this issue and label it appropriately.