-
Notifications
You must be signed in to change notification settings - Fork 738
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
Update Usage docs to use Result
#669
Update Usage docs to use Result
#669
Conversation
Also updates link to GraphQL Error spec
I put it everywhere else, might as well make it consistent.
@giladronat: 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/ |
|
||
In addition to an optional `data` property, `result` contains an optional `errors` array with GraphQL errors (for more on this, see the sections on [error handling](https://facebook.github.io/graphql/#sec-Error-handling) and the [response format](https://facebook.github.io/graphql/#sec-Response-Format) in the GraphQL specification). | ||
In addition to an optional `data` property, `success(Success)` result case contains an optional `errors` array with GraphQL errors (for more on this, see the sections on [response format errors](https://graphql.github.io/graphql-spec/June2018/#sec-Errors) in the GraphQL specification). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link may need to be updated in the future, but the past one redirects to the spec home page which is less useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌠Excellent change - thanks for changing this!
@giladronat Can you please sign the CLA so I can merge this? |
Done! Was confused by what's Meteor so didn't know if it was necessary. |
Meteor Development Group is our...I guess parent company? Corporate legal entity? The thing on work documents I have to sign. Anyway yep, that's us. Thank you! |
I set up Apollo for the first time, got hit by the new
Result
#644 Generic Parameter "Query" could not be inferred, and went to check the Usage docs to see what I was doing wrong. Later found out the cause, and decided to update the docs to reflect the change. I changed everything that previously used the(result, error)
to the new Result.I used
guard let data = try? result.get().data else { return }
liberally as the easiest happy path just-give-me-data in most places, while including an example on handling actual errors of bothGraphQLResult
andResult.failure
. Feel free to change it to any other way to get the data that may be more readable -- I'm still new toResult
.