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

[pull] main from graphql:main #6

Open
wants to merge 273 commits into
base: main
Choose a base branch
from
Open

[pull] main from graphql:main #6

wants to merge 273 commits into from

Conversation

pull[bot]
Copy link

@pull pull bot commented Aug 16, 2022

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@github-actions
Copy link

Hi @pull[bot], I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@pull pull bot added the ⤵️ pull label Aug 16, 2022
@pull pull bot added the merge-conflict Resolve conflicts manually label Oct 3, 2022
yaacovCR and others added 30 commits October 18, 2024 04:29
[#3086 rebased on
main](#3086).

Depends on #3812 

@leebyron comments from original PR:

> Factors out input validation to reusable functions:
> 
> * Introduces `validateInputLiteral` by extracting this behavior from
`ValuesOfCorrectTypeRule`.
> * Introduces `validateInputValue` by extracting this behavior from
`coerceInputValue`
> * Simplifies `coerceInputValue` to return early on validation error
> * Unifies error reporting between `validateInputValue` and
`validateInputLiteral`, causing some error message strings to change,
but error data (eg locations) are preserved.
> 
> These two parallel functions will be used to validate default values
> 
> Potentially breaking if you rely on the existing behavior of
`coerceInputValue` to call a callback function, as the call signature
has changed. GraphQL behavior should not change, though error messages
are now slightly different.

Note: also breaking if you rely on the default callback function to
throw. Grossly similar behavior is available with
`validateInputValue()`.

Co-authored-by: Lee Byron <[email protected]>
Alternative to #2181

This adds a new `findSchemaChanges` export and deprecates the breaking
and dangerous alternative to be removed in v18. While adding
safe-schema-changes I noticed that we lack directive arg validation
here, we should probably add these to breaking/dangerous changes in the
future.
This adds support for aborting execution from the outside or resolvers,
this adds a few tests and tries to make the support as easy as possible.

Do we want to support having abort support on subscriptions, I guess it
makes sense for server-sent events.

I've chosen 2 places to place these interrupts

- `executeFieldsSerially` - every time we start a new mutation we check
whether the runtime has interrupted
- `executeFields` - every time we start executing a new field we check
whether the runtime has interrupted
- inside of the catch block as well so we return a singular error, all
though this doesn't really matter as the consumer would not receive
anything
  - this here should also take care of deferred fields

When comparing this to `graphql-tools/execute` I am not sure whether we
want to match this behavior, this throws a DomException which would be a
whole new exception that gets thrown while normally during execution we
wrap everything with GraphQLErrors.

Supersedes #3791
Resolves #3764

Co-authored-by: yaacovCR <[email protected]>
This way people can add this to metrics and inform themselves about sane
values to use with the `maxTokens` option on `parse`. This was added as
a non-enumerable property to increase backwards compatability.

Currently there is now way to know the tokens and by extension set sane
defaults.
incremental completion errors are indicated if and only if null bubbling
has caused a previously sent response to be null

we must instead just give a partial response with errors within the
incremental array

The GraphQL Tools implementation of this feature has different behavior
where the next call will throw if the operation is aborted, but we have
opted to give partial responses, and so we must adjust.
[#3049 rebased on
main](#3049).

This is the last rebased PR from the original PR stack concluding with
#3049.

* Rebased: #3809 [Original: #3092]
* Rebased: #3810 [Original: #3074]
* Rebased: #3811 [Original: #3077]
* Rebased: #3812 [Original: #3065]
* Rebased: #3813 [Original: #3086]
* Rebased: #3814 (this PR) [Original: #3049]

Update: #3044 and #3145 have been separated from this stack.

Changes from original PR:
1. `astFromValue()` is deprecated instead of being removed.

@leebyron comments from #3049, the original PR:
> Implements
[graphql/graphql-spec#793](graphql/graphql-spec#793)
> 
> * BREAKING: Changes default values from being represented as an
assumed-coerced "internal input value" to a pre-coerced "external input
value" (See chart below).
> This allows programmatically provided default values to be represented
in the same domain as values sent to the service via variable values,
and allows it to have well defined methods for both transforming into a
printed GraphQL literal string for introspection / schema printing (via
`valueToLiteral()`) or coercing into an "internal input value" for use
at runtime (via `coerceInputValue()`)
> To support this change in value type, this PR adds two related
behavioral changes:
>   
> * Adds coercion of default values from external to internal at runtime
(within `coerceInputValue()`)
> * Removes `astFromValue()`, replacing it with `valueToLiteral()` for
use in introspection and schema printing. `astFromValue()` performed
unsafe "uncoercion" to convert an "Internal input value" directly to a
"GraphQL Literal AST", where `valueToLiteral()` performs a well defined
transform from "External input value" to "GraphQL Literal AST".
> * Adds validation of default values during schema validation.
> Since assumed-coerced "internal" values may not pass "external"
validation (for example, Enum values), an additional validation error
has been included which attempts to detect this case and report a
strategy for resolution.
> 
> Here's a broad overview of the intended end state:
> 
> ![GraphQL Value
Flow](https://user-images.githubusercontent.com/50130/118379946-51ac5300-b593-11eb-839f-c483ecfbc875.png)

---------

Co-authored-by: Lee Byron <[email protected]>
this allows e.g. passing the signal to fetch

Note: the `abortSignal` is now the fifth argument to a GraphQLFieldResolverFn. If no resolver if provided, and the parent is an object with a key for the field name with a value that is a function, the `abortSignal` will be the fourth argument, as in the included test, with the `parent` accessible via the `this` keyword.
This gets rid of enums for two reasons

- TypeScript does not really compile them size efficiently, see
[here](https://www.runpkg.com/[email protected]/language/kinds.mjs)
  - Raw size kinds.js before: 2800bytes
  - Raw size kinds.js after: 2200 bytes
- It's not kind to plains strings as seen in
#3356

Resolves #3356

This does not intend to fix the tree-shaking issue as seen in
#4253, for that we'd need to
fully normalize these exports and sacrifice the DX of the namespaces. I
do think that it becomes easier as plain strings will be "easier"
compared to before so if you're not doing comparisons you can opt out of
the `Kind` namespace.

<details>
  <summary>See new output</summary>

```js
/**
 * The set of allowed kind values for AST nodes.
 */
exports.Kind = {
    /** Name */
    NAME: 'Name',
    /** Document */
    DOCUMENT: 'Document',
    OPERATION_DEFINITION: 'OperationDefinition',
    VARIABLE_DEFINITION: 'VariableDefinition',
    SELECTION_SET: 'SelectionSet',
    FIELD: 'Field',
    ARGUMENT: 'Argument',
    FRAGMENT_ARGUMENT: 'FragmentArgument',
    /** Fragments */
    FRAGMENT_SPREAD: 'FragmentSpread',
    INLINE_FRAGMENT: 'InlineFragment',
    FRAGMENT_DEFINITION: 'FragmentDefinition',
    /** Values */
    VARIABLE: 'Variable',
    INT: 'IntValue',
    FLOAT: 'FloatValue',
    STRING: 'StringValue',
    BOOLEAN: 'BooleanValue',
    NULL: 'NullValue',
    ENUM: 'EnumValue',
    LIST: 'ListValue',
    OBJECT: 'ObjectValue',
    OBJECT_FIELD: 'ObjectField',
    /** Directives */
    DIRECTIVE: 'Directive',
    /** Types */
    NAMED_TYPE: 'NamedType',
    LIST_TYPE: 'ListType',
    NON_NULL_TYPE: 'NonNullType',
    /** Type System Definitions */
    SCHEMA_DEFINITION: 'SchemaDefinition',
    OPERATION_TYPE_DEFINITION: 'OperationTypeDefinition',
    /** Type Definitions */
    SCALAR_TYPE_DEFINITION: 'ScalarTypeDefinition',
    OBJECT_TYPE_DEFINITION: 'ObjectTypeDefinition',
    FIELD_DEFINITION: 'FieldDefinition',
    INPUT_VALUE_DEFINITION: 'InputValueDefinition',
    INTERFACE_TYPE_DEFINITION: 'InterfaceTypeDefinition',
    UNION_TYPE_DEFINITION: 'UnionTypeDefinition',
    ENUM_TYPE_DEFINITION: 'EnumTypeDefinition',
    ENUM_VALUE_DEFINITION: 'EnumValueDefinition',
    INPUT_OBJECT_TYPE_DEFINITION: 'InputObjectTypeDefinition',
    /** Directive Definitions */
    DIRECTIVE_DEFINITION: 'DirectiveDefinition',
    /** Type System Extensions */
    SCHEMA_EXTENSION: 'SchemaExtension',
    /** Type Extensions */
    SCALAR_TYPE_EXTENSION: 'ScalarTypeExtension',
    OBJECT_TYPE_EXTENSION: 'ObjectTypeExtension',
    INTERFACE_TYPE_EXTENSION: 'InterfaceTypeExtension',
    UNION_TYPE_EXTENSION: 'UnionTypeExtension',
    ENUM_TYPE_EXTENSION: 'EnumTypeExtension',
    INPUT_OBJECT_TYPE_EXTENSION: 'InputObjectTypeExtension',
};
```

</details>

Currently seeing whether we should disable the redeclare lint-rule and
whether there is a better way to type our `nodeKinds`
…lvers (#4267)

Prior to this pull request, cancellation worked by checking the abort signal status during execution, and throwing the reason if the abort signal has been triggered. This fails if an asynchronous resolver hangs.

This pull request changes the cancellation method to wrap promises returned by resolvers so that they immediately reject on cancellation.
moves simplest case inside the execute[Root|Sub]ExecutionPlan()
functions to better match proposed spec edits

orders function in call order, moving executeSubExecutionPlan down
closer to where it is used

no significant change in performance


![image](https://github.com/user-attachments/assets/221a2696-fd0b-4b53-8519-eb2376327639)
This adds a new safe schema-change when a description changes of any
schema-member. Do note that there are some drive-by fixes in here as I
noticed a bug in safe-argument changes where we'd just always report the
changes rather than actually see whether something changed 😅

Resolves #4245
Resolves #4237

This mirrors the fields/input-object argument logic but specifically for
directives.
When the abort signal is triggered, any pending `.next()` calls should
return immediately
we must update the lexer line number and the line start position,
because lookahead saves the token within the linked list, and so will
never be called again on this token

we do not change the current token, however, until the lexer is advanced

closes #2764
- to separate between this repo and the spec
- suggest adding PRs with failing tests when reporting bugs
- add more flexibility to review/merge process
- suggest using discord to communicate
motivation:
1. can be used to extract common logic from extendSchemaImpl and lexicographicSortSchema
2. can be used further enhance extendSchemaImpl to take resolvers
3. can be exposed to provide a generic safe mapSchemaConfig
…QLEnumValue (#4288)

this extracts logic from #3044 and #3145 (later rebased as #3807 and
#3808) to implement more informative error messages without implementing
[the full schema coordinate
RFC](graphql/graphql-spec#794)

This is a BREAKING CHANGE because these schema elements are now longer
plain objects and function differently in various scenarios, for example
with `String(<schemaElement>` `JSON.stringifu(<schemaElement>` and
`.toString()` and `.toJSON()`

---------

Co-authored-by: Jovi De Croock <[email protected]>
I was just trying to understand the new flows for how scalar values are
represented/parsed/serialized, and noticed that the comments for a
couple of these methods looked backwards.

Really hoping I am not just confused about the terminology, and that
these aren't actually correct 😅
this way, if (when?) we re-enable incemental delivery support with subscriptions, the signature of collect fields will not need to change
… of `operation` (#4309)

this is the 16.x.x behavior, unlocked by #4308, further reducing the set of breaking changes to this utility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⤵️ pull merge-conflict Resolve conflicts manually
Projects
None yet
Development

Successfully merging this pull request may close these issues.