From 15f71810240e220f518f608d623e9d349a5f4d01 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 19 Jun 2024 08:10:05 +0200 Subject: [PATCH 01/10] Avoid stack size limit by moving to an iterator approach --- .../OverlappingFieldsCanBeMergedRule-test.ts | 24 ++++++++ .../rules/OverlappingFieldsCanBeMergedRule.ts | 57 ++++++++++--------- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index 46cf014e46..826f416174 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -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([]); + }); }); diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 4305064a6f..51e75d1950 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -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++) { @@ -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 @@ -234,6 +257,7 @@ function collectConflictsBetweenFieldsAndFragment( areMutuallyExclusive: boolean, fieldMap: NodeAndDefCollection, fragmentName: string, + discoveredFragments: Array>, ): void { const fragment = context.getFragment(fragmentName); if (!fragment) { @@ -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 @@ -433,6 +434,7 @@ function findConflictsBetweenSubSelectionSets( areMutuallyExclusive, fieldMap1, fragmentName2, + [], ); } @@ -447,6 +449,7 @@ function findConflictsBetweenSubSelectionSets( areMutuallyExclusive, fieldMap2, fragmentName1, + [], ); } From a23da10fc9935fbd1782c1c0f416f6632b4f02f4 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 19 Jun 2024 09:39:25 +0200 Subject: [PATCH 02/10] add missing invocations --- .../OverlappingFieldsCanBeMergedRule-test.ts | 1 + .../rules/OverlappingFieldsCanBeMergedRule.ts | 47 ++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index 826f416174..1f9c1764e2 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -1160,6 +1160,7 @@ describe('Validate: Overlapping fields can be merged', () => { __typename } `; + expectErrors(query).toDeepEqual([]); }); }); diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 51e75d1950..0967071e1b 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -425,6 +425,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> = []; for (const fragmentName2 of fragmentNames2) { collectConflictsBetweenFieldsAndFragment( context, @@ -434,7 +435,28 @@ function findConflictsBetweenSubSelectionSets( areMutuallyExclusive, fieldMap1, fragmentName2, - [], + 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], areMutuallyExclusive)) { + continue; + } + const [fragmentName, referencedFragmentName] = item; + comparedFragmentPairs.add(referencedFragmentName, fragmentName, areMutuallyExclusive); + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + areMutuallyExclusive, + fieldMap1, + fragmentName, + discoveredFragments, ); } @@ -449,7 +471,28 @@ function findConflictsBetweenSubSelectionSets( areMutuallyExclusive, fieldMap2, fragmentName1, - [], + 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], areMutuallyExclusive)) { + continue; + } + const [fragmentName, referencedFragmentName] = item; + comparedFragmentPairs.add(referencedFragmentName, fragmentName, areMutuallyExclusive); + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + areMutuallyExclusive, + fieldMap2, + fragmentName, + discoveredFragments, ); } From 56deb05b02b33654b03dda71294442a7c2647745 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 19 Jun 2024 09:58:24 +0200 Subject: [PATCH 03/10] fix bug for discovery --- .../OverlappingFieldsCanBeMergedRule-test.ts | 41 +++++++++++++++++++ .../rules/OverlappingFieldsCanBeMergedRule.ts | 28 +++++++++---- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index 1f9c1764e2..6365506e01 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -1163,4 +1163,45 @@ describe('Validate: Overlapping fields can be merged', () => { 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.', + }, + ]); + }); }); diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 0967071e1b..65b9167991 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -223,7 +223,7 @@ function findConflictsWithinSelectionSet( comparedFragmentPairs, false, fieldMap, - fragmentName, + referencedFragmentName, discoveredFragments, ); } @@ -443,11 +443,18 @@ function findConflictsBetweenSubSelectionSets( // 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], areMutuallyExclusive)) { + if ( + !item || + comparedFragmentPairs.has(item[1], item[0], areMutuallyExclusive) + ) { continue; } const [fragmentName, referencedFragmentName] = item; - comparedFragmentPairs.add(referencedFragmentName, fragmentName, areMutuallyExclusive); + comparedFragmentPairs.add( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ); collectConflictsBetweenFieldsAndFragment( context, conflicts, @@ -455,7 +462,7 @@ function findConflictsBetweenSubSelectionSets( comparedFragmentPairs, areMutuallyExclusive, fieldMap1, - fragmentName, + referencedFragmentName, discoveredFragments, ); } @@ -479,11 +486,18 @@ function findConflictsBetweenSubSelectionSets( // 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], areMutuallyExclusive)) { + if ( + !item || + comparedFragmentPairs.has(item[1], item[0], areMutuallyExclusive) + ) { continue; } const [fragmentName, referencedFragmentName] = item; - comparedFragmentPairs.add(referencedFragmentName, fragmentName, areMutuallyExclusive); + comparedFragmentPairs.add( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ); collectConflictsBetweenFieldsAndFragment( context, conflicts, @@ -491,7 +505,7 @@ function findConflictsBetweenSubSelectionSets( comparedFragmentPairs, areMutuallyExclusive, fieldMap2, - fragmentName, + referencedFragmentName, discoveredFragments, ); } From 54e1e0605abddae0e117f57c400700f1e7c67323 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 19 Jun 2024 10:02:43 +0200 Subject: [PATCH 04/10] save some array allocations --- src/validation/rules/OverlappingFieldsCanBeMergedRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 65b9167991..466326a2ce 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -191,8 +191,8 @@ function findConflictsWithinSelectionSet( fieldMap, ); + const discoveredFragments: Array<[string, string]> = []; 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++) { From d4c5449163d1caec69838f8e9157ef0cc52b129e Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 19 Jun 2024 10:12:14 +0200 Subject: [PATCH 05/10] abstract into function --- .../rules/OverlappingFieldsCanBeMergedRule.ts | 129 ++++++++---------- 1 file changed, 60 insertions(+), 69 deletions(-) diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 466326a2ce..2f61e5b0e7 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -207,26 +207,15 @@ function findConflictsWithinSelectionSet( 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, - referencedFragmentName, - discoveredFragments, - ); - } + processDiscoveredFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + false, + fieldMap, + 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 @@ -439,33 +428,15 @@ function findConflictsBetweenSubSelectionSets( ); } - // (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], areMutuallyExclusive) - ) { - continue; - } - const [fragmentName, referencedFragmentName] = item; - comparedFragmentPairs.add( - referencedFragmentName, - fragmentName, - areMutuallyExclusive, - ); - collectConflictsBetweenFieldsAndFragment( - context, - conflicts, - cachedFieldsAndFragmentNames, - comparedFragmentPairs, - areMutuallyExclusive, - fieldMap1, - referencedFragmentName, - 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. @@ -482,8 +453,46 @@ function findConflictsBetweenSubSelectionSets( ); } - // (E) Then collect any conflicts between the provided collection of fields - // and any fragment names found in the given fragment. + 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. + for (const fragmentName1 of fragmentNames1) { + for (const fragmentName2 of fragmentNames2) { + collectConflictsBetweenFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + areMutuallyExclusive, + fragmentName1, + fragmentName2, + ); + } + } + return conflicts; +} + +// (E) Then collect any conflicts between the provided collection of fields +// and any fragment names found in the given fragment. +const processDiscoveredFragments = ( + context: ValidationContext, + conflicts: Array, + cachedFieldsAndFragmentNames: Map, + comparedFragmentPairs: PairSet, + areMutuallyExclusive: boolean, + fieldMap: NodeAndDefCollection, + discoveredFragments: Array>, +) => { while (discoveredFragments.length !== 0) { const item = discoveredFragments.pop(); if ( @@ -504,30 +513,12 @@ function findConflictsBetweenSubSelectionSets( cachedFieldsAndFragmentNames, comparedFragmentPairs, areMutuallyExclusive, - fieldMap2, + fieldMap, referencedFragmentName, 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. - for (const fragmentName1 of fragmentNames1) { - for (const fragmentName2 of fragmentNames2) { - collectConflictsBetweenFragments( - context, - conflicts, - cachedFieldsAndFragmentNames, - comparedFragmentPairs, - areMutuallyExclusive, - fragmentName1, - fragmentName2, - ); - } - } - return conflicts; -} +}; // Collect all Conflicts "within" one collection of fields. function collectConflictsWithin( From f6fd3cf6f93cdea0cccf76abb0a3428fd0629cc6 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Wed, 19 Jun 2024 10:21:11 +0200 Subject: [PATCH 06/10] delay procesing discovered fragments --- .../rules/OverlappingFieldsCanBeMergedRule.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 2f61e5b0e7..60d5c680e9 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -207,15 +207,6 @@ function findConflictsWithinSelectionSet( discoveredFragments, ); - processDiscoveredFragments( - context, - conflicts, - cachedFieldsAndFragmentNames, - comparedFragmentPairs, - false, - fieldMap, - 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 @@ -232,6 +223,16 @@ function findConflictsWithinSelectionSet( ); } } + + processDiscoveredFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + false, + fieldMap, + discoveredFragments, + ); } return conflicts; } From 002a2adb6e805af9df58d4ed48c54433a05940e3 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Wed, 19 Jun 2024 11:53:42 +0200 Subject: [PATCH 07/10] Apply suggestion Co-Authored-By: yaacovCR --- .../rules/OverlappingFieldsCanBeMergedRule.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 60d5c680e9..8f5b07756f 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -485,7 +485,7 @@ function findConflictsBetweenSubSelectionSets( // (E) Then collect any conflicts between the provided collection of fields // and any fragment names found in the given fragment. -const processDiscoveredFragments = ( +function processDiscoveredFragments( context: ValidationContext, conflicts: Array, cachedFieldsAndFragmentNames: Map, @@ -493,16 +493,19 @@ const processDiscoveredFragments = ( areMutuallyExclusive: boolean, fieldMap: NodeAndDefCollection, discoveredFragments: Array>, -) => { - while (discoveredFragments.length !== 0) { - const item = discoveredFragments.pop(); +) { + let item; + while ((item = discoveredFragments.pop()) !== undefined) { + const [fragmentName, referencedFragmentName] = item; if ( - !item || - comparedFragmentPairs.has(item[1], item[0], areMutuallyExclusive) + comparedFragmentPairs.has( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ) ) { continue; } - const [fragmentName, referencedFragmentName] = item; comparedFragmentPairs.add( referencedFragmentName, fragmentName, @@ -519,7 +522,7 @@ const processDiscoveredFragments = ( discoveredFragments, ); } -}; +} // Collect all Conflicts "within" one collection of fields. function collectConflictsWithin( From d9c4bd3eefb50dbe6ffa15b0c21b76e5d761bcd5 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Wed, 19 Jun 2024 13:06:15 +0200 Subject: [PATCH 08/10] show failure in execution --- src/execution/__tests__/executor-test.ts | 44 ++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index c758d3e426..ff366c6631 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -22,6 +22,7 @@ import { GraphQLBoolean, GraphQLInt, GraphQLString } from '../../type/scalars'; import { GraphQLSchema } from '../../type/schema'; import { execute, executeSync } from '../execute'; +import { print } from '../../language'; describe('Execute: Handles basic execution tasks', () => { it('throws if no document is provided', () => { @@ -72,6 +73,49 @@ describe('Execute: Handles basic execution tasks', () => { ); }); + it('works with deeply nested fragments', async () => { + const DataType: GraphQLObjectType = new GraphQLObjectType({ + name: 'Query', + fields: () => ({ + a: { type: GraphQLString, resolve: () => 'Apple' }, + }), + }); + + + 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 document = parse(` + query { + ...X${n} + __typename + } + ${fragments} + fragment X0 on Query { + a + } + `); + + const result = await execute({ + schema: new GraphQLSchema({ query: DataType }), + document, + }); + + expect(result).to.deep.equal({ + data: { + a: 'Apple', + }, + }); + }); + it('executes arbitrary code', async () => { const data = { a: () => 'Apple', From 006e077e3b3791a000b3ef55ab8f9611440c44a5 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Wed, 19 Jun 2024 13:17:51 +0200 Subject: [PATCH 09/10] fix in execute as well --- src/execution/__tests__/executor-test.ts | 3 -- src/execution/collectFields.ts | 55 +++++++++++++++++++----- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index ff366c6631..a283dcbf23 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -22,7 +22,6 @@ import { GraphQLBoolean, GraphQLInt, GraphQLString } from '../../type/scalars'; import { GraphQLSchema } from '../../type/schema'; import { execute, executeSync } from '../execute'; -import { print } from '../../language'; describe('Execute: Handles basic execution tasks', () => { it('throws if no document is provided', () => { @@ -81,7 +80,6 @@ describe('Execute: Handles basic execution tasks', () => { }), }); - const n = 10000; const fragments = Array.from(Array(n).keys()).reduce( (acc, next) => @@ -96,7 +94,6 @@ describe('Execute: Handles basic execution tasks', () => { const document = parse(` query { ...X${n} - __typename } ${fragments} fragment X0 on Query { diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index d0961bfae8..06ade32c5b 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -21,6 +21,11 @@ import { typeFromAST } from '../utilities/typeFromAST'; import { getDirectiveValues } from './values'; +interface FragmentEntry { + fragment: FragmentDefinitionNode; + runtimeType: GraphQLObjectType; +} + /** * Given a selectionSet, collects all of the fields and returns them. * @@ -37,7 +42,9 @@ export function collectFields( runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, ): Map> { + const foundFragments: Array = []; const fields = new Map(); + const visited = new Set(); collectFieldsImpl( schema, fragments, @@ -45,8 +52,24 @@ export function collectFields( runtimeType, selectionSet, fields, - new Set(), + visited, + foundFragments, ); + + let entry; + while ((entry = foundFragments.pop()) !== undefined) { + collectFieldsImpl( + schema, + fragments, + variableValues, + entry.runtimeType, + entry.fragment.selectionSet, + fields, + visited, + foundFragments, + ); + } + return fields; } @@ -68,6 +91,7 @@ export function collectSubfields( fieldNodes: ReadonlyArray, ): Map> { const subFieldNodes = new Map(); + const foundFragments: Array = []; const visitedFragmentNames = new Set(); for (const node of fieldNodes) { if (node.selectionSet) { @@ -79,9 +103,25 @@ export function collectSubfields( node.selectionSet, subFieldNodes, visitedFragmentNames, + foundFragments, ); } } + + let entry; + while ((entry = foundFragments.pop()) !== undefined) { + collectFieldsImpl( + schema, + fragments, + variableValues, + entry.runtimeType, + entry.fragment.selectionSet, + subFieldNodes, + visitedFragmentNames, + foundFragments, + ); + } + return subFieldNodes; } @@ -93,6 +133,7 @@ function collectFieldsImpl( selectionSet: SelectionSetNode, fields: Map>, visitedFragmentNames: Set, + foundFragments: Array, ): void { for (const selection of selectionSet.selections) { switch (selection.kind) { @@ -124,6 +165,7 @@ function collectFieldsImpl( selection.selectionSet, fields, visitedFragmentNames, + foundFragments, ); break; } @@ -143,15 +185,8 @@ function collectFieldsImpl( ) { continue; } - collectFieldsImpl( - schema, - fragments, - variableValues, - runtimeType, - fragment.selectionSet, - fields, - visitedFragmentNames, - ); + + foundFragments.push({ runtimeType, fragment }); break; } } From 735e88bc49775924ab31cf166d98715864ec4416 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Mon, 24 Jun 2024 17:16:17 +0200 Subject: [PATCH 10/10] stack iterator depth first --- src/execution/collectFields.ts | 118 ++++++++++++++++----------------- 1 file changed, 56 insertions(+), 62 deletions(-) diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index 06ade32c5b..e70f70ce69 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -21,11 +21,18 @@ import { typeFromAST } from '../utilities/typeFromAST'; import { getDirectiveValues } from './values'; -interface FragmentEntry { - fragment: FragmentDefinitionNode; +interface FieldEntry { + selection: FieldNode; + name: string; +} + +interface EntryWithSelectionset { + selectionSet: SelectionSetNode; runtimeType: GraphQLObjectType; } +type StackEntry = EntryWithSelectionset | FieldEntry; + /** * Given a selectionSet, collects all of the fields and returns them. * @@ -42,32 +49,30 @@ export function collectFields( runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, ): Map> { - const foundFragments: Array = []; + const stack: Array = [{ selectionSet, runtimeType }]; const fields = new Map(); const visited = new Set(); - collectFieldsImpl( - schema, - fragments, - variableValues, - runtimeType, - selectionSet, - fields, - visited, - foundFragments, - ); let entry; - while ((entry = foundFragments.pop()) !== undefined) { - collectFieldsImpl( - schema, - fragments, - variableValues, - entry.runtimeType, - entry.fragment.selectionSet, - fields, - visited, - foundFragments, - ); + while ((entry = stack.shift()) !== undefined) { + if ('selectionSet' in entry) { + collectFieldsImpl( + schema, + fragments, + variableValues, + entry.runtimeType, + entry.selectionSet, + visited, + stack, + ); + } else { + const fieldList = fields.get(entry.name); + if (fieldList !== undefined) { + fieldList.push(entry.selection); + } else { + fields.set(entry.name, [entry.selection]); + } + } } return fields; @@ -91,37 +96,36 @@ export function collectSubfields( fieldNodes: ReadonlyArray, ): Map> { const subFieldNodes = new Map(); - const foundFragments: Array = []; + const stack: Array = []; const visitedFragmentNames = new Set(); for (const node of fieldNodes) { if (node.selectionSet) { + stack.push({ selectionSet: node.selectionSet, runtimeType: returnType }); + } + } + + let entry; + while ((entry = stack.shift()) !== undefined) { + if ('selectionSet' in entry) { collectFieldsImpl( schema, fragments, variableValues, - returnType, - node.selectionSet, - subFieldNodes, + entry.runtimeType, + entry.selectionSet, visitedFragmentNames, - foundFragments, + stack, ); + } else { + const fieldList = subFieldNodes.get(entry.name); + if (fieldList !== undefined) { + fieldList.push(entry.selection); + } else { + subFieldNodes.set(entry.name, [entry.selection]); + } } } - let entry; - while ((entry = foundFragments.pop()) !== undefined) { - collectFieldsImpl( - schema, - fragments, - variableValues, - entry.runtimeType, - entry.fragment.selectionSet, - subFieldNodes, - visitedFragmentNames, - foundFragments, - ); - } - return subFieldNodes; } @@ -131,10 +135,10 @@ function collectFieldsImpl( variableValues: { [variable: string]: unknown }, runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, - fields: Map>, visitedFragmentNames: Set, - foundFragments: Array, + stack: Array, ): void { + const discovered = []; for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { @@ -142,12 +146,7 @@ function collectFieldsImpl( continue; } const name = getFieldEntryKey(selection); - const fieldList = fields.get(name); - if (fieldList !== undefined) { - fieldList.push(selection); - } else { - fields.set(name, [selection]); - } + discovered.push({ selection, name }); break; } case Kind.INLINE_FRAGMENT: { @@ -157,16 +156,7 @@ function collectFieldsImpl( ) { continue; } - collectFieldsImpl( - schema, - fragments, - variableValues, - runtimeType, - selection.selectionSet, - fields, - visitedFragmentNames, - foundFragments, - ); + discovered.push({ selectionSet: selection.selectionSet, runtimeType }); break; } case Kind.FRAGMENT_SPREAD: { @@ -186,11 +176,15 @@ function collectFieldsImpl( continue; } - foundFragments.push({ runtimeType, fragment }); + discovered.push({ selectionSet: fragment.selectionSet, runtimeType }); break; } } } + + if (discovered.length !== 0) { + stack.unshift(...discovered); + } } /**