From b6b15c6ae2b5fb20d2dfe949f9d09fad34ba333b Mon Sep 17 00:00:00 2001
From: Yaacov Rydzinski <yaacovCR@gmail.com>
Date: Tue, 7 Nov 2023 12:11:18 +0200
Subject: [PATCH] Cancels deferred fields when overlapping deferred results
exhibits null bubbling
---
src/execution/IncrementalPublisher.ts | 53 +++++++++++---
src/execution/__tests__/defer-test.ts | 58 +++++++++++++++
src/execution/collectFields.ts | 28 ++++----
src/execution/execute.ts | 100 ++++++++++++++++----------
4 files changed, 174 insertions(+), 65 deletions(-)
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<StreamRecord>();
const children = this._getChildren(erroringIncrementalDataRecord);
- const descendants = this._getDescendants(children);
+ const descendants = new Set<SubsequentResultRecord>();
+ 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<string | number>,
+ erroringIncrementalDataRecord: DeferredGroupedFieldSetRecord,
+ descendants: Set<SubsequentResultRecord>,
+ ): Set<SubsequentResultRecord> {
+ 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<DeferredFragmentRecord | StreamRecord>,
): Array<PendingResult> {
@@ -694,10 +725,13 @@ export class IncrementalPublisher {
private _getDescendants(
children: ReadonlySet<SubsequentResultRecord>,
descendants = new Set<SubsequentResultRecord>(),
- ): ReadonlySet<SubsequentResultRecord> {
+ predicate = (_child: SubsequentResultRecord) => true,
+ ): Set<SubsequentResultRecord> {
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<string | number>;
deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
groupedFieldSet: GroupedFieldSet;
- shouldInitiateDefer: boolean;
errors: Array<GraphQLError>;
data: ObjMap<unknown> | undefined;
sent: boolean;
@@ -769,12 +802,10 @@ export class DeferredGroupedFieldSetRecord {
path: Path | undefined;
deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
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<string, FieldGroup>;
-export interface GroupedFieldSetDetails {
- groupedFieldSet: GroupedFieldSet;
- shouldInitiateDefer: boolean;
-}
-
export interface CollectFieldsResult {
groupedFieldSet: GroupedFieldSet;
- newGroupedFieldSetDetails: Map<DeferUsageSet, GroupedFieldSetDetails>;
+ supplementalGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
+ newDeferredGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
newDeferUsages: ReadonlyArray<DeferUsage>;
}
@@ -358,7 +354,8 @@ function buildGroupedFieldSets(
parentTargets = NON_DEFERRED_TARGET_SET,
): {
groupedFieldSet: GroupedFieldSet;
- newGroupedFieldSetDetails: Map<DeferUsageSet, GroupedFieldSetDetails>;
+ supplementalGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
+ newDeferredGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
} {
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<DeferUsageSet, GroupedFieldSet>();
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<DeferUsageSet, GroupedFieldSetDetails>,
+ deferredGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>,
deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord>,
path?: Path | undefined,
): ReadonlyArray<DeferredGroupedFieldSetRecord> {
- const newDeferredGroupedFieldSetRecords: Array<DeferredGroupedFieldSetRecord> =
+ const deferredGroupedFieldSetRecords: Array<DeferredGroupedFieldSetRecord> =
[];
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<DeferUsage, DeferredFragmentRecord>,
): PromiseOrValue<ObjMap<unknown>> {
// 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<DeferredGroupedFieldSetRecord>,
newDeferredGroupedFieldSetRecords: ReadonlyArray<DeferredGroupedFieldSetRecord>,
deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord>,
): 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(