-
Notifications
You must be signed in to change notification settings - Fork 2k
Context does not maintain value across resolve functions #953
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
Comments
Hey @ErikWittern let schema = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'RootQueryType',
fields: {
user: {
type: new GraphQLObjectType({
name: 'user',
fields: {
name: {type: GraphQLString},
address: {
type: new GraphQLObjectType({
name: 'address',
fields: {
city: {type: GraphQLString},
street: {type: GraphQLString}
}
}),
resolve (root, args, ctx) {
return {
city: 'New York',
street: root.street,
}
}
}
}
}),
resolve (root, args, ctx) {
return {
name: 'Erik',
street: '10th Street',
}
}
}
}
})
})
let query = '{user: {address: {street}}}'
graphql(schema, query) |
@DxCx Thank you for your comments! I have two follow-up questions, maybe you can help me with them. First, I wonder whether having to explicitly provide a context is a good design-choice? I personally don't like the syntax Second, I struggle to understand why the context should not be modified in queries. I assumed queries should be safe and idempotent with regard to the underlying data, but why with regard to the context? To give the concrete use-case I face: I have a resolve function for {
user (id: "123") {
car { // needs to be resolved by fetching data via HTTP from ../users/{id}/car
model
}
}
} To perform the request |
Nope, context should be used for isolating implementation from schema itself. {
getUser(id),
getUserCar(id),
} this way the resolver itself of user will call the function provided in context may send http request to reterive data, and later on can be updated to connect to database instead, schema remains the same. |
@DxCx Again, thank you for your comments! I am getting a much better picture now about the intent / semantics of context. In my specific scenario, the resolve function of In any case, I will close this issue as it was rather a misunderstanding on my part than an actual problem with |
@ErikWittern why not multiplexing the original id given to getUser? hence: function getUser(id) {
return getUserDataFromEndpoint(id)
.then((res) => Object.assign({ id }, res));
} |
@DxCx Yes, that makes sense in this example, I agree! Thank you! |
I have problems using the
context
in resolve functions. Specifically, I want to add / change values in the context to reuse them in different resolve functions across a single query. Consider this example:In the resolve function for user,
ctx
is initially undefined. My attempt to setctx
to{street: '10th Street'}
in the resolve function for user does not succeed: once the resolve function foraddress
is called,ctx
is once againundefined
, causing an error.The above code does work if I provide a context explicitly during querying (i.e., if I change the last line in the code), for example:
While some libraries pass a context per default, (for example, express-graphql sets the context to the Express.js request object) should
graphql-js
not per default provide an empty context object? Or at least hold the value set toctx
across resolve functions related to a single query?This issue may be related to #354, the discussion of which drifted towards parallel execution of resolve functions, though.
The text was updated successfully, but these errors were encountered: