Skip to content

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

Closed
wants to merge 21 commits into from
Closed

Backport v16 commits #4388

wants to merge 21 commits into from

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented May 4, 2025

No description provided.

JoviDeCroock and others added 20 commits May 4, 2025 10:22
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>
Co-authored-by: Jovi De Croock <decroockjovi@gmail.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>
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.
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>
@JoviDeCroock JoviDeCroock requested a review from a team as a code owner May 4, 2025 08:33
@JoviDeCroock JoviDeCroock changed the title Add redirect for /api (#4327) Backport v16 commits May 4, 2025
Copy link
Contributor

@yaacovCR yaacovCR left a 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" }.',
Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i set up a PR with what I mean in #4413 => i also port the changes in coerceInputValue from #4367 to validateInputValue

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.

10 participants