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

Example for the rootValue field documentation was incorrect #3169

Merged
merged 2 commits into from
Aug 26, 2019
Merged

Example for the rootValue field documentation was incorrect #3169

merged 2 commits into from
Aug 26, 2019

Conversation

ianfabs
Copy link
Contributor

@ianfabs ianfabs commented Aug 17, 2019

Example code on the rootValue field was incorrect

The JavaScript example for the rootValue field was syntactically incorrect, the original example tried to return an object inside a closure (exactly how one would do so, if they were trying to return an object from an arrow function), like so:

const server = new ApolloServer({
  typeDefs,
  resolvers,
  rootValue: (documentAST) => ({
    const op = getOperationAST(documentNode)
    return op === 'mutation' ? mutationRoot : queryRoot;
  }),
});

when in-fact the intent - as apparent from the code inside the {} - was to have said code inside the body of the arrow function. The correct syntax would be as follows:

const server = new ApolloServer({
  typeDefs,
  resolvers,
  rootValue: (documentAST) => {
    const op = getOperationAST(documentNode)
    return op === 'mutation' ? mutationRoot : queryRoot;
  },
});

Removing the () around the {} solves the issue.

Thank you for considering my PR.

Example code on the rootValue field was incorrect
@apollo-cla
Copy link

@ianfabs: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@ianfabs
Copy link
Contributor Author

ianfabs commented Aug 17, 2019

Do not approve, this example has more problems. I will fix these when I have an opportunity. Thank you for your time and consideration

@abernix
Copy link
Member

abernix commented Aug 20, 2019

@ianfabs Regardless, thank you for attempting to improve it! We'll leave this open for a bit, but you're also welcome to close it and re-open a new one, as necessary. :)

@trevor-scheer
Copy link
Member

@ianfabs this looks like a good change to me, even if there are some other fixes to make in the vicinity. If you'd like to make further improvements in the future, maybe we can land this now and accept said improvements in a future PR? I'll check back in a few days and merge this unless I hear from you. Thanks again for the contribution!

@ianfabs
Copy link
Contributor Author

ianfabs commented Aug 26, 2019

I would be okay with this being accepted now, in the near future I will have time to expand upon this example further. Thank you for your time 😊

@trevor-scheer
Copy link
Member

No, thank you!

@trevor-scheer trevor-scheer merged commit 459e109 into apollographql:master Aug 26, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
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.

4 participants