Skip to content

Commit 7d75d1f

Browse files
yaacovCRyaacovCR
committed
alternative solution to excessive adding of deferred fragments
1 parent e2e1ab6 commit 7d75d1f

File tree

5 files changed

+233
-68
lines changed

5 files changed

+233
-68
lines changed

‎src/execution/IncrementalGraph.ts

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,22 @@ export class IncrementalGraph {
3535
}
3636

3737
getNewRootNodes(
38+
newDeferredFragmentRecords:
39+
| ReadonlyArray<DeferredFragmentRecord>
40+
| undefined,
3841
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
3942
): ReadonlyArray<DeliveryGroup> {
4043
const initialResultChildren = new Set<DeliveryGroup>();
44+
45+
if (newDeferredFragmentRecords !== undefined) {
46+
for (const deferredFragmentRecord of newDeferredFragmentRecords) {
47+
this._addDeferredFragment(
48+
deferredFragmentRecord,
49+
initialResultChildren,
50+
);
51+
}
52+
}
53+
4154
this._addIncrementalDataRecords(
4255
incrementalDataRecords,
4356
undefined,
@@ -49,8 +62,17 @@ export class IncrementalGraph {
4962
addCompletedSuccessfulExecutionGroup(
5063
successfulExecutionGroup: SuccessfulExecutionGroup,
5164
): void {
52-
const { pendingExecutionGroup, incrementalDataRecords } =
53-
successfulExecutionGroup;
65+
const {
66+
pendingExecutionGroup,
67+
newDeferredFragmentRecords,
68+
incrementalDataRecords,
69+
} = successfulExecutionGroup;
70+
71+
if (newDeferredFragmentRecords !== undefined) {
72+
for (const deferredFragmentRecord of newDeferredFragmentRecords) {
73+
this._addDeferredFragment(deferredFragmentRecord, undefined);
74+
}
75+
}
5476

5577
const deferredFragmentRecords =
5678
pendingExecutionGroup.deferredFragmentRecords;
@@ -132,8 +154,10 @@ export class IncrementalGraph {
132154
return { newRootNodes, successfulExecutionGroups };
133155
}
134156

135-
removeDeferredFragment(deferredFragmentRecord: DeferredFragmentRecord): void {
136-
this._rootNodes.delete(deferredFragmentRecord);
157+
removeDeferredFragment(
158+
deferredFragmentRecord: DeferredFragmentRecord,
159+
): boolean {
160+
return this._rootNodes.delete(deferredFragmentRecord);
137161
}
138162

139163
removeStream(streamRecord: StreamRecord): void {
@@ -148,10 +172,6 @@ export class IncrementalGraph {
148172
for (const incrementalDataRecord of incrementalDataRecords) {
149173
if (isPendingExecutionGroup(incrementalDataRecord)) {
150174
for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) {
151-
this._addDeferredFragment(
152-
deferredFragmentRecord,
153-
initialResultChildren,
154-
);
155175
deferredFragmentRecord.pendingExecutionGroups.add(
156176
incrementalDataRecord,
157177
);
@@ -164,7 +184,6 @@ export class IncrementalGraph {
164184
initialResultChildren.add(incrementalDataRecord);
165185
} else {
166186
for (const parent of parents) {
167-
this._addDeferredFragment(parent, initialResultChildren);
168187
parent.children.add(incrementalDataRecord);
169188
}
170189
}
@@ -213,20 +232,13 @@ export class IncrementalGraph {
213232
deferredFragmentRecord: DeferredFragmentRecord,
214233
initialResultChildren: Set<DeliveryGroup> | undefined,
215234
): void {
216-
if (
217-
deferredFragmentRecord.failed ||
218-
this._rootNodes.has(deferredFragmentRecord)
219-
) {
220-
return;
221-
}
222235
const parent = deferredFragmentRecord.parent;
223236
if (parent === undefined) {
224237
invariant(initialResultChildren !== undefined);
225238
initialResultChildren.add(deferredFragmentRecord);
226239
return;
227240
}
228241
parent.children.add(deferredFragmentRecord);
229-
this._addDeferredFragment(parent, initialResultChildren);
230242
}
231243

232244
private _onExecutionGroup(
@@ -248,6 +260,7 @@ export class IncrementalGraph {
248260
private async _onStreamItems(streamRecord: StreamRecord): Promise<void> {
249261
let items: Array<unknown> = [];
250262
let errors: Array<GraphQLError> = [];
263+
let newDeferredFragmentRecords: Array<DeferredFragmentRecord> = [];
251264
let incrementalDataRecords: Array<IncrementalDataRecord> = [];
252265
const streamItemQueue = streamRecord.streamItemQueue;
253266
let streamItemRecord: StreamItemRecord | undefined;
@@ -265,10 +278,12 @@ export class IncrementalGraph {
265278
errors.length > 0 /* c8 ignore start */
266279
? { items, errors } /* c8 ignore stop */
267280
: { items },
281+
newDeferredFragmentRecords,
268282
incrementalDataRecords,
269283
});
270284
items = [];
271285
errors = [];
286+
newDeferredFragmentRecords = [];
272287
incrementalDataRecords = [];
273288
}
274289
// eslint-disable-next-line no-await-in-loop
@@ -283,6 +298,7 @@ export class IncrementalGraph {
283298
this._enqueue({
284299
streamRecord,
285300
result: errors.length > 0 ? { items, errors } : { items },
301+
newDeferredFragmentRecords,
286302
incrementalDataRecords,
287303
});
288304
}
@@ -300,6 +316,9 @@ export class IncrementalGraph {
300316
if (result.errors !== undefined) {
301317
errors.push(...result.errors);
302318
}
319+
if (result.newDeferredFragmentRecords !== undefined) {
320+
newDeferredFragmentRecords.push(...result.newDeferredFragmentRecords);
321+
}
303322
if (result.incrementalDataRecords !== undefined) {
304323
incrementalDataRecords.push(...result.incrementalDataRecords);
305324
}

‎src/execution/IncrementalPublisher.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ export function buildIncrementalResponse(
3333
context: IncrementalPublisherContext,
3434
result: ObjMap<unknown>,
3535
errors: ReadonlyArray<GraphQLError> | undefined,
36+
newDeferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord> | undefined,
3637
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
3738
): ExperimentalIncrementalExecutionResults {
3839
const incrementalPublisher = new IncrementalPublisher(context);
3940
return incrementalPublisher.buildResponse(
4041
result,
4142
errors,
43+
newDeferredFragmentRecords,
4244
incrementalDataRecords,
4345
);
4446
}
@@ -74,9 +76,13 @@ class IncrementalPublisher {
7476
buildResponse(
7577
data: ObjMap<unknown>,
7678
errors: ReadonlyArray<GraphQLError> | undefined,
79+
newDeferredFragmentRecords:
80+
| ReadonlyArray<DeferredFragmentRecord>
81+
| undefined,
7782
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
7883
): ExperimentalIncrementalExecutionResults {
7984
const newRootNodes = this._incrementalGraph.getNewRootNodes(
85+
newDeferredFragmentRecords,
8086
incrementalDataRecords,
8187
);
8288

@@ -228,18 +234,16 @@ class IncrementalPublisher {
228234
if (isFailedExecutionGroup(completedExecutionGroup)) {
229235
for (const deferredFragmentRecord of completedExecutionGroup
230236
.pendingExecutionGroup.deferredFragmentRecords) {
231-
const failed = deferredFragmentRecord.failed;
232-
if (failed) {
233-
continue;
237+
if (
238+
this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord)
239+
) {
240+
const id = deferredFragmentRecord.id;
241+
invariant(id !== undefined);
242+
context.completed.push({
243+
id,
244+
errors: completedExecutionGroup.errors,
245+
});
234246
}
235-
deferredFragmentRecord.failed = true;
236-
const id = deferredFragmentRecord.id;
237-
this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord);
238-
invariant(id !== undefined);
239-
context.completed.push({
240-
id,
241-
errors: completedExecutionGroup.errors,
242-
});
243247
}
244248
return;
245249
}
@@ -316,9 +320,11 @@ class IncrementalPublisher {
316320

317321
context.incremental.push(incrementalEntry);
318322

319-
const incrementalDataRecords = streamItemsResult.incrementalDataRecords;
323+
const { newDeferredFragmentRecords, incrementalDataRecords } =
324+
streamItemsResult;
320325
if (incrementalDataRecords !== undefined) {
321326
const newRootNodes = this._incrementalGraph.getNewRootNodes(
327+
newDeferredFragmentRecords,
322328
incrementalDataRecords,
323329
);
324330
context.pending.push(...this._toPendingResults(newRootNodes));

‎src/execution/__tests__/defer-test.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,26 +1581,26 @@ describe('Execute: defer directive', () => {
15811581
a: {},
15821582
},
15831583
pending: [
1584-
{ id: '0', path: [] },
1585-
{ id: '1', path: ['a'] },
1584+
{ id: '0', path: ['a'] },
1585+
{ id: '1', path: [] },
15861586
],
15871587
hasNext: true,
15881588
},
15891589
{
15901590
incremental: [
15911591
{
15921592
data: { b: { c: {} } },
1593-
id: '1',
1593+
id: '0',
15941594
},
15951595
{
15961596
data: { d: 'd' },
1597-
id: '1',
1597+
id: '0',
15981598
subPath: ['b', 'c'],
15991599
},
16001600
],
16011601
completed: [
16021602
{
1603-
id: '0',
1603+
id: '1',
16041604
errors: [
16051605
{
16061606
message:
@@ -1610,7 +1610,7 @@ describe('Execute: defer directive', () => {
16101610
},
16111611
],
16121612
},
1613-
{ id: '1' },
1613+
{ id: '0' },
16141614
],
16151615
hasNext: false,
16161616
},
@@ -1653,27 +1653,27 @@ describe('Execute: defer directive', () => {
16531653
a: {},
16541654
},
16551655
pending: [
1656-
{ id: '0', path: [] },
1657-
{ id: '1', path: ['a'] },
1656+
{ id: '0', path: ['a'] },
1657+
{ id: '1', path: [] },
16581658
],
16591659
hasNext: true,
16601660
},
16611661
{
16621662
incremental: [
16631663
{
16641664
data: { b: { c: {} } },
1665-
id: '1',
1665+
id: '0',
16661666
},
16671667
{
16681668
data: { d: 'd' },
1669-
id: '0',
1669+
id: '1',
16701670
subPath: ['a', 'b', 'c'],
16711671
},
16721672
],
16731673
completed: [
1674-
{ id: '0' },
1674+
{ id: '1' },
16751675
{
1676-
id: '1',
1676+
id: '0',
16771677
errors: [
16781678
{
16791679
message:
@@ -1727,15 +1727,15 @@ describe('Execute: defer directive', () => {
17271727
a: {},
17281728
},
17291729
pending: [
1730-
{ id: '0', path: [] },
1731-
{ id: '1', path: ['a'] },
1730+
{ id: '0', path: ['a'] },
1731+
{ id: '1', path: [] },
17321732
],
17331733
hasNext: true,
17341734
},
17351735
{
17361736
completed: [
17371737
{
1738-
id: '0',
1738+
id: '1',
17391739
errors: [
17401740
{
17411741
message:
@@ -1752,15 +1752,15 @@ describe('Execute: defer directive', () => {
17521752
incremental: [
17531753
{
17541754
data: { b: {}, someField: 'someField' },
1755-
id: '1',
1755+
id: '0',
17561756
},
17571757
{
17581758
data: { e: { f: 'f' } },
1759-
id: '1',
1759+
id: '0',
17601760
subPath: ['b'],
17611761
},
17621762
],
1763-
completed: [{ id: '1' }],
1763+
completed: [{ id: '0' }],
17641764
hasNext: false,
17651765
},
17661766
]);
@@ -1960,30 +1960,30 @@ describe('Execute: defer directive', () => {
19601960
a: {},
19611961
},
19621962
pending: [
1963-
{ id: '0', path: [] },
1964-
{ id: '1', path: ['a'] },
1963+
{ id: '0', path: ['a'] },
1964+
{ id: '1', path: [] },
19651965
],
19661966
hasNext: true,
19671967
},
19681968
{
19691969
incremental: [
19701970
{
19711971
data: { b: { c: {} } },
1972-
id: '1',
1972+
id: '0',
19731973
},
19741974
{
19751975
data: { d: 'd' },
1976-
id: '1',
1976+
id: '0',
19771977
subPath: ['b', 'c'],
19781978
},
19791979
],
1980-
completed: [{ id: '1' }],
1980+
completed: [{ id: '0' }],
19811981
hasNext: true,
19821982
},
19831983
{
19841984
completed: [
19851985
{
1986-
id: '0',
1986+
id: '1',
19871987
errors: [
19881988
{
19891989
message:

0 commit comments

Comments
 (0)