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

Spec edits for incremental delivery, Section 3 & 7 only #1124

Open
wants to merge 4 commits into
base: incremental-integration
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cspell.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ ignoreRegExpList:
- /[a-z]{2,}'s/
words:
# Terms of art
- deprioritization
- endianness
- interoperation
- monospace
Expand Down
191 changes: 191 additions & 0 deletions spec/Appendix C -- Examples.md
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
}
```
2 changes: 2 additions & 0 deletions spec/GraphQL.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,5 @@ Note: This is an example of a non-normative note.
# [Appendix: Notation Conventions](Appendix%20A%20--%20Notation%20Conventions.md)

# [Appendix: Grammar Summary](Appendix%20B%20--%20Grammar%20Summary.md)

# [Appendix: Examples](Appendix%20C%20--%20Examples.md)
111 changes: 109 additions & 2 deletions spec/Section 3 -- Type System.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +797 to +799
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
that are skipped via `@skip` or `@include` directives or temporarily skipped via
`@defer`. This ordering is correctly produced when using the {CollectFields()}
algorithm.
that are skipped via `@skip` or `@include` directives or postponed via `@defer`
or `@stream`. This ordering is correctly produced when using the
{CollectFields()} algorithm.

Copy link
Contributor

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.)


Response serialization formats capable of representing ordered maps should
maintain this ordering. Serialization formats which can only represent unordered
Expand Down Expand Up @@ -1946,6 +1947,14 @@ GraphQL implementations that support the type system definition language must
provide the `@deprecated` directive if representing deprecated portions of the
schema.

GraphQL services 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 services that do not
support these directives must not make them available via introspection. The
[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.
Comment on lines +1950 to +1956
Copy link
Member

Choose a reason for hiding this comment

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

First sentence: let's rewrite this to use RFC2119 may/optional.

implemented -> provided: If you provide the @defer directive then it must adhere to these specifications - this is broader text than before (i.e. it doesn't matter or not whether you were attempting to implement the same functionality, if @defer exists then it must be this @defer).

Remove middle sentence: since if you provide a directive, it is made available in introspection (I think), and the only directives in introspection are the ones you provide (I think), this middle sentence is not required (I think).

Final sentence: this is a non-normative note to aid the reader.

Suggested change
GraphQL services 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 services that do not
support these directives must not make them available via introspection. The
[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.
An implementation may provide the `@defer` and/or `@stream` directives. If
either or both of these directives are provided, they must conform to the
requirements defined in this specification.
Note: The [Directives Are Defined](#sec-Directives-Are-Defined) validation rule
ensures that GraphQL Operations containing the `@defer` or `@stream` directives
cannot be executed by a GraphQL service that does not support them.


GraphQL implementations that support the type system definition language should
provide the `@specifiedBy` directive if representing custom scalar definitions.

Expand Down Expand Up @@ -2162,3 +2171,101 @@ to the relevant IETF specification.
```graphql example
scalar UUID @specifiedBy(url: "https://tools.ietf.org/html/rfc4122")
```

### @defer

```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`.
Comment on lines +2184 to +2189
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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`.
The `@defer` directive may be provided on a fragment spread or inline fragment
to indicate that execution of the related selection set should be deferred. When
a request includes the `@defer` directive, the response may consist of multiple
payloads: the initial payload containing all non-deferred data, while subsequent
payloads include deferred data.
The `@include` and `@skip` directives take precedence over `@defer`.


```graphql example
query myQuery($shouldDefer: Boolean) {
Copy link
Member

Choose a reason for hiding this comment

The 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
query myQuery($shouldDefer: Boolean) {
query myQuery($shouldDefer: Boolean! = true) {

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

nits: let's not mention "initial response" because for the case of @defer nested inside @stream. It's not always guaranteed to be in response #0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

- `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.
Comment on lines +2211 to +2216
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `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.
- `label: String` - An optional string literal (variables are disallowed) used
by GraphQL clients to identify data from responses and associate it with the
corresponding defer directive. If provided, the GraphQL service must include
this label in the corresponding pending object within the response. The
`label` argument must be unique across all `@defer` and `@stream` directives
in the document.


### @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.
Comment on lines +2228 to +2234
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried about using List this explicitly, since [Int!]! isn't a List type, it's a Non-Null type (NonNull(List(NonNull(Int)))). I couldn't find from a quick search spec text referring to this; my edit here is a little unwieldy so suggestions welcome!

Suggested change
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.
The `@stream` directive may be provided for a field whose type incorporates a
`List` type modifier; the directive enables the backend to leverage technology
such as asynchronous iterators to provide a partial list in the initial payload,
and additional list items in subsequent payloads.
The `@include` and `@skip` directives take precedence over `@stream`.
Note: The [Directives Are Defined](#sec-Directives-Are-Defined) validation rule
ensures that GraphQL Operations containing the `@stream` directive cannot be
executed by a GraphQL service that does not support this directive.


```graphql example
query myQuery($shouldStream: Boolean) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
query myQuery($shouldStream: Boolean) {
query myQuery($shouldStream: Boolean! = true) {

user {
friends(first: 10) {
nodes @stream(label: "friendsStream", initialCount: 5, if: $shouldStream)
Copy link
Member

Choose a reason for hiding this comment

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

Presumably this needs a selection set?

Suggested change
nodes @stream(label: "friendsStream", initialCount: 5, if: $shouldStream)
nodes @stream(label: "friendsStream", initialCount: 5, if: $shouldStream) {
name
}

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Love this section!!!

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 +2268 to +2271
Copy link
Member

Choose a reason for hiding this comment

The 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 initialCount: 2 I'd argue that at least 2 results should be supplied, or the entire thing should not be deferred (e.g. if there are fewer than 2 results). Skipping @defer / @include result in the client getting more data up front, but ignoring initialCount allows for the client to get less data up front, and that's a problem to my mind.

Loading
Loading