Skip to content

Commit

Permalink
Avoid stack size limit by moving to an iterator approach
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Jun 19, 2024
1 parent 08779a0 commit 15f7181
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 27 deletions.
24 changes: 24 additions & 0 deletions src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1138,4 +1138,28 @@ describe('Validate: Overlapping fields can be merged', () => {
},
]);
});

it('does not hit stack size limits', () => {
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 {
...X${n}
}
${fragments}
fragment X0 on Query {
__typename
}
`;
expectErrors(query).toDeepEqual([]);
});
});
57 changes: 30 additions & 27 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ function findConflictsWithinSelectionSet(
);

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

// (E) Then collect any conflicts between the provided collection of fields
// and any fragment names found in the given fragment.
while (discoveredFragments.length !== 0) {
const item = discoveredFragments.pop();
if (!item || comparedFragmentPairs.has(item[1], item[0], false)) {
continue;
}
const [fragmentName, referencedFragmentName] = item;
comparedFragmentPairs.add(referencedFragmentName, fragmentName, false);
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
false,
fieldMap,
fragmentName,
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 Down Expand Up @@ -234,6 +257,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 +288,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 @@ -433,6 +434,7 @@ function findConflictsBetweenSubSelectionSets(
areMutuallyExclusive,
fieldMap1,
fragmentName2,
[],
);
}

Expand All @@ -447,6 +449,7 @@ function findConflictsBetweenSubSelectionSets(
areMutuallyExclusive,
fieldMap2,
fragmentName1,
[],
);
}

Expand Down

0 comments on commit 15f7181

Please sign in to comment.