-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Spec edits for incremental delivery, Validation #1223
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
Open
robrichard
wants to merge
2
commits into
incremental-integration
Choose a base branch
from
incremental-integration-validation
base: incremental-integration
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+179
−0
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -560,6 +560,7 @@ FieldsInSetCanMerge(set): | |||||||||
| {set} including visiting fragments and inline fragments. | ||||||||||
| - Given each pair of distinct members {fieldA} and {fieldB} in {fieldsForName}: | ||||||||||
| - {SameResponseShape(fieldA, fieldB)} must be true. | ||||||||||
| - {SameStreamDirective(fieldA, fieldB)} must be true. | ||||||||||
| - If the parent types of {fieldA} and {fieldB} are equal or if either is not | ||||||||||
| an Object Type: | ||||||||||
| - {fieldA} and {fieldB} must have identical field names. | ||||||||||
|
|
@@ -595,6 +596,16 @@ SameResponseShape(fieldA, fieldB): | |||||||||
| - If {SameResponseShape(subfieldA, subfieldB)} is {false}, return {false}. | ||||||||||
| - Return {true}. | ||||||||||
|
|
||||||||||
| SameStreamDirective(fieldA, fieldB): | ||||||||||
|
|
||||||||||
| - If neither {fieldA} nor {fieldB} has a directive named `stream`. | ||||||||||
| - Return {true}. | ||||||||||
| - If both {fieldA} and {fieldB} have a directive named `stream`. | ||||||||||
| - Let {streamA} be the directive named `stream` on {fieldA}. | ||||||||||
| - Let {streamB} be the directive named `stream` on {fieldB}. | ||||||||||
| - If {streamA} and {streamB} have identical sets of arguments, return {true}. | ||||||||||
| - Return {false}. | ||||||||||
|
|
||||||||||
| Note: In prior versions of the spec the term "composite" was used to signal a | ||||||||||
| type that is either an Object, Interface or Union type. | ||||||||||
|
|
||||||||||
|
|
@@ -1695,6 +1706,174 @@ query ($foo: Boolean = true, $bar: Boolean = false) { | |||||||||
| } | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ### Defer And Stream Directives Are Used On Valid Root Field | ||||||||||
|
|
||||||||||
| ** Formal Specification ** | ||||||||||
|
|
||||||||||
| - For every {directive} in a document. | ||||||||||
| - Let {directiveName} be the name of {directive}. | ||||||||||
| - Let {mutationType} be the root Mutation type in {schema}. | ||||||||||
| - Let {subscriptionType} be the root Subscription type in {schema}. | ||||||||||
| - If {directiveName} is "defer" or "stream": | ||||||||||
| - The parent type of {directive} must not be {mutationType} or | ||||||||||
| {subscriptionType}. | ||||||||||
|
|
||||||||||
| **Explanatory Text** | ||||||||||
|
|
||||||||||
| The defer and stream directives are not allowed to be used on root fields of the | ||||||||||
| mutation or subscription type. | ||||||||||
|
Comment on lines
+1723
to
+1724
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| For example, the following document will not pass validation because `@defer` | ||||||||||
| has been used on a root mutation field: | ||||||||||
|
|
||||||||||
| ```raw graphql counter-example | ||||||||||
| mutation { | ||||||||||
| ... @defer { | ||||||||||
| mutationField | ||||||||||
| } | ||||||||||
| } | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ### Defer And Stream Directives Are Used On Valid Operations | ||||||||||
|
|
||||||||||
| ** Formal Specification ** | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| - Let {subscriptionFragments} be the empty set. | ||||||||||
| - For each {operation} in a document: | ||||||||||
| - If {operation} is a subscription operation: | ||||||||||
| - Let {fragments} be every fragment referenced by that {operation} | ||||||||||
| transitively. | ||||||||||
| - For each {fragment} in {fragments}: | ||||||||||
| - Let {fragmentName} be the name of {fragment}. | ||||||||||
| - Add {fragmentName} to {subscriptionFragments}. | ||||||||||
| - For every {directive} in a document: | ||||||||||
| - If {directiveName} is not "defer" or "stream": | ||||||||||
| - Continue to the next {directive}. | ||||||||||
| - Let {ancestor} be the ancestor operation or fragment definition of | ||||||||||
| {directive}. | ||||||||||
| - If {ancestor} is a fragment definition: | ||||||||||
| - If the fragment name of {ancestor} is not present in | ||||||||||
| {subscriptionFragments}: | ||||||||||
| - Continue to the next {directive}. | ||||||||||
| - If {ancestor} is not a subscription operation: | ||||||||||
| - Continue to the next {directive}. | ||||||||||
| - Let {if} be the argument named "if" on {directive}. | ||||||||||
| - {if} must be defined. | ||||||||||
| - Let {argumentValue} be the value passed to {if}. | ||||||||||
| - {argumentValue} must be a variable, or the boolean value "false". | ||||||||||
|
|
||||||||||
| **Explanatory Text** | ||||||||||
|
|
||||||||||
| The defer and stream directives can not be used to defer or stream data in | ||||||||||
| subscription operations. If these directives appear in a subscription operation | ||||||||||
| they must be disabled using the "if" argument. This rule will not permit any | ||||||||||
| defer or stream directives on a subscription operation that cannot be disabled | ||||||||||
| using the "if" argument. | ||||||||||
|
|
||||||||||
| For example, the following document will not pass validation because `@defer` | ||||||||||
| has been used in a subscription operation with no "if" argument defined: | ||||||||||
|
|
||||||||||
| ```raw graphql counter-example | ||||||||||
| subscription sub { | ||||||||||
| newMessage { | ||||||||||
| ... @defer { | ||||||||||
| body | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ### Defer And Stream Directive Labels Are Unique | ||||||||||
|
|
||||||||||
| ** Formal Specification ** | ||||||||||
|
|
||||||||||
| - Let {labelValues} be an empty set. | ||||||||||
| - For every {directive} in the document: | ||||||||||
| - Let {directiveName} be the name of {directive}. | ||||||||||
| - If {directiveName} is "defer" or "stream": | ||||||||||
| - For every {argument} in {directive}: | ||||||||||
| - Let {argumentName} be the name of {argument}. | ||||||||||
| - Let {argumentValue} be the value passed to {argument}. | ||||||||||
| - If {argumentName} is "label": | ||||||||||
| - {argumentValue} must not be a variable. | ||||||||||
| - {argumentValue} must not be present in {labelValues}. | ||||||||||
| - Add {argumentValue} to {labelValues}. | ||||||||||
|
|
||||||||||
| **Explanatory Text** | ||||||||||
|
|
||||||||||
| The `@defer` and `@stream` directives each accept an argument "label". This | ||||||||||
| label may be used by GraphQL clients to uniquely identify response payloads. If | ||||||||||
| a label is passed, it must not be a variable and it must be unique within all | ||||||||||
| other `@defer` and `@stream` directives in the document. | ||||||||||
|
|
||||||||||
| For example the following document is valid: | ||||||||||
|
|
||||||||||
| ```graphql example | ||||||||||
| { | ||||||||||
| dog { | ||||||||||
| ...fragmentOne | ||||||||||
| ...fragmentTwo @defer(label: "dogDefer") | ||||||||||
| } | ||||||||||
| pets @stream(label: "petStream") { | ||||||||||
| name | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fragment fragmentOne on Dog { | ||||||||||
| name | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fragment fragmentTwo on Dog { | ||||||||||
| owner { | ||||||||||
| name | ||||||||||
| } | ||||||||||
| } | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| For example, the following document will not pass validation because the same | ||||||||||
| label is used in different `@defer` and `@stream` directives.: | ||||||||||
|
|
||||||||||
| ```raw graphql counter-example | ||||||||||
| { | ||||||||||
| dog { | ||||||||||
| ...fragmentOne @defer(label: "MyLabel") | ||||||||||
| } | ||||||||||
| pets @stream(label: "MyLabel") { | ||||||||||
| name | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fragment fragmentOne on Dog { | ||||||||||
| name | ||||||||||
| } | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ### Stream Directives Are Used On List Fields | ||||||||||
|
|
||||||||||
| **Formal Specification** | ||||||||||
|
|
||||||||||
| - For every {directive} in a document. | ||||||||||
| - Let {directiveName} be the name of {directive}. | ||||||||||
| - If {directiveName} is "stream": | ||||||||||
| - Let {adjacent} be the AST node the directive affects. | ||||||||||
| - {adjacent} must be a List type. | ||||||||||
|
|
||||||||||
| **Explanatory Text** | ||||||||||
|
|
||||||||||
| GraphQL directive locations do not provide enough granularity to distinguish the | ||||||||||
| type of fields used in a GraphQL document. Since the stream directive is only | ||||||||||
| valid on list fields, an additional validation rule must be used to ensure it is | ||||||||||
| used correctly. | ||||||||||
|
|
||||||||||
| For example, the following document will only pass validation if `field` is | ||||||||||
| defined as a List type in the associated schema. | ||||||||||
|
|
||||||||||
| ```graphql counter-example | ||||||||||
| query { | ||||||||||
| field @stream(initialCount: 0) | ||||||||||
| } | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ## Variables | ||||||||||
|
|
||||||||||
| ### Variable Uniqueness | ||||||||||
|
|
||||||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Let's split this up into a
@streamand@deferrule:For defer it's on fragments so it's a little different. I'm not a big fan of this suggested edit, it's inspired by https://spec.graphql.org/draft/#sec-Fragment-Spread-Type-Existence but I feel like maybe we should branch on inline fragment vs named fragment spread.