-
Notifications
You must be signed in to change notification settings - Fork 2k
Backport v16 commits #4388
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
Backport v16 commits #4388
Conversation
This is popular on google as an index page
This replaces our expensive method that changes the underlying V8 shape multiple times with a loop that preserves the identity as much as possible. ``` ⏱ Print kitchen sink document 1 tests completed. 2 tests completed. HEAD x 9,290 ops/sec ±0.21% x 1.51 KB/op (24 runs sampled) BASE x 2,645 ops/sec ±0.18% x 2.18 KB/op (11 runs sampled) ``` --------- Co-authored-by: Benjie <code@benjiegillam.com>
See graphql/graphql.github.io#1951 For someone following the tutorial they may well get as far as running the server (`node server.js`) and then attempt to visit their new API and get confused because they're met with an error such as `{"errors":[{"message":"Missing query"}]}`. This PR adds some joining text to make it clear this is the expected outcome, and they must read on to get the GraphiQL IDE set up so that they can write queries.
…4343) * removes extra parenthesis from getting started code snippet
Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>
See: [Archived repo](https://github.com/html5rocks/www.html5rocks.com/blob/master/content/tutorials/es6/promises/en/index.html#L6) redirected the links.
Issue The server throws an error because the root variable is not defined. The correct variable name should be rootValue, which was previously declared in the code. Fix Updated rootValue in the createHandler function to use the correct variable name. Changes Replaced root with rootValue in the GraphQL handler configuration.
…4366) This PR add `executionOptions` property to the `ExecutionArgs`. Currently only one option can be configured, as is the one I need, but I have built an object so it can easily be extended in the future. The property allows the configuration of the maximum number of errors (`maxErrors`) for coercion. This allows the current hardcoded limit (`50`) to be reduced to a small number to avoid possible DoS attacks. > Also, it updates the execution docs to reflect this new change. I think the documentation was outdated since functions were using positional arguments and now they only accept an object. No longer updates the docs.
This refactors the code-first examples to use inline-resolvers rather than the root-value to show a difference between SDL and code-first.
Follow up to #4378
adds anatomy of a resolver guide This is a part of the effort to expand GraphQL.js documentation
Adds a guide "Understanding GraphQL.js Errors" I added some additional resources, feel free to suggest others/more if needed Co-authored-by: Jovi De Croock <decroockjovi@gmail.com>
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.
Made a negligible suggestion to preserve a comment.
@@ -1441,7 +1441,7 @@ describe('Execute: Handles basic execution tasks', () => { | |||
errors: [ | |||
{ | |||
message: | |||
'Variable "$data" got invalid value { email: "", wrongArg: "wrong", wrongArg2: "wrong", wrongArg3: "wrong" }; Field "wrongArg" is not defined by type "User".', | |||
'Variable "$data" has invalid value: Expected value of type "User" not to include unknown field "wrongArg", found: { email: "", wrongArg: "wrong", wrongArg2: "wrong", wrongArg3: "wrong" }.', |
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 text fix should be integrated into the individual cherry-picked PR so that all tests pass on git bisect when we merge this PR with the rebase strategy.
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.
in particular looks like this commit could be integrated into the backporting of #4366
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.
No description provided.