Skip to content
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

Fix possible fragment-related assertion error during query planning. #2596

Merged
merged 1 commit into from
May 24, 2023

Conversation

pcmanus
Copy link
Contributor

@pcmanus pcmanus commented May 23, 2023

The assertion throw had message of the form Cannot add fragment of condition X (runtimes: ...) to parent type Y (runtimes: ...) and was due to not always properly maintaining the "parent type" information when fragment spreads expanding into some other spread.

Additionally, this commit fixes a small issue in the code computing the "diff" of what remains in a selection set after a fragment has be "reused", which could lead to inefficient (and weird looking) selections in fetches.

@pcmanus pcmanus requested a review from a team as a code owner May 23, 2023 13:35
@netlify
Copy link

netlify bot commented May 23, 2023

👷 Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 12a1d27

@changeset-bot
Copy link

changeset-bot bot commented May 23, 2023

🦋 Changeset detected

Latest commit: 12a1d27

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@apollo/query-planner Patch
@apollo/federation-internals Patch
@apollo/gateway Patch
@apollo/composition Patch
@apollo/query-graphs Patch
@apollo/subgraph Patch
apollo-federation-integration-testsuite Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pcmanus pcmanus self-assigned this May 23, 2023
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 23, 2023

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.

Copy link
Member

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments, lgtm!

query-planner-js/src/__tests__/buildPlan.test.ts Outdated Show resolved Hide resolved
The assertion throw had message of the form `Cannot add fragment
of condition X (runtimes: ...) to parent type Y (runtimes: ...)` and
was due to not always properly maintaining the "parent type" information
when fragment spreads expanding into some other spread.

Additionally, this commit fixes a small issue in the code computing
the "diff" of what remains in a selection set after a fragment has
be "reused", which could lead to inefficient (and weird looking)
selections in fetches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants