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(16.x.x.): avoid stack size limit in overlapping field rules as well as execute #4116

Closed
Original file line number Diff line number Diff line change
Expand Up @@ -1138,4 +1138,70 @@ describe('Validate: Overlapping fields can be merged', () => {
},
]);
});

it('does not hit stack size limits', () => {
const n = 10000;
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
const fragments = Array.from(Array(n).keys()).reduce(
(acc, next) =>
acc.concat(`\n
fragment X${next + 1} on Query {
...X${next}
}
`),
'',
);

const query = `
query Test {
...X${n}
}
${fragments}
fragment X0 on Query {
__typename
}
`;

expectErrors(query).toDeepEqual([]);
});

it('finds conflicts in nested fragments', () => {
const n = 10000;
const fragments = Array.from(Array(n).keys()).reduce(
(acc, next) =>
acc.concat(`\n
fragment X${next + 1} on Query {
...X${next}
}
`),
'',
);

const query = `
query Test {
type: conflict
...X${n}
}
${fragments}
fragment X0 on Query {
type: conflict2
__typename
}
`;
expectErrors(query).toDeepEqual([
{
locations: [
{
column: 9,
line: 3,
},
{
column: 9,
line: 50008,
},
],
message:
'Fields "type" conflict because "conflict" and "conflict2" are different fields. Use different aliases on the fields to fetch both if this was intentional.',
},
]);
});
});
106 changes: 79 additions & 27 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ function findConflictsWithinSelectionSet(
fieldMap,
);

const discoveredFragments: Array<[string, string]> = [];
if (fragmentNames.length !== 0) {
// (B) Then collect conflicts between these fields and those represented by
// each spread fragment name found.
Expand All @@ -203,7 +204,9 @@ function findConflictsWithinSelectionSet(
false,
fieldMap,
fragmentNames[i],
discoveredFragments,
);

// (C) Then compare this fragment with all other fragments found in this
// selection set to collect conflicts between fragments spread together.
// This compares each item in the list of fragment names to every other
Expand All @@ -220,6 +223,16 @@ function findConflictsWithinSelectionSet(
);
}
}

processDiscoveredFragments(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
false,
fieldMap,
discoveredFragments,
);
}
return conflicts;
}
Expand All @@ -234,6 +247,7 @@ function collectConflictsBetweenFieldsAndFragment(
areMutuallyExclusive: boolean,
fieldMap: NodeAndDefCollection,
fragmentName: string,
discoveredFragments: Array<Array<string>>,
): void {
const fragment = context.getFragment(fragmentName);
if (!fragment) {
Expand Down Expand Up @@ -264,35 +278,12 @@ function collectConflictsBetweenFieldsAndFragment(
fieldMap2,
);

// (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,
discoveredFragments.push(
...referencedFragmentNames.map((referencedFragmentName) => [
fragmentName,
areMutuallyExclusive,
);

collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap,
referencedFragmentName,
);
}
]),
);
}

// Collect all conflicts found between two fragments, including via spreading in
Expand Down Expand Up @@ -424,6 +415,7 @@ function findConflictsBetweenSubSelectionSets(

// (I) Then collect conflicts between the first collection of fields and
// those referenced by each fragment name associated with the second.
const discoveredFragments: Array<Array<string>> = [];
for (const fragmentName2 of fragmentNames2) {
collectConflictsBetweenFieldsAndFragment(
context,
Expand All @@ -433,9 +425,20 @@ function findConflictsBetweenSubSelectionSets(
areMutuallyExclusive,
fieldMap1,
fragmentName2,
discoveredFragments,
);
}

processDiscoveredFragments(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap1,
discoveredFragments,
);

// (I) Then collect conflicts between the second collection of fields and
// those referenced by each fragment name associated with the first.
for (const fragmentName1 of fragmentNames1) {
Expand All @@ -447,9 +450,20 @@ function findConflictsBetweenSubSelectionSets(
areMutuallyExclusive,
fieldMap2,
fragmentName1,
discoveredFragments,
);
}

processDiscoveredFragments(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap2,
discoveredFragments,
);

// (J) Also collect conflicts between any fragment names by the first and
// fragment names by the second. This compares each item in the first set of
// names to each item in the second set of names.
Expand All @@ -469,6 +483,44 @@ function findConflictsBetweenSubSelectionSets(
return conflicts;
}

// (E) Then collect any conflicts between the provided collection of fields
// and any fragment names found in the given fragment.
const processDiscoveredFragments = (
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
context: ValidationContext,
conflicts: Array<Conflict>,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
areMutuallyExclusive: boolean,
fieldMap: NodeAndDefCollection,
discoveredFragments: Array<Array<string>>,
) => {
while (discoveredFragments.length !== 0) {
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
const item = discoveredFragments.pop();
if (
!item ||
comparedFragmentPairs.has(item[1], item[0], areMutuallyExclusive)
) {
continue;
}
const [fragmentName, referencedFragmentName] = item;
comparedFragmentPairs.add(
referencedFragmentName,
fragmentName,
areMutuallyExclusive,
);
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap,
referencedFragmentName,
discoveredFragments,
);
}
};

// Collect all Conflicts "within" one collection of fields.
function collectConflictsWithin(
context: ValidationContext,
Expand Down
Loading