Spec edits for incremental delivery, Execution: CollectFields#1234
Open
robrichard wants to merge 1 commit into
Open
Spec edits for incremental delivery, Execution: CollectFields#1234robrichard wants to merge 1 commit into
robrichard wants to merge 1 commit into
Conversation
robrichard
force-pushed
the
robrichard/incremental-integration-collectfields
branch
3 times, most recently
from
July 15, 2026 19:29
776bcf4 to
04ec588
Compare
Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>
robrichard
force-pushed
the
robrichard/incremental-integration-collectfields
branch
from
July 16, 2026 17:41
04ec588 to
1df6a26
Compare
benjie
reviewed
Jul 16, 2026
Comment on lines
+447
to
+453
| :: A _defer usage_ is an unordered map representing the usage of a `@defer` | ||
| directive within a given operation. _Defer usage_ are "abstract" in that they | ||
| include information about the `@defer` directive from the AST of the GraphQL | ||
| document. A single _defer usage_ may be used to create many "concrete" Delivery | ||
| Groups when a `@defer` is included within a list type. | ||
|
|
||
| A _defer usage_ contains the following information: |
Member
There was a problem hiding this comment.
Suggested change
| :: A _defer usage_ is an unordered map representing the usage of a `@defer` | |
| directive within a given operation. _Defer usage_ are "abstract" in that they | |
| include information about the `@defer` directive from the AST of the GraphQL | |
| document. A single _defer usage_ may be used to create many "concrete" Delivery | |
| Groups when a `@defer` is included within a list type. | |
| A _defer usage_ contains the following information: | |
| :: A _defer usage_ represents the usage of a `@defer` directive during field | |
| collection; it contains the following information: |
Contributor
Author
There was a problem hiding this comment.
Update algorithm references to defer usage to require a map datastructure
- Let {fragmentDeferUsage} be an unordered map containing {label} and
{parentDeferUsage}.
+ Let {fragmentDeferUsage} be {label} and {parentDeferUsage}.
benjie
reviewed
Jul 16, 2026
| The {parentDeferUsage} entry is used to build distinct Execution Groups as | ||
| discussed within the Execution Plan Generation section below. | ||
|
|
||
| :: A _field detail_ is an unordered maps containing the following entries: |
Member
There was a problem hiding this comment.
Needs a more descriptive definition
benjie
reviewed
Jul 16, 2026
Comment on lines
+466
to
+467
| - {deferUsage}: the _defer usage_ enclosing the selection, if any, otherwise | ||
| {undefined}. |
Member
There was a problem hiding this comment.
Suggested change
| - {deferUsage}: the _defer usage_ enclosing the selection, if any, otherwise | |
| {undefined}. | |
| - {deferUsage}: the _defer usage_ enclosing the selection, if any. |
benjie
reviewed
Jul 16, 2026
Comment on lines
+472
to
+477
| The {CollectFields()} algorithm returns: | ||
|
|
||
| - {collectedFieldsMap}: the _collected fields map_ for the fields in the | ||
| selection set. | ||
| - {newDeferUsages}: a list of new _defer usage_ encountered during this field | ||
| collection. |
Member
There was a problem hiding this comment.
Suggested change
| The {CollectFields()} algorithm returns: | |
| - {collectedFieldsMap}: the _collected fields map_ for the fields in the | |
| selection set. | |
| - {newDeferUsages}: a list of new _defer usage_ encountered during this field | |
| collection. |
robrichard
commented
Jul 16, 2026
Comment on lines
+442
to
+443
| CollectFields(objectType, selectionSet, variableValues, deferUsage, | ||
| visitedFragments): |
Contributor
Author
There was a problem hiding this comment.
move to directly above the algorithm definition
robrichard
commented
Jul 16, 2026
|
|
||
| Note: When completing a List field, the {CollectFields} algorithm is invoked | ||
| with the same arguments for each element of the list. GraphQL Services may | ||
| choose to memoize their implementations of {CollectFields}. |
Contributor
Author
There was a problem hiding this comment.
Make this a stronger recommendation and explain why
benjie
reviewed
Jul 16, 2026
| {false}: | ||
| - Let {deferDirective} be that directive. | ||
| - If this execution is for a subscription operation, raise an _execution | ||
| error_. |
Member
There was a problem hiding this comment.
Suggested change
| error_. | |
| error_. | |
| - Otherwise let {deferDirective} be {null}. |
robrichard
commented
Jul 16, 2026
Comment on lines
+514
to
+515
| - If {deferDirective} is not defined: | ||
| - Add {fragmentSpreadName} to {visitedFragments}. |
Contributor
Author
There was a problem hiding this comment.
needs to be updated to mirror what's in graphql-js https://github.com/graphql/graphql-js/blob/17.x.x/src/execution/collectFields.ts#L294-L314
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Extracted from the full PR (#1110) and targeting an integration branch to aid in review.
Helpful reference material: