Skip to content
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

Fix: RootValue is a subvalue of fourth argument in the resolver method #160

Merged
merged 1 commit into from
Oct 6, 2016

Conversation

nodkz
Copy link
Contributor

@nodkz nodkz commented Oct 6, 2016

No description provided.

@stubailo
Copy link
Contributor

stubailo commented Oct 6, 2016

I feel like we should just get rid of rootValue entirely - isn't context always the right way to do this?

@nodkz
Copy link
Contributor Author

nodkz commented Oct 6, 2016

Eg. DB connection pool is host specific (not request/session) so it better to keep in rootValue.

Conversation in past relay-tools/react-relay-network-layer#9 (comment)

@stubailo
Copy link
Contributor

stubailo commented Oct 6, 2016

DB connection pool is host specific (not request/session) so it better to keep in rootValue.

Why does this mean it can't be part of context?

@stubailo
Copy link
Contributor

stubailo commented Oct 6, 2016

Based on my understanding, context is for session based data, such as current user, etc, while rootValue is for "everything else."

I disagree with this assessment - I think rootValue should almost never be used, and if it's used then only for actual data, not configuration. All configuration-type things should like on context IMO.

@nodkz
Copy link
Contributor Author

nodkz commented Oct 6, 2016

I don't mind. All can be placed to context. Without rootValue examples more simple, but hide additional possibility to pass data to resolver.

Suggest to wait for the opinion of others.

@stubailo
Copy link
Contributor

stubailo commented Oct 6, 2016

For now I'll merge this, since the conversation can be had asynchronously.

@stubailo stubailo merged commit 35dd486 into graphql:source Oct 6, 2016
Dipeshwagle pushed a commit to Dipeshwagle/graphql.github.io that referenced this pull request Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants