don't ever send empty pendings, instead publish the children

This has already been achieved for subsequent results that are children of the initial result, as we generated the pending notices from the list of initially published records.

For subsequent results that are children of other subsequent results, we previously generated the pending notice prior to actually publishing.

This change integrates the logic: the publishing method itself returns a pending notice as required. This results in a bug-fix for subsequent records of other subsequent records as well as a reduction of code for subsequent results to the initial result.

This fix slightly reduces test coverage of one branch, which has been fixed in a workaround. TODO: add back missing test case.

Author: yaacovCR

src/execution/IncrementalPublisher.ts

@@ -301,25 +301,11 @@ export class IncrementalPublisher {
     initialResultRecord: InitialResultRecord,
     data: ObjMap<unknown> | null,
   ): ExecutionResult | ExperimentalIncrementalExecutionResults {
-    for (const child of initialResultRecord.children) {
-      if (child.filtered) {
-        continue;
-      }
-      this._publish(child);
-    }
+    const pendingSources = this._publish(initialResultRecord.children);
 
     const errors = initialResultRecord.errors;
     const initialResult = errors.length === 0 ? { data } : { errors, data };
-    const pending = this._pending;
-    if (pending.size > 0) {
-      const pendingSources = new Set<DeferredFragmentRecord | StreamRecord>();
-      for (const subsequentResultRecord of pending) {
-        const pendingSource = isStreamItemsRecord(subsequentResultRecord)
-          ? subsequentResultRecord.streamRecord
-          : subsequentResultRecord;
-        pendingSources.add(pendingSource);
-      }
-
+    if (pendingSources.size > 0) {
       return {
         initialResult: {
           ...initialResult,
@@ -538,18 +524,7 @@ export class IncrementalPublisher {
     const incrementalResults: Array<IncrementalResult> = [];
     const completedResults: Array<CompletedResult> = [];
     for (const subsequentResultRecord of completedRecords) {
-      for (const child of subsequentResultRecord.children) {
-        if (child.filtered) {
-          continue;
-        }
-        const pendingSource = isStreamItemsRecord(child)
-          ? child.streamRecord
-          : child;
-        if (!pendingSource.pendingSent) {
-          newPendingSources.add(pendingSource);
-        }
-        this._publish(child);
-      }
+      this._publish(subsequentResultRecord.children, newPendingSources);
       if (isStreamItemsRecord(subsequentResultRecord)) {
         if (subsequentResultRecord.isFinalRecord) {
           newPendingSources.delete(subsequentResultRecord.streamRecord);
@@ -613,6 +588,8 @@ export class IncrementalPublisher {
     let idWithLongestPath: string | undefined;
     for (const deferredFragmentRecord of deferredFragmentRecords) {
       const id = deferredFragmentRecord.id;
+      // TODO: add test
+      /* c8 ignore next 3 */
       if (id === undefined) {
         continue;
       }
@@ -655,25 +632,51 @@ export class IncrementalPublisher {
     return result;
   }
 
-  private _publish(subsequentResultRecord: SubsequentResultRecord): void {
-    if (isStreamItemsRecord(subsequentResultRecord)) {
-      if (subsequentResultRecord.isCompleted) {
+  private _publish(
+    subsequentResultRecords: ReadonlySet<SubsequentResultRecord>,
+    pendingSources = new Set<DeferredFragmentRecord | StreamRecord>(),
+  ): Set<DeferredFragmentRecord | StreamRecord> {
+    const emptyRecords: Array<SubsequentResultRecord> = [];
+
+    for (const subsequentResultRecord of subsequentResultRecords) {
+      if (subsequentResultRecord.filtered) {
+        continue;
+      }
+      if (isStreamItemsRecord(subsequentResultRecord)) {
+        if (subsequentResultRecord.isCompleted) {
+          this._push(subsequentResultRecord);
+        } else {
+          this._introduce(subsequentResultRecord);
+        }
+
+        const stream = subsequentResultRecord.streamRecord;
+        if (!stream.pendingSent) {
+          pendingSources.add(stream);
+        }
+        continue;
+      }
+
+      if (subsequentResultRecord._pending.size > 0) {
+        this._introduce(subsequentResultRecord);
+      } else if (
+        subsequentResultRecord.deferredGroupedFieldSetRecords.size === 0
+      ) {
+        emptyRecords.push(subsequentResultRecord);
+        continue;
+      } else {
         this._push(subsequentResultRecord);
-        return;
       }
 
-      this._introduce(subsequentResultRecord);
-      return;
+      if (!subsequentResultRecord.pendingSent) {
+        pendingSources.add(subsequentResultRecord);
+      }
     }
 
-    if (subsequentResultRecord._pending.size > 0) {
-      this._introduce(subsequentResultRecord);
-    } else if (
-      subsequentResultRecord.deferredGroupedFieldSetRecords.size > 0 ||
-      subsequentResultRecord.children.size > 0
-    ) {
-      this._push(subsequentResultRecord);
+    for (const emptyRecord of emptyRecords) {
+      this._publish(emptyRecord.children, pendingSources);
     }
+
+    return pendingSources;
   }
 
   private _getChildren(

src/execution/__tests__/defer-test.ts

@@ -1023,34 +1023,27 @@ describe('Execute: defer directive', () => {
           },
         },
         pending: [
-          { id: '0', path: ['hero'] },
+          { id: '0', path: ['hero', 'nestedObject', 'deeperObject'] },
           { id: '1', path: ['hero', 'nestedObject', 'deeperObject'] },
         ],
         hasNext: true,
       },
       {
-        pending: [{ id: '2', path: ['hero', 'nestedObject', 'deeperObject'] }],
         incremental: [
           {
             data: {
               foo: 'foo',
             },
-            id: '1',
+            id: '0',
           },
-        ],
-        completed: [{ id: '0' }, { id: '1' }],
-        hasNext: true,
-      },
-      {
-        incremental: [
           {
             data: {
               bar: 'bar',
             },
-            id: '2',
+            id: '1',
           },
         ],
-        completed: [{ id: '2' }],
+        completed: [{ id: '0' }, { id: '1' }],
         hasNext: false,
       },
     ]);