incremental: add new deferred fragments to the wrapped graphql result #4358
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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:
|
yaacovCR
changed the title
robrichard
approved these changes
Jul 17, 2025
|
I like this approach, i think this will translate well to the spec algorithms |
yaacovCR
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
alternative solution to #4357 in which we add the new deferred fragments to the wrapped graphql result
After an execution group "fails," i.e. a null has bubbled up too far, it fails the entire associated deferred fragment. But if the query includes a shared execution group between the failing deferred fragment and a non-failing deferred fragment with some unique subfields, those subfields will still be added to the graph for the failed deferred fragment, which is not present.
This throws an error, because the code assumes that in the situation above, i.e. a shared execution group between two fragments and some unique subfields, both of the deferred fragments must already be in the graph. So a runtime-error is triggered because some of the invariants we rely on when adding deferred fragments are violated.
But even if we didn't have an error, this would be an erroneous situation, as we should never add back a node to the graph that has been failed!
After this change, new deferred fragments are explicitly returned as part of the GraphQLWrappedResult and are only added at a single point when building the graph such that this error can be avoided entirely.