-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Spec edits for incremental delivery, Section 3 & 7 only #1124
base: incremental-integration
Are you sure you want to change the base?
Changes from 3 commits
abafb76
0310656
fcf898f
ffb8cad
c7c3ee1
2934a59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,191 @@ | ||
# C. Appendix: Examples | ||
|
||
## Incremental Delivery Examples | ||
|
||
### Example 1 - A query containing both defer and stream | ||
|
||
```graphql example | ||
query { | ||
person(id: "cGVvcGxlOjE=") { | ||
...HomeWorldFragment @defer(label: "homeWorldDefer") | ||
name | ||
films @stream(initialCount: 1, label: "filmsStream") { | ||
title | ||
} | ||
} | ||
} | ||
fragment HomeWorldFragment on Person { | ||
homeWorld { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
The response stream might look like: | ||
|
||
Payload 1, the initial response does not contain any deferred or streamed | ||
results in the `data` entry. The initial response contains a `hasNext` entry, | ||
indicating that subsequent payloads will be delivered. There are two Pending | ||
Responses indicating that results for both the `@defer` and `@stream` in the | ||
query will be delivered in the subsequent payloads. | ||
|
||
```json example | ||
{ | ||
"data": { | ||
"person": { | ||
"name": "Luke Skywalker", | ||
"films": [{ "title": "A New Hope" }] | ||
} | ||
}, | ||
"pending": [ | ||
{ "id": "0", "path": ["person"], "label": "homeWorldDefer" }, | ||
{ "id": "1", "path": ["person", "films"], "label": "filmsStream" } | ||
], | ||
"hasNext": true | ||
} | ||
``` | ||
|
||
Payload 2, contains the deferred data and the first streamed list item. There is | ||
one Completed Result, indicating that the deferred data has been completely | ||
delivered. | ||
|
||
```json example | ||
{ | ||
"incremental": [ | ||
{ | ||
"id": "0", | ||
"data": { "homeWorld": { "name": "Tatooine" } } | ||
}, | ||
{ | ||
"id": "1", | ||
"items": [{ "title": "The Empire Strikes Back" }] | ||
} | ||
], | ||
"completed": [ | ||
{"id": "0"} | ||
] | ||
"hasNext": true | ||
} | ||
``` | ||
|
||
Payload 3, contains the final stream payload. In this example, the underlying | ||
iterator does not close synchronously so {hasNext} is set to {true}. If this | ||
iterator did close synchronously, {hasNext} would be set to {false} and this | ||
would be the final response. | ||
|
||
```json example | ||
{ | ||
"incremental": [ | ||
{ | ||
"id": "1", | ||
"items": [{ "title": "Return of the Jedi" }] | ||
} | ||
], | ||
"hasNext": true | ||
} | ||
``` | ||
|
||
Payload 4, contains no incremental data. {hasNext} set to {false} indicates the | ||
end of the response stream. This response is sent when the underlying iterator | ||
of the `films` field closes. | ||
|
||
```json example | ||
{ | ||
"hasNext": false | ||
} | ||
``` | ||
|
||
### Example 2 - A query containing overlapping defers | ||
|
||
```graphql example | ||
query { | ||
person(id: "cGVvcGxlOjE=") { | ||
...HomeWorldFragment @defer(label: "homeWorldDefer") | ||
...NameAndHomeWorldFragment @defer(label: "nameAndWorld") | ||
firstName | ||
} | ||
} | ||
fragment HomeWorldFragment on Person { | ||
homeWorld { | ||
name | ||
terrain | ||
} | ||
} | ||
|
||
fragment NameAndHomeWorldFragment on Person { | ||
firstName | ||
lastName | ||
homeWorld { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
The response stream might look like: | ||
|
||
Payload 1, the initial response contains the results of the `firstName` field. | ||
Even though it is also present in the `HomeWorldFragment`, it must be returned | ||
in the initial payload because it is also defined outside of any fragments with | ||
the `@defer` directive. Additionally, There are two Pending Responses indicating | ||
that results for both `@defer`s in the query will be delivered in the subsequent | ||
payloads. | ||
|
||
```json example | ||
{ | ||
"data": { | ||
"person": { | ||
"firstName": "Luke" | ||
} | ||
}, | ||
"pending": [ | ||
{ "id": "0", "path": ["person"], "label": "homeWorldDefer" }, | ||
{ "id": "1", "path": ["person"], "label": "nameAndWorld" } | ||
], | ||
"hasNext": true | ||
} | ||
``` | ||
|
||
Payload 2, contains the deferred data from `HomeWorldFragment`. There is one | ||
Completed Result, indicating that `HomeWorldFragment` has been completely | ||
delivered. Because the `homeWorld` field is present in two separate `@defer`s, | ||
it is separated into its own Incremental Result. | ||
|
||
The second Incremental Result contains the data for the `terrain` field. This | ||
incremental result contains a `subPath` property to indicate to clients that the | ||
path of this result can be determined by concatenating the path from the Pending | ||
Result with id `"0"` and this `subPath` entry. | ||
|
||
```json example | ||
{ | ||
"incremental": [ | ||
{ | ||
"id": "0", | ||
"data": { "homeWorld": { "name": "Tatooine" } } | ||
}, | ||
{ | ||
"id": "0", | ||
"subPath": ["homeWorld"], | ||
"data": { "terrain": "desert" } | ||
} | ||
], | ||
"completed": [{ "id": "0" }], | ||
"hasNext": true | ||
} | ||
``` | ||
|
||
Payload 3, contains the remaining data from the `NameAndHomeWorldFragment`. | ||
`lastName` is the only remaining field that has not been delivered in a previous | ||
payload. | ||
|
||
```json example | ||
{ | ||
"incremental": [ | ||
{ | ||
"id": "1", | ||
"data": { "lastName": "Skywalker" } | ||
} | ||
], | ||
"completed": [{ "id": "1" }], | ||
"hasNext": false | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -794,8 +794,9 @@ And will yield the subset of each object type queried: | |||||||||||||||||||||||||||||||||||
When querying an Object, the resulting mapping of fields are conceptually | ||||||||||||||||||||||||||||||||||||
ordered in the same order in which they were encountered during execution, | ||||||||||||||||||||||||||||||||||||
excluding fragments for which the type does not apply and fields or fragments | ||||||||||||||||||||||||||||||||||||
that are skipped via `@skip` or `@include` directives. This ordering is | ||||||||||||||||||||||||||||||||||||
correctly produced when using the {CollectFields()} algorithm. | ||||||||||||||||||||||||||||||||||||
that are skipped via `@skip` or `@include` directives or temporarily skipped via | ||||||||||||||||||||||||||||||||||||
`@defer`. This ordering is correctly produced when using the {CollectFields()} | ||||||||||||||||||||||||||||||||||||
algorithm. | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
Response serialization formats capable of representing ordered maps should | ||||||||||||||||||||||||||||||||||||
maintain this ordering. Serialization formats which can only represent unordered | ||||||||||||||||||||||||||||||||||||
|
@@ -2162,3 +2163,109 @@ to the relevant IETF specification. | |||||||||||||||||||||||||||||||||||
```graphql example | ||||||||||||||||||||||||||||||||||||
scalar UUID @specifiedBy(url: "https://tools.ietf.org/html/rfc4122") | ||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
### @defer | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
GraphQL implementations are not required to implement the `@defer` and `@stream` | ||||||||||||||||||||||||||||||||||||
directives. If either or both of these directives are implemented, they must be | ||||||||||||||||||||||||||||||||||||
implemented according to this specification. GraphQL implementations that do not | ||||||||||||||||||||||||||||||||||||
support these directives must not make them available via introspection. The | ||||||||||||||||||||||||||||||||||||
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. Can we clarify what is meant by implementation here? My current understanding is that an "implementation" is typically a library, such as If that's the case, the "not required" part could be confusing. If I'm writing the 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. @martinbonnin thanks for reviewing. I updated this to say 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. Thanks for the clarification 🙏 Would it make sense to use language similar to
This is the closest to feature discovery we have in GraphQL and using common language pattern would make the spec more palatable. Or is there something fundamentally different about 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. I'm not sure this applies to I think a GraphQL server could not support the "type system definition language" but still support query {
__schema {
directives {
name
}
}
} I think the inverse could also be true. A GraphQL Server that supports the type system definition language may have no desire to support It's possible I'm not thinking about this correctly, let me know what you think. 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. Apologies in advance, a bunch of nitpicking on the vocabulary/glossary is coming below.
I don't think it makes sense for a GraphQL server to support the "type system definition language". A server reads request in and writes responses out. SDL is an implementation detail for the server. But maybe not for its implementation (==lib?). A lib takes SDL in and generates a compliant service out of it. I always thought of Regardless, if we use this service vs implementation distinction, I would rephrase like so:
I think this is the same for
That's why I'm tempted to put them in the same bucket of features even if 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. @martinbonnin I am definitely open to changing this, just want to talk it through to make sure I fully understand. Is this statement true for
Since The same is not true for
Again, I could be wrong about this but my understand for For 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.
Gotcha, thanks! So we've got:
Did I get that right? If yes, I was missing the 2. vs 3. distinction, thanks for pointing that out 🙏 I'm still not 100% sold on it though. I kind of like that other directives all use the same sentence and feels a bit weird that What about using RFC 2119
I don't want to bikeshed this too much especially if it was discussed already. Is there any reading material and/or pointers I could catch up on?
It can't but it's fine? My reading of Supporting the "type system definition language" is a feature of some GraphQL libs that has nothing to do with how the grammar is internally implemented. I read all of these sentences as: "if a lib reads a SDL written by a user, then the lib must/should/may provide the built in directive definitions":
The way I see it the directive location (executable or type system) is irrelevant. These sentences are about the directive definitions that must/should/may be added to the resulting schema. But again this is my very personal understanding of it and maybe PTSD from the full schemas discussion. Thanks for diving into this with me! 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. @martinbonnin I agree that we should probably rewrite this to use RFC 2119 I still don't think we should include I think a GraphQL server that creates everything programmatically fits the definition of an implementation that does not support the type system definition. My interpretation is that the spec is not providing any sort of guidance for the But for 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.
I see it now! Thanks for explaining 🙏
Agreed here as well 👍 But I also think this may be a problem? Do we have somewhere in the spec where we say what built-in directives are required in a valid full/final schema and which ones are not? I initially thought this was the place but since it excludes code-first implementations it can't reasonnably be it?
Right 👍 I actually like the sentence as it is now because it applies to services. I think using Finally, I'd say there should be a similar sentence for other directives? 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.
For
I do think this is the correct place, and with the above sentence for
Perhaps! Maybe it's worth a WG discussion to clarify? |
||||||||||||||||||||||||||||||||||||
[Directives Are Defined](#sec-Directives-Are-Defined) validation rule will | ||||||||||||||||||||||||||||||||||||
prevent GraphQL Operations containing the `@defer` or `@stream` directive from | ||||||||||||||||||||||||||||||||||||
being executed by a GraphQL service that does not implement these directives. | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
```graphql | ||||||||||||||||||||||||||||||||||||
directive @defer( | ||||||||||||||||||||||||||||||||||||
label: String | ||||||||||||||||||||||||||||||||||||
if: Boolean! = true | ||||||||||||||||||||||||||||||||||||
) on FRAGMENT_SPREAD | INLINE_FRAGMENT | ||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
The `@defer` directive may be provided for fragment spreads and inline fragments | ||||||||||||||||||||||||||||||||||||
to inform the executor to delay the execution of the current fragment to | ||||||||||||||||||||||||||||||||||||
indicate deprioritization of the current fragment. A query with `@defer` | ||||||||||||||||||||||||||||||||||||
directive will cause the request to potentially return multiple responses, where | ||||||||||||||||||||||||||||||||||||
deferred data is delivered in subsequent responses. `@include` and `@skip` take | ||||||||||||||||||||||||||||||||||||
precedence over `@defer`. | ||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
```graphql example | ||||||||||||||||||||||||||||||||||||
query myQuery($shouldDefer: Boolean) { | ||||||||||||||||||||||||||||||||||||
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. This falls foul of the CoerceArgumentValues() bug that I've proposed we fix previously. Also, since this variable is nullable you'd expect to be able to set it to null, but doing so would be an error. Let's give it a default here so as to not introduce more problematic examples into the spec.
Suggested change
|
||||||||||||||||||||||||||||||||||||
user { | ||||||||||||||||||||||||||||||||||||
name | ||||||||||||||||||||||||||||||||||||
...someFragment @defer(label: "someLabel", if: $shouldDefer) | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
fragment someFragment on User { | ||||||||||||||||||||||||||||||||||||
id | ||||||||||||||||||||||||||||||||||||
profile_picture { | ||||||||||||||||||||||||||||||||||||
uri | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#### @defer Arguments | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
- `if: Boolean! = true` - When `true`, fragment _should_ be deferred (see | ||||||||||||||||||||||||||||||||||||
related note below). When `false`, fragment will not be deferred and data will | ||||||||||||||||||||||||||||||||||||
be included in the initial response. Defaults to `true` when omitted. | ||||||||||||||||||||||||||||||||||||
robrichard marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
- `label: String` - May be used by GraphQL clients to identify the data from | ||||||||||||||||||||||||||||||||||||
responses and associate it with the corresponding defer directive. If | ||||||||||||||||||||||||||||||||||||
provided, the GraphQL service must add it to the corresponding pending object | ||||||||||||||||||||||||||||||||||||
in the response. `label` must be unique label across all `@defer` and | ||||||||||||||||||||||||||||||||||||
`@stream` directives in a document. `label` must not be provided as a | ||||||||||||||||||||||||||||||||||||
variable. | ||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
### @stream | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
```graphql | ||||||||||||||||||||||||||||||||||||
directive @stream( | ||||||||||||||||||||||||||||||||||||
label: String | ||||||||||||||||||||||||||||||||||||
if: Boolean! = true | ||||||||||||||||||||||||||||||||||||
initialCount: Int = 0 | ||||||||||||||||||||||||||||||||||||
) on FIELD | ||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
The `@stream` directive may be provided for a field of `List` type so that the | ||||||||||||||||||||||||||||||||||||
backend can leverage technology such as asynchronous iterators to provide a | ||||||||||||||||||||||||||||||||||||
partial list in the initial response, and additional list items in subsequent | ||||||||||||||||||||||||||||||||||||
responses. `@include` and `@skip` take precedence over `@stream`. The | ||||||||||||||||||||||||||||||||||||
[Stream Directives Are Used On List Fields](#sec-Stream-Directives-Are-Used-On-List-Fields) | ||||||||||||||||||||||||||||||||||||
validation rule is used to prevent the `@stream` directive from being applied to | ||||||||||||||||||||||||||||||||||||
a field that is not a `List` type. | ||||||||||||||||||||||||||||||||||||
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. I'm worried about using
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
```graphql example | ||||||||||||||||||||||||||||||||||||
query myQuery($shouldStream: Boolean) { | ||||||||||||||||||||||||||||||||||||
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
|
||||||||||||||||||||||||||||||||||||
user { | ||||||||||||||||||||||||||||||||||||
friends(first: 10) { | ||||||||||||||||||||||||||||||||||||
nodes @stream(label: "friendsStream", initialCount: 5, if: $shouldStream) | ||||||||||||||||||||||||||||||||||||
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. Presumably this needs a selection set?
Suggested change
Also should this example come with a schema? |
||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#### @stream Arguments | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
- `if: Boolean! = true` - When `true`, field _should_ be streamed (see related | ||||||||||||||||||||||||||||||||||||
note below). When `false`, the field will not be streamed and all list items | ||||||||||||||||||||||||||||||||||||
will be included in the initial response. Defaults to `true` when omitted. | ||||||||||||||||||||||||||||||||||||
- `label: String` - May be used by GraphQL clients to identify the data from | ||||||||||||||||||||||||||||||||||||
responses and associate it with the corresponding stream directive. If | ||||||||||||||||||||||||||||||||||||
provided, the GraphQL service must add it to the corresponding pending object | ||||||||||||||||||||||||||||||||||||
in the response. `label` must be unique label across all `@defer` and | ||||||||||||||||||||||||||||||||||||
`@stream` directives in a document. `label` must not be provided as a | ||||||||||||||||||||||||||||||||||||
variable. | ||||||||||||||||||||||||||||||||||||
- `initialCount: Int` - The number of list items the service should return as | ||||||||||||||||||||||||||||||||||||
part of the initial response. If omitted, defaults to `0`. A field error will | ||||||||||||||||||||||||||||||||||||
be raised if the value of this argument is less than `0`. | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
Note: The ability to defer and/or stream parts of a response can have a | ||||||||||||||||||||||||||||||||||||
robrichard marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
potentially significant impact on application performance. Developers generally | ||||||||||||||||||||||||||||||||||||
need clear, predictable control over their application's performance. It is | ||||||||||||||||||||||||||||||||||||
highly recommended that GraphQL services honor the `@defer` and `@stream` | ||||||||||||||||||||||||||||||||||||
directives on each execution. However, the specification allows advanced use | ||||||||||||||||||||||||||||||||||||
cases where the service can determine that it is more performant to not defer | ||||||||||||||||||||||||||||||||||||
and/or stream. Therefore, GraphQL clients _must_ be able to process a response | ||||||||||||||||||||||||||||||||||||
that ignores the `@defer` and/or `@stream` directives. This also applies to the | ||||||||||||||||||||||||||||||||||||
`initialCount` argument on the `@stream` directive. Clients _must_ be able to | ||||||||||||||||||||||||||||||||||||
process a streamed response that contains a different number of initial list | ||||||||||||||||||||||||||||||||||||
items than what was specified in the `initialCount` argument. | ||||||||||||||||||||||||||||||||||||
Comment on lines
+2275
to
+2278
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. Interesting, I didn't realise we had applied this to initialCount - I thought it was either non-deferred (i.e. give you the whole list) or deferred with initialCount supplied. I think we should revisit this discussion, it's quite different to the general "don't defer" and "don't stream" optimizations in my mind - specifically if you specify |
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.
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.
Small tweak: in this context, I think only defer is relevant as a cause for fields being postponed. (Stream may cause subfields to be postponed, but there would always be an empty list for the initial field.)