alternative solution to #4357

src/execution/IncrementalGraph.ts

@@ -35,9 +35,22 @@ export class IncrementalGraph {
   }
 
   getNewRootNodes(
+    newDeferredFragmentRecords:
+      | ReadonlyArray<DeferredFragmentRecord>
+      | undefined,
     incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
   ): ReadonlyArray<DeliveryGroup> {
     const initialResultChildren = new Set<DeliveryGroup>();
+
+    if (newDeferredFragmentRecords !== undefined) {
+      for (const deferredFragmentRecord of newDeferredFragmentRecords) {
+        this._addDeferredFragment(
+          deferredFragmentRecord,
+          initialResultChildren,
+        );
+      }
+    }
+
     this._addIncrementalDataRecords(
       incrementalDataRecords,
       undefined,
@@ -49,8 +62,17 @@ export class IncrementalGraph {
   addCompletedSuccessfulExecutionGroup(
     successfulExecutionGroup: SuccessfulExecutionGroup,
   ): void {
-    const { pendingExecutionGroup, incrementalDataRecords } =
-      successfulExecutionGroup;
+    const {
+      pendingExecutionGroup,
+      newDeferredFragmentRecords,
+      incrementalDataRecords,
+    } = successfulExecutionGroup;
+
+    if (newDeferredFragmentRecords !== undefined) {
+      for (const deferredFragmentRecord of newDeferredFragmentRecords) {
+        this._addDeferredFragment(deferredFragmentRecord, undefined);
+      }
+    }
 
     const deferredFragmentRecords =
       pendingExecutionGroup.deferredFragmentRecords;
@@ -135,11 +157,7 @@ export class IncrementalGraph {
   removeDeferredFragment(
     deferredFragmentRecord: DeferredFragmentRecord,
   ): boolean {
-    if (!this._rootNodes.has(deferredFragmentRecord)) {
-      return false;
-    }
-    this._rootNodes.delete(deferredFragmentRecord);
-    return true;
+    return this._rootNodes.delete(deferredFragmentRecord);
   }
 
   removeStream(streamRecord: StreamRecord): void {
@@ -154,10 +172,6 @@ export class IncrementalGraph {
     for (const incrementalDataRecord of incrementalDataRecords) {
       if (isPendingExecutionGroup(incrementalDataRecord)) {
         for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) {
-          this._addDeferredFragment(
-            deferredFragmentRecord,
-            initialResultChildren,
-          );
           deferredFragmentRecord.pendingExecutionGroups.add(
             incrementalDataRecord,
           );
@@ -170,7 +184,6 @@ export class IncrementalGraph {
         initialResultChildren.add(incrementalDataRecord);
       } else {
         for (const parent of parents) {
-          this._addDeferredFragment(parent, initialResultChildren);
           parent.children.add(incrementalDataRecord);
         }
       }
@@ -219,17 +232,13 @@ export class IncrementalGraph {
     deferredFragmentRecord: DeferredFragmentRecord,
     initialResultChildren: Set<DeliveryGroup> | undefined,
   ): void {
-    if (this._rootNodes.has(deferredFragmentRecord)) {
-      return;
-    }
     const parent = deferredFragmentRecord.parent;
     if (parent === undefined) {
       invariant(initialResultChildren !== undefined);
       initialResultChildren.add(deferredFragmentRecord);
       return;
     }
     parent.children.add(deferredFragmentRecord);
-    this._addDeferredFragment(parent, initialResultChildren);
   }
 
   private _onExecutionGroup(
@@ -251,6 +260,7 @@ export class IncrementalGraph {
   private async _onStreamItems(streamRecord: StreamRecord): Promise<void> {
     let items: Array<unknown> = [];
     let errors: Array<GraphQLError> = [];
+    let newDeferredFragmentRecords: Array<DeferredFragmentRecord> = [];
     let incrementalDataRecords: Array<IncrementalDataRecord> = [];
     const streamItemQueue = streamRecord.streamItemQueue;
     let streamItemRecord: StreamItemRecord | undefined;
@@ -268,10 +278,12 @@ export class IncrementalGraph {
               errors.length > 0 /* c8 ignore start */
                 ? { items, errors } /* c8 ignore stop */
                 : { items },
+            newDeferredFragmentRecords,
             incrementalDataRecords,
           });
           items = [];
           errors = [];
+          newDeferredFragmentRecords = [];
           incrementalDataRecords = [];
         }
         // eslint-disable-next-line no-await-in-loop
@@ -286,6 +298,7 @@ export class IncrementalGraph {
           this._enqueue({
             streamRecord,
             result: errors.length > 0 ? { items, errors } : { items },
+            newDeferredFragmentRecords,
             incrementalDataRecords,
           });
         }
@@ -303,6 +316,9 @@ export class IncrementalGraph {
       if (result.errors !== undefined) {
         errors.push(...result.errors);
       }
+      if (result.newDeferredFragmentRecords !== undefined) {
+        newDeferredFragmentRecords.push(...result.newDeferredFragmentRecords);
+      }
       if (result.incrementalDataRecords !== undefined) {
         incrementalDataRecords.push(...result.incrementalDataRecords);
       }

src/execution/IncrementalPublisher.ts

@@ -33,12 +33,14 @@ export function buildIncrementalResponse(
   context: IncrementalPublisherContext,
   result: ObjMap<unknown>,
   errors: ReadonlyArray<GraphQLError> | undefined,
+  newDeferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord> | undefined,
   incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
 ): ExperimentalIncrementalExecutionResults {
   const incrementalPublisher = new IncrementalPublisher(context);
   return incrementalPublisher.buildResponse(
     result,
     errors,
+    newDeferredFragmentRecords,
     incrementalDataRecords,
   );
 }
@@ -74,9 +76,13 @@ class IncrementalPublisher {
   buildResponse(
     data: ObjMap<unknown>,
     errors: ReadonlyArray<GraphQLError> | undefined,
+    newDeferredFragmentRecords:
+      | ReadonlyArray<DeferredFragmentRecord>
+      | undefined,
     incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
   ): ExperimentalIncrementalExecutionResults {
     const newRootNodes = this._incrementalGraph.getNewRootNodes(
+      newDeferredFragmentRecords,
       incrementalDataRecords,
     );
 
@@ -228,18 +234,16 @@ class IncrementalPublisher {
     if (isFailedExecutionGroup(completedExecutionGroup)) {
       for (const deferredFragmentRecord of completedExecutionGroup
         .pendingExecutionGroup.deferredFragmentRecords) {
-        const id = deferredFragmentRecord.id;
         if (
-          !this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord)
+          this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord)
         ) {
-          // This can occur if multiple deferred grouped field sets error for a fragment.
-          continue;
+          const id = deferredFragmentRecord.id;
+          invariant(id !== undefined);
+          context.completed.push({
+            id,
+            errors: completedExecutionGroup.errors,
+          });
         }
-        invariant(id !== undefined);
-        context.completed.push({
-          id,
-          errors: completedExecutionGroup.errors,
-        });
       }
       return;
     }
@@ -316,9 +320,11 @@ class IncrementalPublisher {
 
       context.incremental.push(incrementalEntry);
 
-      const incrementalDataRecords = streamItemsResult.incrementalDataRecords;
+      const { newDeferredFragmentRecords, incrementalDataRecords } =
+        streamItemsResult;
       if (incrementalDataRecords !== undefined) {
         const newRootNodes = this._incrementalGraph.getNewRootNodes(
+          newDeferredFragmentRecords,
           incrementalDataRecords,
         );
         context.pending.push(...this._toPendingResults(newRootNodes));