-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Added support for new incremental delivery format #7736
base: main
Are you sure you want to change the base?
Added support for new incremental delivery format #7736
Conversation
@nathan-knight: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
👷 Deploy request for apollo-server-docs pending review.Visit the deploys page to approve it
|
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 31f0747:
|
@nathan-knight thanks for putting this together. The support for this in Apollo Server is still considered experimental / subject to change for exactly this reason, so I would like the smoke and integration tests to be updated to use the new alpha as well. I didn't look too closely, but I'd also like for the "polyfill" stuff to match the latest alpha rather than attempt to be compatible with both. i.e. if there's anything in those types we should remove that are no longer relevant let's do that (and maybe you already did, I haven't verified against the graphql-js types or looked too closely at the PR). |
@trevor-scheer I didn't remove the old logic but if removing support for 17.0.0-alpha.1 and 17.0.0-alpha.2 is an option then I can try my hand at updating the tests and removing the outdated parts |
@nathan-knight yeah I think that's ideal. Right now we reference an old canary build in various places, we should replace all references to those with To run the
OR
Second option is less messy but the test output won't be as great. |
Any news about this PR? |
What are the blockers on getting this merged? |
The format for incremental delivery has been updated: graphql/defer-stream-wg#69
I have added the new fields and ensured they get transmitted if they are present. It should still work with the previous format.
It is released in graphql@17.0.0-alpha.3, but I didn't want to disturb the existing integration test that uses it so I haven't added anything for it. In order to maintain the two in parallel the CI test would need to be duplicated to run with this version.