Fix issue with path in query plan's deferred nodes #2281
Merged
Conversation
This file contains 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
👷 Deploy request for apollo-federation-docs pending review.Visit the deploys page to approve it
|
pcmanus
force-pushed
the
defer-path-update
branch
from
67ad9cf
to
282aeb9
Compare
|
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. |
The `path` currently included in the query plan for deferred nodes is a "response" path, meaning that it's essentially a path in a json object. But for deferred node, the idea was that the path, alongside the `subselection` that the deferred node also include, could be used to rebuild the subset of the query corresponding to the deferred part it represents, but because `path` misses fragments, this is not always possible to do. This patch changes the `path` format to include fragments so it can be used to rebuild queries more directly (the new format also do not include `@` for lists: this gets in the way of rebuilding queries from the path more than it helps, and ever when using the `path` to walk on responses, it's easy enough to handle lists without needing this).
pcmanus
force-pushed
the
defer-path-update
branch
from
282aeb9
to
cc29ee4
Compare
clenfest
approved these changes
Nov 30, 2022
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.
The
pathcurrently included in the query plan for deferred nodes is a "response" path, meaning that it's essentially a path in a json object. But for deferred node, the idea was that the path, alongside thesubselectionthat the deferred node also include, could be used to rebuild the subset of the query corresponding to the deferred part it represents, but becausepathmisses fragments, this is not always possible to do.This patch changes the
pathformat to include fragments so it can be used to rebuild queries more directly (the new format also do not include@for lists: this gets in the way of rebuilding queries from the path more than it helps, and ever when using thepathto walk on responses, it's easy enough to handle lists without needing this).