-
Notifications
You must be signed in to change notification settings - Fork 254
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 handling of __typename optimizations #3156
Conversation
Fixes handling of a `__typename` selection during query planning process. When expanding fragments we were keeping references to the same `Field`s regardless where those fragments appeared in our original selection set. This was generally fine as in most cases we would have same inline fragment selection sets across whole operation but was causing problems when we were applying another optimization by collapsing those expanded inline fragments creating a new selection set. As a result, if any single field selection (within that fragment) would perform optimization around the usage of `__typename`, ALL occurrences of that field selection would get that optimization as well. See example below ```graphql foo { f1 { ... on Bar { __typename ...onBar # will be collapsed and sub selections will optimize __typename } } f2 { ...onBar # sub selections will get __typename optimization from above } } fragment onBar on Bar { b c } ```
🦋 Changeset detectedLatest commit: f168c7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 |
✅ Deploy Preview for apollo-federation-docs canceled.
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed up some changes in this commit to fix some things, other than that looks good to me.
To summarize:
- In the changes to
optimizeSiblingTypenames()
, a new variable namedfirstFieldIndex
was being set, that is expected to be the index of the first non-optimized-away field inupdatedSelections
(which is only populated if there are any selections to update, i.e. if a__typename
field is found, or a field/inline fragment's selection set was recursively updated). The code before was not settingfirstFieldIndex
ifupdatedSelections
was falsy, i.e. if no selections to update had been encountered by that point in the loop. I've updatedoptimizeSiblingTypenames()
to always setfirstFieldIndex
whenfirstFieldSelection
is set (regardless of whetherupdatedSelections
actually gets populated). - I've moved the
Field
-copying code insideFieldSelection.withAttachment()
into a newField.copy()
method that's next to the otherField.with*()
methods. The idea here is that if someone adds an optional argument toField
's constructor, it would be easier to not accidentally omit this argument copying if the methods that copy were insideField
(IMO we probably shouldn't be using optional arguments in those constructors to begin with, but that's not something to worry about dealing with now).- There's also a slight change here, in that we copy the attachments from the old field as well. This is for if we ever decide to add more attachments in the future, and shouldn't affect the case of the typename attachment (since we don't try to sibling-optimize the results of a sibling-optimized query).
- I've updated comments for the
attachement
->attachment
rename.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @apollo/[email protected] ### Patch Changes - Updated dependencies \[[`2192f355f50db33fe0807d16153f357696b9f190`](2192f35), [`e1e2605b30efc488b57f62ba43436606a38a3607`](e1e2605), [`5ac01b534318105e904c1e6598070f753add3bb1`](5ac01b5)]: - @apollo/[email protected] - @apollo/[email protected] ## @apollo/[email protected] ### Patch Changes - Updated dependencies \[[`2192f355f50db33fe0807d16153f357696b9f190`](2192f35), [`e1e2605b30efc488b57f62ba43436606a38a3607`](e1e2605), [`5ac01b534318105e904c1e6598070f753add3bb1`](5ac01b5)]: - @apollo/[email protected] - @apollo/[email protected] - @apollo/[email protected] ## @apollo/[email protected] ### Patch Changes - Fixes handling of a `__typename` selection during query planning process. ([#3156](#3156)) When expanding fragments we were keeping references to the same `Field`s regardless where those fragments appeared in our original selection set. This was generally fine as in most cases we would have same inline fragment selection sets across whole operation but was causing problems when we were applying another optimization by collapsing those expanded inline fragments creating a new selection set. As a result, if any single field selection (within that fragment) would perform optimization around the usage of `__typename`, ALL occurrences of that field selection would get that optimization as well. - Add validations for demand control directive applications ([#3148](#3148)) ## @apollo/[email protected] ### Patch Changes - Fixes issue where contextual parameters can have naming collisions if used in multiple subgraphs ([#3155](#3155)) - Updated dependencies \[[`2192f355f50db33fe0807d16153f357696b9f190`](2192f35), [`5ac01b534318105e904c1e6598070f753add3bb1`](5ac01b5)]: - @apollo/[email protected] ## @apollo/[email protected] ### Patch Changes - Fixes handling of a `__typename` selection during query planning process. ([#3156](#3156)) When expanding fragments we were keeping references to the same `Field`s regardless where those fragments appeared in our original selection set. This was generally fine as in most cases we would have same inline fragment selection sets across whole operation but was causing problems when we were applying another optimization by collapsing those expanded inline fragments creating a new selection set. As a result, if any single field selection (within that fragment) would perform optimization around the usage of `__typename`, ALL occurrences of that field selection would get that optimization as well. - Fixes issue where contextual parameters can have naming collisions if used in multiple subgraphs ([#3155](#3155)) - Updated dependencies \[[`2192f355f50db33fe0807d16153f357696b9f190`](2192f35), [`e1e2605b30efc488b57f62ba43436606a38a3607`](e1e2605), [`5ac01b534318105e904c1e6598070f753add3bb1`](5ac01b5)]: - @apollo/[email protected] - @apollo/[email protected] ## @apollo/[email protected] ### Patch Changes - Updated dependencies \[[`2192f355f50db33fe0807d16153f357696b9f190`](2192f35), [`5ac01b534318105e904c1e6598070f753add3bb1`](5ac01b5)]: - @apollo/[email protected] ## [email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Fixes handling of a
__typename
selection during query planning process.When expanding fragments we were keeping references to the same
Field
s regardless where those fragments appeared in our original selection set. This was generally fine as in most cases we would have same inline fragment selection sets across whole operation but was causing problems when we were applying another optimization by collapsing those expanded inline fragments creating a new selection set. As a result, if any single field selection (within that fragment) would perform optimization around the usage of__typename
, ALL occurrences of that field selection would get that optimization as well. See example below