fix(incremental): fix bug on initiation of supplemental deferred grou…

…ped 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)

src/execution/IncrementalGraph.ts

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

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,31 @@ function executeDeferredGroupedFieldSets(
         deferMap,
       );
 
-    const shouldDeferThisDeferUsageSet = shouldDefer(
-      parentDeferUsages,
-      deferUsageSet,
-    );
-
-    deferredGroupedFieldSetRecord.result = shouldDeferThisDeferUsageSet
-      ? exeContext.enableEarlyExecution
+    if (exeContext.enableEarlyExecution) {
+      deferredGroupedFieldSetRecord.result = shouldDefer(
+        parentDeferUsages,
+        deferUsageSet,
+      )
         ? new BoxedPromiseOrValue(Promise.resolve().then(executor))
-        : () => new BoxedPromiseOrValue(executor())
-      : new BoxedPromiseOrValue(executor());
+        : new BoxedPromiseOrValue(executor());
+    } else if (
+      deferredFragmentRecords.some(
+        (deferredFragmentRecord) => deferredFragmentRecord.pending,
+      )
+    ) {
+      deferredGroupedFieldSetRecord.result = new BoxedPromiseOrValue(
+        executor(),
+      );
+    } else {
+      const resolve = () => {
+        deferredGroupedFieldSetRecord.result = new BoxedPromiseOrValue(
+          executor(),
+        );
+      };
+      for (const deferredFragmentRecord of deferredFragmentRecords) {
+        deferredFragmentRecord.onPending(resolve);
+      }
+    }
 
     newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord);
   }

src/execution/types.ts

@@ -203,21 +203,43 @@ export function isNonReconcilableDeferredGroupedFieldSetResult(
   return deferredGroupedFieldSetResult.errors !== undefined;
 }
 
-type ThunkIncrementalResult<T> =
-  | BoxedPromiseOrValue<T>
-  | (() => BoxedPromiseOrValue<T>);
-
 export interface DeferredGroupedFieldSetRecord {
   deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
-  result: ThunkIncrementalResult<DeferredGroupedFieldSetResult>;
+  result: BoxedPromiseOrValue<DeferredGroupedFieldSetResult>;
 }
 
 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 {
@@ -226,6 +248,10 @@ export interface StreamItemResult {
   errors?: ReadonlyArray<GraphQLError> | undefined;
 }
 
+type ThunkIncrementalResult<T> =
+  | BoxedPromiseOrValue<T>
+  | (() => BoxedPromiseOrValue<T>);
+
 export type StreamItemRecord = ThunkIncrementalResult<StreamItemResult>;
 
 export interface StreamRecord {