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 68dd666
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 29 deletions.
22 changes: 22 additions & 0 deletions src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1138,4 +1138,26 @@ 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((fragments, next) => {

Check failure on line 1144 in src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

'fragments' is already declared in the upper scope

Check failure on line 1144 in src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Unexpected block statement surrounding arrow body; move the returned value immediately after the `=>`
return fragments.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([])
})
});
67 changes: 38 additions & 29 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,40 @@ 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 +268,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 +299,7 @@ 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,
fragmentName,
areMutuallyExclusive,
);

collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap,
referencedFragmentName,
);
}
discoveredFragments.push(...referencedFragmentNames.map(referencedFragmentName => [fragmentName, referencedFragmentName]));
}

// Collect all conflicts found between two fragments, including via spreading in
Expand Down Expand Up @@ -433,6 +440,7 @@ function findConflictsBetweenSubSelectionSets(
areMutuallyExclusive,
fieldMap1,
fragmentName2,
[]
);
}

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

Expand Down

0 comments on commit 68dd666

Please sign in to comment.