diff --git a/src/execution/__tests__/collectFields-test.ts b/src/execution/__tests__/collectFields-test.ts new file mode 100644 index 0000000000..245d8424fd --- /dev/null +++ b/src/execution/__tests__/collectFields-test.ts @@ -0,0 +1,109 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { invariant } from '../../jsutils/invariant.js'; + +import { parse } from '../../language/parser.js'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import { collectFields } from '../collectFields.js'; +import { validateExecutionArgs } from '../execute.js'; + +const schema = buildSchema(` + type Query { + field: String + } +`); + +function collectRootFields(query: string) { + const validatedExecutionArgs = validateExecutionArgs({ + schema, + document: parse(query), + }); + + invariant('operation' in validatedExecutionArgs); + + const { operation, fragments, variableValues } = validatedExecutionArgs; + + const queryType = schema.getQueryType(); + + invariant(queryType != null); + + return collectFields( + schema, + fragments, + variableValues, + queryType, + operation.selectionSet, + false, + ); +} + +describe('collectFields', () => { + describe('overlapping fragment spreads', () => { + it('should not collect a deferred spread after a non-deferred spread has been collected', () => { + const { newDeferUsages } = collectRootFields(` + query { + ...FragmentName + ...FragmentName @defer + } + fragment FragmentName on Query { + field + } + `); + + expect(newDeferUsages).to.have.lengthOf(0); + }); + + it('should not collect a deferred spread after a deferred spread has been collected', () => { + const { newDeferUsages } = collectRootFields(` + query { + ...FragmentName @defer + ...FragmentName @defer + } + fragment FragmentName on Query { + field + } + `); + + expect(newDeferUsages).to.have.lengthOf(1); + }); + + it('should collect a non-deferred spread after a deferred spread has been collected', () => { + const { groupedFieldSet } = collectRootFields(` + query { + ...FragmentName @defer + ...FragmentName + } + fragment FragmentName on Query { + field + } + `); + + const fieldDetailsList = groupedFieldSet.get('field'); + + invariant(fieldDetailsList != null); + + expect(fieldDetailsList).to.have.lengthOf(2); + }); + + it('should not collect a non-deferred spread after a non-deferred spread has been collected', () => { + const { groupedFieldSet } = collectRootFields(` + query { + ...FragmentName + ...FragmentName + } + fragment FragmentName on Query { + field + } + `); + + const fieldDetailsList = groupedFieldSet.get('field'); + + invariant(fieldDetailsList != null); + + expect(fieldDetailsList).to.have.lengthOf(1); + }); + }); +}); diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index 26367387f3..2c166296f4 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -67,7 +67,7 @@ interface CollectFieldsContext { fragments: ObjMap; variableValues: VariableValues; runtimeType: GraphQLObjectType; - visitedFragmentNames: Set; + visitedFragmentNames: Map; hideSuggestions: boolean; forbiddenDirectiveInstances: Array; forbidSkipAndInclude: boolean; @@ -103,7 +103,7 @@ export function collectFields( fragments, variableValues, runtimeType, - visitedFragmentNames: new Set(), + visitedFragmentNames: new Map(), hideSuggestions, forbiddenDirectiveInstances: [], forbidSkipAndInclude, @@ -144,7 +144,7 @@ export function collectSubfields( fragments, variableValues, runtimeType: returnType, - visitedFragmentNames: new Set(), + visitedFragmentNames: new Map(), hideSuggestions, forbiddenDirectiveInstances: [], forbidSkipAndInclude: false, @@ -258,7 +258,6 @@ function collectFieldsImpl( const fragName = selection.name.value; if ( - visitedFragmentNames.has(fragName) || !shouldIncludeNode( context, selection, @@ -284,6 +283,28 @@ function collectFieldsImpl( deferUsage, ); + const visitedAsDeferred = visitedFragmentNames.get(fragName); + + let maybeNewDeferUsage: DeferUsage | undefined; + if (!newDeferUsage) { + // If this spread is not deferred, it may be skipped when already visited + // as a non-deferred spread. If it was previously visited as a deferred spread, + // it must be revisited. + if (visitedAsDeferred === false) { + continue; + } + visitedFragmentNames.set(fragName, false); + maybeNewDeferUsage = deferUsage; + } else { + // If this spread is deferred, it can be skipped if it has already been visited. + if (visitedAsDeferred !== undefined) { + continue; + } + visitedFragmentNames.set(fragName, true); + newDeferUsages.push(newDeferUsage); + maybeNewDeferUsage = newDeferUsage; + } + const fragmentVariableSignatures = fragment.variableSignatures; let newFragmentVariableValues: FragmentVariableValues | undefined; if (fragmentVariableSignatures) { @@ -296,27 +317,14 @@ function collectFieldsImpl( ); } - if (!newDeferUsage) { - visitedFragmentNames.add(fragName); - collectFieldsImpl( - context, - fragment.definition.selectionSet, - groupedFieldSet, - newDeferUsages, - deferUsage, - newFragmentVariableValues, - ); - } else { - newDeferUsages.push(newDeferUsage); - collectFieldsImpl( - context, - fragment.definition.selectionSet, - groupedFieldSet, - newDeferUsages, - newDeferUsage, - newFragmentVariableValues, - ); - } + collectFieldsImpl( + context, + fragment.definition.selectionSet, + groupedFieldSet, + newDeferUsages, + maybeNewDeferUsage, + newFragmentVariableValues, + ); break; } }