Skip to content

Commit ba99adb

Browse files
committed
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
1 parent 435169c commit ba99adb

File tree

4 files changed

+180
-23
lines changed

4 files changed

+180
-23
lines changed

src/execution/IncrementalGraph.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { GraphQLError } from '../error/GraphQLError.js';
88
import type {
99
DeferredFragmentRecord,
1010
DeferredGroupedFieldSetRecord,
11+
DeferredGroupedFieldSetResult,
1112
IncrementalDataRecord,
1213
IncrementalDataRecordResult,
1314
ReconcilableDeferredGroupedFieldSetResult,
@@ -113,12 +114,10 @@ export class IncrementalGraph {
113114
reconcilableResults: ReadonlyArray<ReconcilableDeferredGroupedFieldSetResult>;
114115
}
115116
| undefined {
116-
// TODO: add test case?
117-
/* c8 ignore next 3 */
118-
if (!this._rootNodes.has(deferredFragmentRecord)) {
119-
return;
120-
}
121-
if (deferredFragmentRecord.deferredGroupedFieldSetRecords.size > 0) {
117+
if (
118+
!this._rootNodes.has(deferredFragmentRecord) ||
119+
deferredFragmentRecord.deferredGroupedFieldSetRecords.size > 0
120+
) {
122121
return;
123122
}
124123
const reconcilableResults = Array.from(
@@ -202,6 +201,7 @@ export class IncrementalGraph {
202201
for (const subsequentResultRecord of maybeEmptyNewPending) {
203202
if (isDeferredFragmentRecord(subsequentResultRecord)) {
204203
if (subsequentResultRecord.deferredGroupedFieldSetRecords.size > 0) {
204+
subsequentResultRecord.setAsPending();
205205
for (const deferredGroupedFieldSetRecord of subsequentResultRecord.deferredGroupedFieldSetRecords) {
206206
if (!this._completesRootNode(deferredGroupedFieldSetRecord)) {
207207
this._onDeferredGroupedFieldSet(deferredGroupedFieldSetRecord);
@@ -253,12 +253,9 @@ export class IncrementalGraph {
253253
private _onDeferredGroupedFieldSet(
254254
deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord,
255255
): void {
256-
const deferredGroupedFieldSetResult = deferredGroupedFieldSetRecord.result;
257-
const result =
258-
deferredGroupedFieldSetResult instanceof BoxedPromiseOrValue
259-
? deferredGroupedFieldSetResult.value
260-
: deferredGroupedFieldSetResult().value;
261-
256+
const result = (
257+
deferredGroupedFieldSetRecord.result as BoxedPromiseOrValue<DeferredGroupedFieldSetResult>
258+
).value;
262259
if (isPromise(result)) {
263260
// eslint-disable-next-line @typescript-eslint/no-floating-promises
264261
result.then((resolved) => this._enqueue(resolved));

src/execution/__tests__/defer-test.ts

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
import { expect } from 'chai';
1+
import { assert, expect } from 'chai';
22
import { describe, it } from 'mocha';
33

44
import { expectJSON } from '../../__testUtils__/expectJSON.js';
55
import { expectPromise } from '../../__testUtils__/expectPromise.js';
66
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
77

8+
import { promiseWithResolvers } from '../../jsutils/promiseWithResolvers.js';
9+
810
import type { DocumentNode } from '../../language/ast.js';
911
import { parse } from '../../language/parser.js';
1012

@@ -856,6 +858,134 @@ describe('Execute: defer directive', () => {
856858
]);
857859
});
858860

861+
it('Initiates all deferred grouped field sets immediately if and only if they have been released as pending', async () => {
862+
const document = parse(`
863+
query {
864+
... @defer {
865+
a {
866+
... @defer {
867+
b {
868+
c { d }
869+
}
870+
}
871+
}
872+
}
873+
... @defer {
874+
a {
875+
someField
876+
... @defer {
877+
b {
878+
e { f }
879+
}
880+
}
881+
}
882+
}
883+
}
884+
`);
885+
886+
const { promise: slowFieldPromise, resolve: resolveSlowField } =
887+
promiseWithResolvers();
888+
let cResolverCalled = false;
889+
let eResolverCalled = false;
890+
const executeResult = experimentalExecuteIncrementally({
891+
schema,
892+
document,
893+
rootValue: {
894+
a: {
895+
someField: slowFieldPromise,
896+
b: {
897+
c: () => {
898+
cResolverCalled = true;
899+
return { d: 'd' };
900+
},
901+
e: () => {
902+
eResolverCalled = true;
903+
return { f: 'f' };
904+
},
905+
},
906+
},
907+
},
908+
enableEarlyExecution: false,
909+
});
910+
911+
assert('initialResult' in executeResult);
912+
913+
const result1 = executeResult.initialResult;
914+
expectJSON(result1).toDeepEqual({
915+
data: {},
916+
pending: [
917+
{ id: '0', path: [] },
918+
{ id: '1', path: [] },
919+
],
920+
hasNext: true,
921+
});
922+
923+
const iterator = executeResult.subsequentResults[Symbol.asyncIterator]();
924+
925+
expect(cResolverCalled).to.equal(false);
926+
expect(eResolverCalled).to.equal(false);
927+
928+
const result2 = await iterator.next();
929+
expectJSON(result2).toDeepEqual({
930+
value: {
931+
pending: [{ id: '2', path: ['a'] }],
932+
incremental: [
933+
{
934+
data: { a: {} },
935+
id: '0',
936+
},
937+
{
938+
data: { b: {} },
939+
id: '2',
940+
},
941+
{
942+
data: { c: { d: 'd' } },
943+
id: '2',
944+
subPath: ['b'],
945+
},
946+
],
947+
completed: [{ id: '0' }, { id: '2' }],
948+
hasNext: true,
949+
},
950+
done: false,
951+
});
952+
953+
expect(cResolverCalled).to.equal(true);
954+
expect(eResolverCalled).to.equal(false);
955+
956+
resolveSlowField('someField');
957+
958+
const result3 = await iterator.next();
959+
expectJSON(result3).toDeepEqual({
960+
value: {
961+
pending: [{ id: '3', path: ['a'] }],
962+
incremental: [
963+
{
964+
data: { someField: 'someField' },
965+
id: '1',
966+
subPath: ['a'],
967+
},
968+
{
969+
data: { e: { f: 'f' } },
970+
id: '3',
971+
subPath: ['b'],
972+
},
973+
],
974+
completed: [{ id: '1' }, { id: '3' }],
975+
hasNext: false,
976+
},
977+
done: false,
978+
});
979+
980+
expect(eResolverCalled).to.equal(true);
981+
982+
const result4 = await iterator.next();
983+
expectJSON(result4).toDeepEqual({
984+
value: undefined,
985+
done: true,
986+
});
987+
});
988+
859989
it('Can deduplicate multiple defers on the same object', async () => {
860990
const document = parse(`
861991
query {

src/execution/execute.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,16 +2114,25 @@ function executeDeferredGroupedFieldSets(
21142114
deferMap,
21152115
);
21162116

2117-
const shouldDeferThisDeferUsageSet = shouldDefer(
2118-
parentDeferUsages,
2119-
deferUsageSet,
2120-
);
2121-
2122-
deferredGroupedFieldSetRecord.result = shouldDeferThisDeferUsageSet
2123-
? exeContext.enableEarlyExecution
2124-
? new BoxedPromiseOrValue(Promise.resolve().then(executor))
2125-
: () => new BoxedPromiseOrValue(executor())
2126-
: new BoxedPromiseOrValue(executor());
2117+
if (exeContext.enableEarlyExecution) {
2118+
deferredGroupedFieldSetRecord.result = new BoxedPromiseOrValue(
2119+
shouldDefer(parentDeferUsages, deferUsageSet)
2120+
? Promise.resolve().then(executor)
2121+
: executor(),
2122+
);
2123+
} else {
2124+
deferredGroupedFieldSetRecord.result = () =>
2125+
new BoxedPromiseOrValue(executor());
2126+
const resolveThunk = () => {
2127+
const maybeThunk = deferredGroupedFieldSetRecord.result;
2128+
if (!(maybeThunk instanceof BoxedPromiseOrValue)) {
2129+
deferredGroupedFieldSetRecord.result = maybeThunk();
2130+
}
2131+
};
2132+
for (const deferredFragmentRecord of deferredFragmentRecords) {
2133+
deferredFragmentRecord.onPending(resolveThunk);
2134+
}
2135+
}
21272136

21282137
newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord);
21292138
}

src/execution/types.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,9 @@ export class DeferredFragmentRecord {
224224
reconcilableResults: Set<ReconcilableDeferredGroupedFieldSetResult>;
225225
children: Set<SubsequentResultRecord>;
226226

227+
private pending: boolean;
228+
private fns: Array<() => void>;
229+
227230
constructor(
228231
path: Path | undefined,
229232
label: string | undefined,
@@ -235,6 +238,24 @@ export class DeferredFragmentRecord {
235238
this.deferredGroupedFieldSetRecords = new Set();
236239
this.reconcilableResults = new Set();
237240
this.children = new Set();
241+
this.pending = false;
242+
this.fns = [];
243+
}
244+
245+
onPending(fn: () => void): void {
246+
if (this.pending) {
247+
fn();
248+
} else {
249+
this.fns.push(fn);
250+
}
251+
}
252+
253+
setAsPending(): void {
254+
this.pending = true;
255+
let fn;
256+
while ((fn = this.fns.shift()) !== undefined) {
257+
fn();
258+
}
238259
}
239260
}
240261

0 commit comments

Comments
 (0)