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

Reapply #2639 (with fix for #2680) #2684

Merged
merged 7 commits into from
Jul 19, 2023
Merged

Conversation

trevor-scheer
Copy link
Member

@trevor-scheer trevor-scheer commented Jul 19, 2023

Yesterday we reverted #2639 due to a bug in the fragment optimization logic highlighted by #2680.

This PR reapplies #2639 with additional commits related to reproducing and resolving #2680. The initial commit can be ignored if the reviewer chooses to (it's previously been reviewed), but scrutiny is always welcome.

The bug existed in the SelectionSet.contains logic in the final check to provide a ContainsResult. Here the lengths of the compared selection sets are used to determine subset or strictly equal. In the case that the __typename field was ignored up above, the comparison becomes invalid unless we offset the comparison by 1 to account for the non-existent field.

@trevor-scheer trevor-scheer requested a review from a team as a code owner July 19, 2023 00:40
@changeset-bot
Copy link

changeset-bot bot commented Jul 19, 2023

🦋 Changeset detected

Latest commit: 2f67317

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

@netlify
Copy link

netlify bot commented Jul 19, 2023

Deploy Preview for apollo-federation-docs canceled.

Name Link
🔨 Latest commit d45bcd4
🔍 Latest deploy log https://app.netlify.com/sites/apollo-federation-docs/deploys/64b80a4f21e779000752caf0

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 19, 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.

@trevor-scheer trevor-scheer changed the base branch from main to version-2.4 July 19, 2023 16:17
@trevor-scheer trevor-scheer merged commit a740e07 into version-2.4 Jul 19, 2023
10 checks passed
@trevor-scheer trevor-scheer deleted the trevor/unrevert-2639 branch July 19, 2023 16:38
trevor-scheer added a commit that referenced this pull request Jul 19, 2023
This commit reapplies #2639 with an additional fix related to
reproducing and resolving #2680.

The bug existed in the SelectionSet.contains logic in the final
check to provide a ContainsResult. Here the lengths of the
compared selection sets are used to determine subset or
strictly equal. In the case that the __typename field was
ignored up above, the comparison becomes invalid unless we
offset the comparison by 1 to account for the non-existent field.
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.

None yet

2 participants