fix(incremental): properly initiate nested deferred grouped field sets

when early execution is disabled, deferred grouped field sets should start immediately if and only if one of their deferred fragments is released as pending

see: graphql/defer-stream-wg#84 (reply in thread)

Author: yaacovCR

src/execution/IncrementalGraph.ts

@@ -8,6 +8,7 @@ import type { GraphQLError } from '../error/GraphQLError.js';
 import type {
   DeferredFragmentRecord,
   DeferredGroupedFieldSetRecord,
+  DeferredGroupedFieldSetResult,
   IncrementalDataRecord,
   IncrementalDataRecordResult,
   ReconcilableDeferredGroupedFieldSetResult,
@@ -237,6 +238,7 @@ export class IncrementalGraph {
     for (const node of newPendingNodes) {
       if (isDeferredFragmentNode(node)) {
         if (node.deferredGroupedFieldSetRecords.size > 0) {
+          node.deferredFragmentRecord.setAsPending();
           for (const deferredGroupedFieldSetRecord of node.deferredGroupedFieldSetRecords) {
             if (!this._hasPendingFragment(deferredGroupedFieldSetRecord)) {
               this._onDeferredGroupedFieldSet(deferredGroupedFieldSetRecord);
@@ -313,12 +315,9 @@ export class IncrementalGraph {
   private _onDeferredGroupedFieldSet(
     deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord,
   ): void {
-    const deferredGroupedFieldSetResult = deferredGroupedFieldSetRecord.result;
-    const result =
-      deferredGroupedFieldSetResult instanceof BoxedPromiseOrValue
-        ? deferredGroupedFieldSetResult.value
-        : deferredGroupedFieldSetResult().value;
-
+    const result = (
+      deferredGroupedFieldSetRecord.result as BoxedPromiseOrValue<DeferredGroupedFieldSetResult>
+    ).value;
     if (isPromise(result)) {
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       result.then((resolved) => this._enqueue(resolved));

src/execution/__tests__/defer-test.ts

@@ -1,10 +1,12 @@
-import { expect } from 'chai';
+import { assert, expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { expectJSON } from '../../__testUtils__/expectJSON.js';
 import { expectPromise } from '../../__testUtils__/expectPromise.js';
 import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
 
+import { promiseWithResolvers } from '../../jsutils/promiseWithResolvers.js';
+
 import type { DocumentNode } from '../../language/ast.js';
 import { parse } from '../../language/parser.js';
 
@@ -856,6 +858,134 @@ describe('Execute: defer directive', () => {
     ]);
   });
 
+  it('Initiates all deferred grouped field sets immediately if and only if they have been released as pending', async () => {
+    const document = parse(`
+      query {
+        ... @defer {
+          a {
+            ... @defer {
+              b {
+                c { d }
+              }
+            }
+          }
+        }
+        ... @defer {
+          a {
+            someField
+            ... @defer {
+              b {
+                e { f }
+              }
+            }
+          }
+        }
+      }
+    `);
+
+    const { promise: slowFieldPromise, resolve: resolveSlowField } =
+      promiseWithResolvers();
+    let cResolverCalled = false;
+    let eResolverCalled = false;
+    const executeResult = experimentalExecuteIncrementally({
+      schema,
+      document,
+      rootValue: {
+        a: {
+          someField: slowFieldPromise,
+          b: {
+            c: () => {
+              cResolverCalled = true;
+              return { d: 'd' };
+            },
+            e: () => {
+              eResolverCalled = true;
+              return { f: 'f' };
+            },
+          },
+        },
+      },
+      enableEarlyExecution: false,
+    });
+
+    assert('initialResult' in executeResult);
+
+    const result1 = executeResult.initialResult;
+    expectJSON(result1).toDeepEqual({
+      data: {},
+      pending: [
+        { id: '0', path: [] },
+        { id: '1', path: [] },
+      ],
+      hasNext: true,
+    });
+
+    const iterator = executeResult.subsequentResults[Symbol.asyncIterator]();
+
+    expect(cResolverCalled).to.equal(false);
+    expect(eResolverCalled).to.equal(false);
+
+    const result2 = await iterator.next();
+    expectJSON(result2).toDeepEqual({
+      value: {
+        pending: [{ id: '2', path: ['a'] }],
+        incremental: [
+          {
+            data: { a: {} },
+            id: '0',
+          },
+          {
+            data: { b: {} },
+            id: '2',
+          },
+          {
+            data: { c: { d: 'd' } },
+            id: '2',
+            subPath: ['b'],
+          },
+        ],
+        completed: [{ id: '0' }, { id: '2' }],
+        hasNext: true,
+      },
+      done: false,
+    });
+
+    expect(cResolverCalled).to.equal(true);
+    expect(eResolverCalled).to.equal(false);
+
+    resolveSlowField('someField');
+
+    const result3 = await iterator.next();
+    expectJSON(result3).toDeepEqual({
+      value: {
+        pending: [{ id: '3', path: ['a'] }],
+        incremental: [
+          {
+            data: { someField: 'someField' },
+            id: '1',
+            subPath: ['a'],
+          },
+          {
+            data: { e: { f: 'f' } },
+            id: '3',
+            subPath: ['b'],
+          },
+        ],
+        completed: [{ id: '1' }, { id: '3' }],
+        hasNext: false,
+      },
+      done: false,
+    });
+
+    expect(eResolverCalled).to.equal(true);
+
+    const result4 = await iterator.next();
+    expectJSON(result4).toDeepEqual({
+      value: undefined,
+      done: true,
+    });
+  });
+
   it('Can deduplicate multiple defers on the same object', async () => {
     const document = parse(`
       query {

src/execution/execute.ts

@@ -63,7 +63,6 @@ import { buildIncrementalResponse } from './IncrementalPublisher.js';
 import { mapAsyncIterable } from './mapAsyncIterable.js';
 import type {
   CancellableStreamRecord,
-  DeferredFragmentRecord,
   DeferredGroupedFieldSetRecord,
   DeferredGroupedFieldSetResult,
   ExecutionResult,
@@ -73,6 +72,7 @@ import type {
   StreamItemResult,
   StreamRecord,
 } from './types.js';
+import { DeferredFragmentRecord } from './types.js';
 import {
   getArgumentValues,
   getDirectiveValues,
@@ -1676,11 +1676,11 @@ function addNewDeferredFragments(
         : deferredFragmentRecordFromDeferUsage(parentDeferUsage, newDeferMap);
 
     // Instantiate the new record.
-    const deferredFragmentRecord: DeferredFragmentRecord = {
+    const deferredFragmentRecord = new DeferredFragmentRecord(
       path,
-      label: newDeferUsage.label,
+      newDeferUsage.label,
       parent,
-    };
+    );
 
     // Update the map.
     newDeferMap.set(newDeferUsage, deferredFragmentRecord);
@@ -2114,16 +2114,33 @@ function executeDeferredGroupedFieldSets(
         deferMap,
       );
 
-    const shouldDeferThisDeferUsageSet = shouldDefer(
-      parentDeferUsages,
-      deferUsageSet,
-    );
-
-    deferredGroupedFieldSetRecord.result = shouldDeferThisDeferUsageSet
-      ? exeContext.enableEarlyExecution
-        ? new BoxedPromiseOrValue(Promise.resolve().then(executor))
-        : () => new BoxedPromiseOrValue(executor())
-      : new BoxedPromiseOrValue(executor());
+    if (exeContext.enableEarlyExecution) {
+      deferredGroupedFieldSetRecord.result = new BoxedPromiseOrValue(
+        shouldDefer(parentDeferUsages, deferUsageSet)
+          ? Promise.resolve().then(executor)
+          : executor(),
+      );
+    } else if (
+      deferredFragmentRecords.some(
+        (deferredFragmentRecord) => deferredFragmentRecord.pending,
+      )
+    ) {
+      deferredGroupedFieldSetRecord.result = new BoxedPromiseOrValue(
+        executor(),
+      );
+    } else {
+      deferredGroupedFieldSetRecord.result = () =>
+        new BoxedPromiseOrValue(executor());
+      const resolveThunk = () => {
+        const maybeThunk = deferredGroupedFieldSetRecord.result;
+        if (!(maybeThunk instanceof BoxedPromiseOrValue)) {
+          deferredGroupedFieldSetRecord.result = maybeThunk();
+        }
+      };
+      for (const deferredFragmentRecord of deferredFragmentRecords) {
+        deferredFragmentRecord.onPending(resolveThunk);
+      }
+    }
 
     newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord);
   }

src/execution/types.ts

@@ -214,10 +214,36 @@ export interface DeferredGroupedFieldSetRecord {
 
 export type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord;
 
-export interface DeferredFragmentRecord {
+/** @internal */
+export class DeferredFragmentRecord {
   path: Path | undefined;
   label: string | undefined;
   parent: DeferredFragmentRecord | undefined;
+  fns: Array<() => void>;
+  pending: boolean;
+
+  constructor(
+    path: Path | undefined,
+    label: string | undefined,
+    parent: DeferredFragmentRecord | undefined,
+  ) {
+    this.path = path;
+    this.label = label;
+    this.parent = parent;
+    this.pending = false;
+    this.fns = [];
+  }
+
+  onPending(fn: () => void): void {
+    this.fns.push(fn);
+  }
+
+  setAsPending(): void {
+    this.pending = true;
+    for (const fn of this.fns) {
+      fn();
+    }
+  }
 }
 
 export interface StreamItemResult {