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

Experimental support for incremental delivery (@defer/@stream) #6827

Merged
merged 6 commits into from
Sep 23, 2022

Conversation

glasser
Copy link
Member

@glasser glasser commented Aug 18, 2022

Also provides improved compatibility with the graphql-over-http spec by
supporting content-type: application/graphql-response+json if requested via
accept header, and adds ; charset=utf-8 to content-types.

Executing @defer and @stream directives requires you to install pre-release
of graphql@17 in your server. This PR does not update package.json to
install that prerelease (nor does it even broaden peer deps, which would be
inadequate as many of its dependencies have peer deps that won't include v17).
However, it does add a new CI step that installs a particular v17 pre-release
and runs the test suite and smoke test (including running some
otherwise-disabled tests that exercise the execution of these directives). We
also add a new __testing_incrementalExecutionResults option that lets us test
transport-level behavior without installing the prerelease.

This change reworks GraphQLResponse and HTTPGraphQLResponse to allow
responses to be single- or multi-part. GraphQLResponse had previously (in v4)
moved most of its fields onto result; we now instead of body with two
kinds determining the structure of the rest of the response.
HTTPGraphQLResponse (new in v4) had tried to anticipate this change, but now
the structure is a bit different.

A few other changes were made for compatibility with graphql@17 such as
removing some uses of the non-options multi-argument GraphQLError constructor.

Add two new plugin APIs: didEncounterSubsequentErrors and
willSendSubsequentPayload.

Updates to plugins:

  • Usage reporting waits until all payloads are ready before it's done with
    a given operation.
  • The response cache does not cache incremental responses (although that would
    likely be quite helpful).
  • No cache-control HTTP headers are written with incremental responses (since we
    don't know all the fields that will be executed yet).
  • Inline traces are not added to incremental delivery responses (though it might
    make sense to add them to the last payload or something).

Fixes #6671.

@netlify
Copy link

netlify bot commented Aug 18, 2022

Deploy Preview for apollo-server-docs ready!

Name Link
🔨 Latest commit 93c14b9
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/632e3f476d67ed0008eb6e2f
😎 Deploy Preview https://deploy-preview-6827--apollo-server-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

// FIXME double check that this will always lead to chunked.
// FIXME error handling? is `then async` above legit?
for await (const chunk of httpGraphQLResponse.body.asyncIterator) {
res.write(chunk);
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe have some sort of flush here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is still a thing, if using compression.

@@ -23,9 +23,9 @@ export interface GraphQLRequest {
export type VariableValues = { [name: string]: any };

export interface GraphQLResponse {
// TODO(AS4): for incremental delivery, maybe we'll have an iterator here
// instead of a single result?
result: FormattedExecutionResult;
Copy link
Member Author

Choose a reason for hiding this comment

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

FormattedExecutionResult | FormattedFirstAsyncExecutionResult

result: FormattedExecutionResult;
// FIXME doc
subsequentResults: AsyncIterable<FormattedExecutionResult> | null;
Copy link
Member Author

Choose a reason for hiding this comment

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

AsyncIterable<FormattedSubsequentAsyncExecutionResult> | null

@glasser glasser force-pushed the glasser/experimental-incremental-delivery branch from 1cf6807 to 9f1e4ef Compare August 24, 2022 01:12
@glasser glasser force-pushed the glasser/experimental-incremental-delivery branch 2 times, most recently from 077cadc to bf5a3cc Compare September 22, 2022 20:22
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 22, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 93c14b9:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@glasser glasser force-pushed the glasser/experimental-incremental-delivery branch 2 times, most recently from 6d7ccf6 to 9c5b20a Compare September 23, 2022 16:31
@glasser glasser changed the title Checkpoint for experimental incremental delivery support Experimental support for incremental delivery (@defer/@stream) Sep 23, 2022
@glasser glasser force-pushed the glasser/experimental-incremental-delivery branch 5 times, most recently from d1f9878 to c1d052b Compare September 23, 2022 22:03
@glasser glasser marked this pull request as ready for review September 23, 2022 22:21
Also provides improved compatibility with the graphql-over-http spec by
supporting `content-type: application/graphql-response+json` if requested via
`accept` header, and adds `; charset=utf-8` to content-types.

Executing `@defer` and `@stream` directives requires you to install pre-release
of `graphql@17` in your server. This PR does not update `package.json` to
install that prerelease (nor does it even broaden peer deps, which would be
inadequate as many of its dependencies have peer deps that won't include v17).
However, it does add a new CI step that installs a particular v17 pre-release
and runs the test suite and smoke test (including running some
otherwise-disabled tests that exercise the execution of these directives). We
also add a new `__testing_incrementalExecutionResults` option that lets us test
transport-level behavior without installing the prerelease.

This change reworks `GraphQLResponse` and `HTTPGraphQLResponse` to allow
responses to be single- or multi-part. `GraphQLResponse` had previously (in v4)
moved most of its fields onto `result`; we now instead of `body` with two
`kind`s determining the structure of the rest of the response.
`HTTPGraphQLResponse` (new in v4) had tried to anticipate this change, but now
the structure is a bit different.

A few other changes were made for compatibility with `graphql@17` such as
removing some uses of the non-options multi-argument GraphQLError constructor.

Add two new plugin APIs: `didEncounterSubsequentErrors` and
`willSendSubsequentPayload`.

Updates to plugins:
- Usage reporting waits until all payloads are ready before it's done with
  a given operation.
- The response cache does not cache incremental responses (although that would
  likely be quite helpful).
- No cache-control HTTP headers are written with incremental responses (since we
  don't know all the fields that will be executed yet).
- Inline traces are not added to incremental delivery responses (though it might
  make sense to add them to the last payload or something).

Fixes #6671.
@glasser glasser force-pushed the glasser/experimental-incremental-delivery branch from c1d052b to f29eae4 Compare September 23, 2022 22:47
docs/source/integrations/building-integrations.md Outdated Show resolved Hide resolved
docs/source/migration.mdx Outdated Show resolved Hide resolved
docs/source/migration.mdx Outdated Show resolved Hide resolved
@@ -1127,7 +1130,7 @@ const server = new ApolloServer({
context: async ({ req }) => ({ name: req.headers.name }),
});

const { result } = await server.executeOperation({
const result = await server.executeOperation({
Copy link
Member

Choose a reason for hiding this comment

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

This was just incorrect before, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, GraphQLResponse in version-4 previously had a result field (new in 4).

docs/source/migration.mdx Outdated Show resolved Hide resolved
Comment on lines +666 to +668
if (!('singleResult' in response.body)) {
throw Error('expected single result');
}
Copy link
Member

Choose a reason for hiding this comment

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

Good use for assert like you demonstrated above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I did it in one place instead of calling the function, but the function seems a little easy to use than the same repeated two lines each time.

glasser and others added 5 commits September 23, 2022 16:05
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
@glasser glasser merged commit 0c2909a into version-4 Sep 23, 2022
@glasser glasser deleted the glasser/experimental-incremental-delivery branch September 23, 2022 23:42
glasser pushed a commit that referenced this pull request Sep 24, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to version-4, this PR
will be updated.

⚠️⚠️⚠️⚠️⚠️⚠️

`version-4` is currently in **pre mode** so this branch has prereleases
rather than normal releases. If you want to exit prereleases, run
`changeset pre exit` on `version-4`.

⚠️⚠️⚠️⚠️⚠️⚠️

# Releases
## @apollo/server-integration-testsuite@4.0.0-alpha.12

### Patch Changes

- [#6827](#6827)
[`0c2909aa1`](0c2909a)
Thanks [@glasser](https://github.com/glasser)! - Experimental support
for incremental delivery (`@defer`/`@stream`) when combined with a
prerelease of `graphql-js`.

- [#6850](#6850)
[`256f2424b`](256f242)
Thanks [@renovate](https://github.com/apps/renovate)! - Expand jest peer
deps to include v29

- [#6910](#6910)
[`6541f92c9`](6541f92)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Update
snapshot format to future jest v29 default

- [#6827](#6827)
[`0c2909aa1`](0c2909a)
Thanks [@glasser](https://github.com/glasser)! - Support
application/graphql-response+json content-type if requested via Accept
header, as per graphql-over-http spec.
    Include `charset=utf-8` in content-type headers.

- Updated dependencies
\[[`0c2909aa1`](0c2909a),
[`0c2909aa1`](0c2909a)]:
    -   @apollo/server@4.0.0-alpha.12

## @apollo/server-plugin-response-cache@4.0.0-alpha.5

### Patch Changes

- [#6827](#6827)
[`0c2909aa1`](0c2909a)
Thanks [@glasser](https://github.com/glasser)! - Experimental support
for incremental delivery (`@defer`/`@stream`) when combined with a
prerelease of `graphql-js`.

- Updated dependencies
\[[`0c2909aa1`](0c2909a),
[`0c2909aa1`](0c2909a)]:
    -   @apollo/server@4.0.0-alpha.12

## @apollo/server@4.0.0-alpha.12

### Patch Changes

- [#6827](#6827)
[`0c2909aa1`](0c2909a)
Thanks [@glasser](https://github.com/glasser)! - Experimental support
for incremental delivery (`@defer`/`@stream`) when combined with a
prerelease of `graphql-js`.

- [#6827](#6827)
[`0c2909aa1`](0c2909a)
Thanks [@glasser](https://github.com/glasser)! - Support
application/graphql-response+json content-type if requested via Accept
header, as per graphql-over-http spec.
    Include `charset=utf-8` in content-type headers.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Oct 10, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants