diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index f8ac936510..1a10e4cc4b 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -350,13 +350,20 @@ export class IncrementalPublisher { const streams = new Set(); const children = this._getChildren(erroringIncrementalDataRecord); - const descendants = this._getDescendants(children); + const descendants = new Set(); + this._getDescendants(children, descendants, (child) => + this._nullsChildSubsequentResultRecord(child, nullPathArray), + ); - for (const child of descendants) { - if (!this._nullsChildSubsequentResultRecord(child, nullPathArray)) { - continue; - } + if (isDeferredGroupedFieldSetRecord(erroringIncrementalDataRecord)) { + this.filterSiblings( + nullPathArray, + erroringIncrementalDataRecord, + descendants, + ); + } + for (const child of descendants) { child.filtered = true; if (isStreamItemsRecord(child)) { @@ -371,6 +378,30 @@ export class IncrementalPublisher { }); } + private filterSiblings( + nullPath: Array, + erroringIncrementalDataRecord: DeferredGroupedFieldSetRecord, + descendants: Set, + ): Set { + for (const deferredFragmentRecord of erroringIncrementalDataRecord.deferredFragmentRecords) { + for (const deferredGroupedFieldSetRecord of deferredFragmentRecord.deferredGroupedFieldSetRecords) { + if (deferredGroupedFieldSetRecord === erroringIncrementalDataRecord) { + continue; + } + if (this._matchesPath(deferredGroupedFieldSetRecord.path, nullPath)) { + deferredFragmentRecord.deferredGroupedFieldSetRecords.delete( + deferredGroupedFieldSetRecord, + ); + deferredFragmentRecord._pending.delete(deferredGroupedFieldSetRecord); + + const children = this._getChildren(deferredGroupedFieldSetRecord); + this._getDescendants(children, descendants); + } + } + } + return descendants; + } + private _pendingSourcesToResults( pendingSources: ReadonlySet, ): Array { @@ -694,10 +725,13 @@ export class IncrementalPublisher { private _getDescendants( children: ReadonlySet, descendants = new Set(), - ): ReadonlySet { + predicate = (_child: SubsequentResultRecord) => true, + ): Set { for (const child of children) { - descendants.add(child); - this._getDescendants(child.children, descendants); + if (predicate(child)) { + descendants.add(child); + this._getDescendants(child.children, descendants, () => true); + } } return descendants; } @@ -760,7 +794,6 @@ export class DeferredGroupedFieldSetRecord { path: ReadonlyArray; deferredFragmentRecords: ReadonlyArray; groupedFieldSet: GroupedFieldSet; - shouldInitiateDefer: boolean; errors: Array; data: ObjMap | undefined; sent: boolean; @@ -769,12 +802,10 @@ export class DeferredGroupedFieldSetRecord { path: Path | undefined; deferredFragmentRecords: ReadonlyArray; groupedFieldSet: GroupedFieldSet; - shouldInitiateDefer: boolean; }) { this.path = pathToArray(opts.path); this.deferredFragmentRecords = opts.deferredFragmentRecords; this.groupedFieldSet = opts.groupedFieldSet; - this.shouldInitiateDefer = opts.shouldInitiateDefer; this.errors = []; this.sent = false; } diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts index 813f4651ea..94840b4f33 100644 --- a/src/execution/__tests__/defer-test.ts +++ b/src/execution/__tests__/defer-test.ts @@ -1478,6 +1478,64 @@ describe('Execute: defer directive', () => { ]); }); + it('Cancels deferred fields when overlapping deferred results exhibits null bubbling', async () => { + const document = parse(` + query { + ... @defer { + hero { + nonNullName + id + } + } + ... @defer { + hero { + nonNullName + name + } + } + } + `); + const result = await complete(document, { + hero: { + ...hero, + nonNullName: Promise.resolve(null), + }, + }); + expectJSON(result).toDeepEqual([ + { + data: {}, + pending: [ + { id: '0', path: [] }, + { id: '1', path: [] }, + ], + hasNext: true, + }, + { + incremental: [ + { + data: { + hero: null, + }, + errors: [ + { + message: + 'Cannot return null for non-nullable field Hero.nonNullName.', + locations: [ + { line: 5, column: 13 }, + { line: 11, column: 13 }, + ], + path: ['hero', 'nonNullName'], + }, + ], + id: '0', + }, + ], + completed: [{ id: '0' }, { id: '1' }], + hasNext: false, + }, + ]); + }); + it('Deduplicates list fields', async () => { const document = parse(` query { diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index 1d0341b4cc..6105f8047b 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -51,14 +51,10 @@ export interface FieldGroup { export type GroupedFieldSet = Map; -export interface GroupedFieldSetDetails { - groupedFieldSet: GroupedFieldSet; - shouldInitiateDefer: boolean; -} - export interface CollectFieldsResult { groupedFieldSet: GroupedFieldSet; - newGroupedFieldSetDetails: Map; + supplementalGroupedFieldSets: Map; + newDeferredGroupedFieldSets: Map; newDeferUsages: ReadonlyArray; } @@ -358,7 +354,8 @@ function buildGroupedFieldSets( parentTargets = NON_DEFERRED_TARGET_SET, ): { groupedFieldSet: GroupedFieldSet; - newGroupedFieldSetDetails: Map; + supplementalGroupedFieldSets: Map; + newDeferredGroupedFieldSets: Map; } { const { parentTargetKeys, targetSetDetailsMap } = getTargetSetDetails( targetsByKey, @@ -375,10 +372,11 @@ function buildGroupedFieldSets( ) : new Map(); - const newGroupedFieldSetDetails = new Map< + const supplementalGroupedFieldSets = new Map< DeferUsageSet, - GroupedFieldSetDetails + GroupedFieldSet >(); + const newDeferredGroupedFieldSets = new Map(); for (const [maskingTargets, targetSetDetails] of targetSetDetailsMap) { const { keys, shouldInitiateDefer } = targetSetDetails; @@ -391,16 +389,16 @@ function buildGroupedFieldSets( ); // All TargetSets that causes new grouped field sets consist only of DeferUsages - // and have shouldInitiateDefer defined - newGroupedFieldSetDetails.set(maskingTargets as DeferUsageSet, { - groupedFieldSet: newGroupedFieldSet, - shouldInitiateDefer, - }); + (shouldInitiateDefer + ? newDeferredGroupedFieldSets + : supplementalGroupedFieldSets + ).set(maskingTargets as DeferUsageSet, newGroupedFieldSet); } return { groupedFieldSet, - newGroupedFieldSetDetails, + supplementalGroupedFieldSets, + newDeferredGroupedFieldSets, }; } diff --git a/src/execution/execute.ts b/src/execution/execute.ts index a19a51a217..2b0c4af160 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -52,7 +52,6 @@ import type { DeferUsageSet, FieldGroup, GroupedFieldSet, - GroupedFieldSetDetails, } from './collectFields.js'; import { collectFields, @@ -405,8 +404,12 @@ function executeOperation( ); } - const { groupedFieldSet, newGroupedFieldSetDetails, newDeferUsages } = - collectFields(schema, fragments, variableValues, rootType, operation); + const { + groupedFieldSet, + supplementalGroupedFieldSets, + newDeferredGroupedFieldSets, + newDeferUsages, + } = collectFields(schema, fragments, variableValues, rootType, operation); const newDeferMap = addNewDeferredFragments( incrementalPublisher, @@ -416,11 +419,17 @@ function executeOperation( const path = undefined; - const newDeferredGroupedFieldSetRecords = addNewDeferredGroupedFieldSets( - incrementalPublisher, - newGroupedFieldSetDetails, - newDeferMap, - path, + const [ + supplementalGroupedFieldSetRecords, + newDeferredGroupedFieldSetRecords, + ] = [supplementalGroupedFieldSets, newDeferredGroupedFieldSets].map( + (groupedFieldSets) => + addNewDeferredGroupedFieldSets( + incrementalPublisher, + groupedFieldSets, + newDeferMap, + path, + ), ); let result; @@ -466,6 +475,7 @@ function executeOperation( rootType, rootValue, path, + supplementalGroupedFieldSetRecords, newDeferredGroupedFieldSetRecords, newDeferMap, ); @@ -1497,17 +1507,17 @@ function deferredFragmentRecordFromDeferUsage( function addNewDeferredGroupedFieldSets( incrementalPublisher: IncrementalPublisher, - newGroupedFieldSetDetails: Map, + deferredGroupedFieldSets: Map, deferMap: ReadonlyMap, path?: Path | undefined, ): ReadonlyArray { - const newDeferredGroupedFieldSetRecords: Array = + const deferredGroupedFieldSetRecords: Array = []; for (const [ newGroupedFieldSetDeferUsages, - { groupedFieldSet, shouldInitiateDefer }, - ] of newGroupedFieldSetDetails) { + groupedFieldSet, + ] of deferredGroupedFieldSets) { const deferredFragmentRecords = getDeferredFragmentRecords( newGroupedFieldSetDeferUsages, deferMap, @@ -1516,15 +1526,14 @@ function addNewDeferredGroupedFieldSets( path, deferredFragmentRecords, groupedFieldSet, - shouldInitiateDefer, }); incrementalPublisher.reportNewDeferredGroupedFieldSetRecord( deferredGroupedFieldSetRecord, ); - newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord); + deferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord); } - return newDeferredGroupedFieldSetRecords; + return deferredGroupedFieldSetRecords; } function getDeferredFragmentRecords( @@ -1546,8 +1555,12 @@ function collectAndExecuteSubfields( deferMap: ReadonlyMap, ): PromiseOrValue> { // Collect sub-fields to execute to complete this value. - const { groupedFieldSet, newGroupedFieldSetDetails, newDeferUsages } = - collectSubfields(exeContext, returnType, fieldGroup); + const { + groupedFieldSet, + supplementalGroupedFieldSets, + newDeferredGroupedFieldSets, + newDeferUsages, + } = collectSubfields(exeContext, returnType, fieldGroup); const incrementalPublisher = exeContext.incrementalPublisher; @@ -1559,11 +1572,17 @@ function collectAndExecuteSubfields( path, ); - const newDeferredGroupedFieldSetRecords = addNewDeferredGroupedFieldSets( - incrementalPublisher, - newGroupedFieldSetDetails, - newDeferMap, - path, + const [ + supplementalGroupedFieldSetRecords, + newDeferredGroupedFieldSetRecords, + ] = [supplementalGroupedFieldSets, newDeferredGroupedFieldSets].map( + (groupedFieldSets) => + addNewDeferredGroupedFieldSets( + incrementalPublisher, + groupedFieldSets, + newDeferMap, + path, + ), ); const subFields = executeFields( @@ -1581,6 +1600,7 @@ function collectAndExecuteSubfields( returnType, result, path, + supplementalGroupedFieldSetRecords, newDeferredGroupedFieldSetRecords, newDeferMap, ); @@ -1888,34 +1908,36 @@ function executeDeferredGroupedFieldSets( parentType: GraphQLObjectType, sourceValue: unknown, path: Path | undefined, + supplementalGroupedFieldSetRecords: ReadonlyArray, newDeferredGroupedFieldSetRecords: ReadonlyArray, deferMap: ReadonlyMap, ): void { - for (const deferredGroupedFieldSetRecord of newDeferredGroupedFieldSetRecords) { - if (deferredGroupedFieldSetRecord.shouldInitiateDefer) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Promise.resolve().then(() => - executeDeferredGroupedFieldSet( - exeContext, - parentType, - sourceValue, - path, - deferredGroupedFieldSetRecord, - deferMap, - ), - ); - continue; - } - + for (const supplementalGroupedFieldSetRecord of supplementalGroupedFieldSetRecords) { executeDeferredGroupedFieldSet( exeContext, parentType, sourceValue, path, - deferredGroupedFieldSetRecord, + supplementalGroupedFieldSetRecord, deferMap, ); } + + if (newDeferredGroupedFieldSetRecords.length > 0) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Promise.resolve().then(() => { + for (const newDeferredGroupedFieldSetRecord of newDeferredGroupedFieldSetRecords) { + executeDeferredGroupedFieldSet( + exeContext, + parentType, + sourceValue, + path, + newDeferredGroupedFieldSetRecord, + deferMap, + ); + } + }); + } } function executeDeferredGroupedFieldSet(