Fix issue when using a type condition on an inaccessible type in @req… #1873
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
|
|
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. |
pcmanus
commented
May 20, 2022
pcmanus
commented
May 20, 2022
pcmanus
pushed a commit
to pcmanus/federation
that referenced
this pull request
May 20, 2022
…uire This is the fed1 version of the fed2 patch at apollographql#1873. Seen that PR/commit for more details.
pcmanus
force-pushed
the
1866-missing-data-requires
branch
from
e928e02
to
16787b6
Compare
|
As mentioned on that ticket, this PR also fixes #1873. |
pcmanus
force-pushed
the
1866-missing-data-requires
branch
from
983cd57
to
6a54f73
Compare
…uire The `@require(fields:)` argument can use some type conditions to require data specific to a subtype. And those condition can even be on an `@inaccessible` type since "inaccessibility is about the federated API schema, not about subgraphs. However, this wasn't properly working because when the execution code was extracting the "requirements" data to pass it to the subgraph having the `@require`, the (federated) API schema we used, and consequently when the code tried to get the definition for the type condition in the `@require`, it didn't find any definition (since the type in question is `@inaccessible` and so not in said API schema) and as a result the corresponding fields were not included in the subgraph query. This commit fixes this issue, ensuring we pass the full supergraph (which _has_ the `@inaccessible` elements) to the execution code so that supergraph can be used when extracting required data. There was also another place within query planning (in `graphPath.ts` more precisely) where the code was looking up the possible implementation types, in the supergraph, of the required `@inaccessible` type, and was using the supergraph API to do so, which was incorrect for the same reason. This commit fix that problem as well, passing the full supergraph instead. Fixes apollographql#1866, apollographql#1863
pcmanus
force-pushed
the
1866-missing-data-requires
branch
from
6a54f73
to
b3f0d80
Compare
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.
…uire
The
@require(fields:)argument can use some type conditions to requiredata specific to a subtype. And those condition can even be on an
@inaccessibletype since "inaccessibility is about the federated APIschema, not about subgraphs.
However, this wasn't properly working because when the execution code
was extracting the "requirements" data to pass it to the subgraph having
the
@require, the (federated) API schema we used, and consequentlywhen the code tried to get the definition for the type condition in the
@require, it didn't find any definition (since the type in question is@inaccessibleand so not in said API schema) and as a result thecorresponding fields were not included in the subgraph query.
This commit fixes this issue, ensuring we pass the full supergraph
(which has the
@inaccessibleelements) to the execution code sothat supergraph can be used when extracting required data.
There was also another place within query planning (in
graphPath.tsmore precisely) where the code was looking up the possible
implementation types, in the supergraph, of the required
@inaccessibletype, and was using the supergraph API to do so, which was incorrect
for the same reason. This commit fix that problem as well, passing
the full supergraph instead.
Fixes #1866