Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Add GraphQLSchema.context to context in runGraphQL #2474

Merged
merged 1 commit into from
Dec 13, 2019

Conversation

doronrk
Copy link
Contributor

@doronrk doronrk commented Dec 11, 2019

This was causing a TypeError in vulcan-files because the file collection was not being added to the context that is passed to the resolvers in that package when querying with runGraphQL

@eric-burel
Copy link
Contributor

Just out of curiosity why merging with the graphQL schema context solves this? As far as I understand the file collection does not belong so it's missing in the Collections object, that was your issue?

@doronrk
Copy link
Contributor Author

doronrk commented Dec 12, 2019

The vulcan-files package adds the file collection to the context with

GraphQLSchema.addToContext({ [collectionName]: FSCollection });

and all of its resolvers expect to be able to find the collection at context[collectionName]. But the collection is missing from the context object when run with runGraphQL because only the apollo-server context initialization did the merge, but not runGraphQL.

That was the specific issue we were facing, but in general, it seems that GraphQLSchema.addToContext should be respected regardless of whether the entry point to the query is from the client via the apollo server or a server-side query via runGraphQL.

Does that make sense? Happy to clarify further.

@eric-burel
Copy link
Contributor

Yeah I think I undestand, I didn't even know about this addtoContext (I barely actually use Vulcan GraphQL related features outside of automated generation)

@eric-burel eric-burel merged commit 5be2b75 into VulcanJS:devel Dec 13, 2019
@eric-burel
Copy link
Contributor

The collection should be registered in the Collection object somehow I think, it's overkill to extend the contextfor this, but you are right that some other part of the code could want to update the context for other reasons through this method.

I think my idea was to use a callback instead to enhance the context, this may need some clarification in future releases.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants