Optional context should return request by default

[katopz]

Hi guys,

I currently use both express-graphql and apollo-server by sharing executable schema, so far so good but suddenly hit some different here.

Refer to express-graphql : https://github.com/graphql/express-graphql/blob/bdd7a954aff7621bb338d722b44371f7643c2258/src/index.js#L133

context = optionsData.context || request;

But apollo-server : https://github.com/apollostack/apollo-server/blob/f4546e7654a2b12bb77ef5af407d89d1a62e8d6d/src/integrations/expressApollo.ts#L94

context: optionsObject.context,

So propose solution should be...

context: optionsObject.context || req,

Which maybe somehow related to #144

Any thought?

[helfer]

@katopz I actually think it's a bad feature of express-graphql that it sets the context to be the request by default. This way it won't be possible to set the context to false, 0 or undefined for example. The right way to set the context to be the request object is to do it explicitly. In my opinion that should be true for both express-graphql and apollo-server.

I think you can just do this for express-graphql?

   app.use( graphqlHTTP(req => ({ context: req }) );

[katopz]

@helfer Cool, so we should open issue to express-graphql instead then?
IMHO it should work the same way or should docs/tests somewhere if it's not.

Thanks

[stubailo]

This was a feature added to express to make it possible to conveniently set context via middleware. I don't think Apollo server needs to be exactly the same as express-graphql though right? I kind of enjoy that Apollo server is minimal and makes it really clear exactly what is happening.

[katopz]

@stubailo Well, it should be docs for the diff in that case :)
BTW I'm not sure how can I add req in this case? or I shouldn't do this?

// `context` must be an object and can't be undefined when using connectors
graphQLServer.use('/graphql', bodyParser.json(), apolloExpress({
  schema: executableSchema,
  context: req // this shouldn't work? req is undefined?
}));

Thanks

[stubailo]

graphQLServer.use('/graphql', bodyParser.json(), apolloExpress((req) => ({
  schema: executableSchema,
  context: req // this shouldn't work? req is undefined?
})));

Did we not document that you can pass options as a function? That's definitely something we should fix.

@helfer do you know if that works for every server, or just for Express?

[helfer]

I know it works for express and connect. Koa will use the context, and I'm not sure about Hapi, but I think it works there as well. The request object is somewhat different though.