reintroduce incremental delivery support for subscriptions #4314
Conversation
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
yaacovCR
added
the
spec RFC
|
Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋 Supported commandsPlease post this commands in separate comments and only one per comment:
|
There was a problem hiding this comment.
I personally don't feel like we should support incremental delivery in subscriptions. They are already push based so whether all fields take 10ms to calculate or 500ms shouldn't make that much of a difference imho.
This is one of those that I wouldn't feel strongly about leaving out in v17, stronger I would encourage us to leave it out. We should assess whether this is widely used before rolling back to this state. Mind linking to the incremental delivery WG or GraphQL WG that touched on subscriptions incremental delivery specifically?
If this is from the last GraphQL WG, I would reduce the scope of this backtracking work to prior payloads but not subscriptions.
|
The raised concern at the WG was with regard to the response format, not subscription incremental delivery support. This is just another change. The reason why subscription support was removed, as far as I recall, was due to expected changes regarding multiplexing, i.e. ability to send different incremental responses in parallel, link to that discussion: graphql/defer-stream-wg#54 |
to preserve backwards compatibility with v16 alphas/experimental branches entrypoint renamed to legacyExperimentalSubscribeIncrementally Co-authored-by: Rob Richard <[email protected]> Co-authored-by: David Glasser <[email protected]>
|
Yes, I would def not add this back, it looks like a footgun and it just broadens the API surface even more. |
yaacovCR
force-pushed
the
experimental-subscribe
branch
from
8e7c261 to
d1500ec
Compare
|
I'm not super devoted to this change, but I assumed that structurally it raises the same issues as the response format changes... I would be happy to not need to merge it. It's handy to have around to know that everything still works (with the tweaks I included). I'll change it to draft for now! |
depends on #4313
Motivation: based on discussion at most recent WG, a concern for v17 migration is that there are breaking changes to incremental delivery support from prior alphas/experimental branches.
This PR reduces one of those changes, i.e. restores an entry point for allowing incremental delivery support with subscriptions.