-
Notifications
You must be signed in to change notification settings - Fork 2k
Added support for new incremental delivery format #7736
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
Conversation
apollo-cla
commented
Sep 26, 2023
@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 graphql@17.0.0-alpha.3 and update tests as necessary.
To run the @defer tests locally, you'll need to:
- temporarily install
graphql@17.0.0-alpha.3at the top level dev dependencies (using--force) - run tests with
INCREMENTAL_DELIVERY_TESTS_ENABLED=t npm run test
OR
- (after updating the canary tag in
prepare.shtographql@17.0.0-alpha.3 - run
INCREMENTAL_DELIVERY_TESTS_ENABLED=t npm run test:smoke
Second option is less messy but the test output won't be as great.
Maxou44
commented
Nov 1, 2023
Any news about this PR?
u873838
commented
Aug 2, 2024
What are the blockers on getting this merged?
At this point we are leaning towards waiting until the new format is approved in the GraphQL Spec and/or GraphQL v17 is officially released to update for the new format.
AS5 will prevent you from combining newer graphql-js with its current incremental delivery format by only supporting incremental delivery with precisely v17.0.0-alpha.2. We can then add support for real v17 later.
We will want to have a deferSpec value for the newer version, and implement it in at least one client too.
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.