Skip to content

Fixes for incremenal delivery integration branch #1232

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.

Sign up for GitHub

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

Merged
robrichard merged 1 commit into from
Jun 15, 2026
+8 −12
Merged

Fixes for incremenal delivery integration branch #1232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions spec/Section 5 -- Validation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +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.
- {HasNoOverlappingStreams(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.
Expand Down Expand Up @@ -596,14 +596,10 @@ SameResponseShape(fieldA, fieldB):
- If {SameResponseShape(subfieldA, subfieldB)} is {false}, return {false}.
- Return {true}.

SameStreamDirective(fieldA, fieldB):
HasNoOverlappingStreams(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}.
Comment on lines -603 to -606

@benjie benjie Jun 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm... "distinct members" may be doing a lot here. For example:

frag A on Foo {
  ...B
  ...B
}
frag B on Foo {
  blah @stream
}

might merge since B.blah is the same field twice, assuming that's what we mean by "distinct members", but

frag A on Foo {
  ...B
  ...C
}
frag B on Foo {
  blah @stream
}
frag C on Foo {
  blah @stream
}

would not, because B.blah != C.blah even though it seems that they should have the same result as in the previous example.

This is probably okay for @stream because it shouldn't really be fragment-merged in the same way regular fields would be (we want @stream to be used reasonably scarcely?)... but it warrants acknowledgement. It's a bit of a break from the norm for GraphQL.

@robrichard robrichard Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes this is the behavior we want. See the discussion here: graphql/defer-stream-wg#100

It becomes an issue when the stream, even with the same arguments has different selection sets. One fragment can influence the performance characteristics of the stream on another fragment, which is why we decided to prohibit this.

@benjie benjie Jun 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm... but what about:

frag A on Foo {
  ...B
  ...C
}
frag B on Foo {
  ...D @include(if: $var1)
}
frag C on Foo {
  ...D @defer(if: $var2)
}
frag D on Foo {
  blah @stream
}

Here, is A>B>D.foo seen as the same as A>C>D.foo? Are they distinct members?
... does it matter? 🤔 I suppose not...

- Return {false}.

Note: In prior versions of the spec the term "composite" was used to signal a
Expand Down
12 changes: 6 additions & 6 deletions spec/Section 7 -- Response.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,10 @@ An _incremental pending notice_ must contain entries with the keys {"id"} and
{"path"}, and may contain an entry with key {"label"}.

The value of {"id"} must be a string. This {"id"} should be used by clients to
correlate incremental pending notices with _incremental result_ and _completed
result_. The {"id"} value must be unique across the entire _incremental stream_
response. There must not be any other incremental pending notice in the
_incremental stream_ with the same {"id"}.
correlate incremental pending notices with _incremental result_ and _incremental
completion notice_. The {"id"} value must be unique across the entire
_incremental stream_ response. There must not be any other incremental pending
notice in the _incremental stream_ with the same {"id"}.

The value of {"path"} must be a _response position_. When the incremental
pending notice is associated with a `@stream` directive, it indicates the list
Expand Down Expand Up @@ -574,8 +574,8 @@ and may contain an entry with the key {"errors"}.
The value of {"id"} must be a string referencing its _associated incremental
pending notice_. The associated incremental pending notice must appear either in
the _initial incremental stream result_, in a prior _incremental stream update
result_, or in the same _incremental stream update result_ as the _completed
result_ that references it.
result_, or in the same _incremental stream update result_ as the _incremental
completion notice_ that references it.

The value of {"errors"}, if present, informs clients that the delivery of the
data from the _associated incremental pending notice_ has failed, due to an
Expand Down
Loading