Skip to content

fix(validation): catch OverlappingFieldsCanBeMergedRule violations with nested fragments #4168

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Sep 23, 2024
Merged

fix(validation): catch OverlappingFieldsCanBeMergedRule violations with nested fragments #4168

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1e2a419
Add test that elicits bug with comparedFragmentPairs usage
sachindshinde Aug 16, 2024
997923d
Revert non-test changes in https://github.com/graphql/graphql-js/pull…
sachindshinde Aug 16, 2024
e1fb65a
Modify collectConflictsBetweenFieldsAndFragment() to track seen fragm…
sachindshinde Aug 16, 2024
99f868a
Add test that elicits bug caused by not recursing into fields
sachindshinde Aug 16, 2024
a359a93
Revert previous changes for collectConflictsBetweenFieldsAndFragment(…
sachindshinde Aug 16, 2024
b2a71fc
Modify comparedFragmentPairs to allow a selection set to replace a fr…
sachindshinde Aug 16, 2024
2d830ca
Separate fields-fragment memoization from fragment-fragment memoization
sachindshinde Sep 8, 2024
4be3fb8
introduce OrderedPairSet class
yaacovCR Sep 9, 2024
dd7d0e6
remove custom comparator option
yaacovCR Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Revert non-test changes in #3442 , which introduced the bugged compar… 
…eFragmentPairs usage
  • Loading branch information
@sachindshinde @yaacovCR
sachindshinde authored and yaacovCR committed Sep 23, 2024
commit 997923d32cfb0069eeafd356f4503d000d122802
16 changes: 0 additions & 16 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,22 +267,6 @@ function collectConflictsBetweenFieldsAndFragment(
// (E) Then collect any conflicts between the provided collection of fields
// and any fragment names found in the given fragment.
for (const referencedFragmentName of referencedFragmentNames) {
// Memoize so two fragments are not compared for conflicts more than once.
if (
comparedFragmentPairs.has(
referencedFragmentName,
fragmentName,
areMutuallyExclusive,
)
) {
continue;
}
comparedFragmentPairs.add(
referencedFragmentName,
fragmentName,
areMutuallyExclusive,
);

collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
Expand Down