From c75866fbfbf102ef7dd73732d45783aa1a84f078 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 16 Jun 2024 17:34:44 +0300 Subject: [PATCH 1/7] polish(incremental): refactor getNewPending return newPending alongside incremental results --- src/execution/IncrementalGraph.ts | 272 ++++++++++++++------------ src/execution/IncrementalPublisher.ts | 30 ++- 2 files changed, 166 insertions(+), 136 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index cf5e95c285..3e55b3587a 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -1,4 +1,5 @@ import { BoxedPromiseOrValue } from '../jsutils/BoxedPromiseOrValue.js'; +import { invariant } from '../jsutils/invariant.js'; import { isPromise } from '../jsutils/isPromise.js'; import { promiseWithResolvers } from '../jsutils/promiseWithResolvers.js'; @@ -20,19 +21,13 @@ interface DeferredFragmentNode { deferredFragmentRecord: DeferredFragmentRecord; deferredGroupedFieldSetRecords: Set; reconcilableResults: Set; - children: Array; + children: Set; } function isDeferredFragmentNode( - node: DeferredFragmentNode | undefined, + node: SubsequentResultNode | undefined, ): node is DeferredFragmentNode { - return node !== undefined; -} - -function isStreamNode( - record: SubsequentResultNode | IncrementalDataRecord, -): record is StreamRecord { - return 'streamItemQueue' in record; + return node !== undefined && 'deferredFragmentRecord' in node; } type SubsequentResultNode = DeferredFragmentNode | StreamRecord; @@ -41,103 +36,56 @@ type SubsequentResultNode = DeferredFragmentNode | StreamRecord; * @internal */ export class IncrementalGraph { - private _pending: Set; + private _rootNodes: Set; private _deferredFragmentNodes: Map< DeferredFragmentRecord, DeferredFragmentNode >; - private _newPending: Set; - private _newIncrementalDataRecords: Set; private _completedQueue: Array; private _nextQueue: Array< (iterable: IteratorResult>) => void >; constructor() { - this._pending = new Set(); + this._rootNodes = new Set(); this._deferredFragmentNodes = new Map(); - this._newIncrementalDataRecords = new Set(); - this._newPending = new Set(); this._completedQueue = []; this._nextQueue = []; } - addIncrementalDataRecords( + getNewPending( incrementalDataRecords: ReadonlyArray, - ): void { - for (const incrementalDataRecord of incrementalDataRecords) { - if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { - this._addDeferredGroupedFieldSetRecord(incrementalDataRecord); - } else { - this._addStreamRecord(incrementalDataRecord); - } - } + ): ReadonlyArray { + const initialResultChildren = new Set(); + this._addIncrementalDataRecords( + incrementalDataRecords, + undefined, + initialResultChildren, + ); + return this._promoteNonEmptyToRoot(initialResultChildren); } addCompletedReconcilableDeferredGroupedFieldSet( reconcilableResult: ReconcilableDeferredGroupedFieldSetResult, ): void { - const deferredFragmentNodes: Array = - reconcilableResult.deferredGroupedFieldSetRecord.deferredFragmentRecords - .map((deferredFragmentRecord) => - this._deferredFragmentNodes.get(deferredFragmentRecord), - ) - .filter(isDeferredFragmentNode); - for (const deferredFragmentNode of deferredFragmentNodes) { + for (const deferredFragmentNode of this._fragmentsToNodes( + reconcilableResult.deferredGroupedFieldSetRecord.deferredFragmentRecords, + )) { deferredFragmentNode.deferredGroupedFieldSetRecords.delete( reconcilableResult.deferredGroupedFieldSetRecord, ); deferredFragmentNode.reconcilableResults.add(reconcilableResult); } - } - getNewPending(): ReadonlyArray { - const newPending: Array = []; - for (const node of this._newPending) { - if (isStreamNode(node)) { - this._pending.add(node); - newPending.push(node); - this._newIncrementalDataRecords.add(node); - } else if (node.deferredGroupedFieldSetRecords.size > 0) { - for (const deferredGroupedFieldSetNode of node.deferredGroupedFieldSetRecords) { - this._newIncrementalDataRecords.add(deferredGroupedFieldSetNode); - } - this._pending.add(node); - newPending.push(node.deferredFragmentRecord); - } else { - for (const child of node.children) { - this._newPending.add(child); - } - } - } - this._newPending.clear(); - - for (const incrementalDataRecord of this._newIncrementalDataRecords) { - if (isStreamNode(incrementalDataRecord)) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this._onStreamItems( - incrementalDataRecord, - incrementalDataRecord.streamItemQueue, - ); - } else { - const deferredGroupedFieldSetResult = incrementalDataRecord.result; - const result = - deferredGroupedFieldSetResult instanceof BoxedPromiseOrValue - ? deferredGroupedFieldSetResult.value - : deferredGroupedFieldSetResult().value; - - if (isPromise(result)) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - result.then((resolved) => this._enqueue(resolved)); - } else { - this._enqueue(result); - } - } + const incrementalDataRecords = reconcilableResult.incrementalDataRecords; + if (incrementalDataRecords !== undefined) { + this._addIncrementalDataRecords( + incrementalDataRecords, + reconcilableResult.deferredGroupedFieldSetRecord + .deferredFragmentRecords, + ); } - this._newIncrementalDataRecords.clear(); - - return newPending; } completedIncrementalData() { @@ -174,19 +122,22 @@ export class IncrementalGraph { } hasNext(): boolean { - return this._pending.size > 0; + return this._rootNodes.size > 0; } - completeDeferredFragment( - deferredFragmentRecord: DeferredFragmentRecord, - ): Array | undefined { + completeDeferredFragment(deferredFragmentRecord: DeferredFragmentRecord): + | { + newPending: ReadonlyArray; + reconcilableResults: ReadonlyArray; + } + | undefined { const deferredFragmentNode = this._deferredFragmentNodes.get( deferredFragmentRecord, ); // TODO: add test case? /* c8 ignore next 3 */ if (deferredFragmentNode === undefined) { - return undefined; + return; } if (deferredFragmentNode.deferredGroupedFieldSetRecords.size > 0) { return; @@ -194,25 +145,21 @@ export class IncrementalGraph { const reconcilableResults = Array.from( deferredFragmentNode.reconcilableResults, ); + this._removePending(deferredFragmentNode); for (const reconcilableResult of reconcilableResults) { - for (const otherDeferredFragmentRecord of reconcilableResult - .deferredGroupedFieldSetRecord.deferredFragmentRecords) { - const otherDeferredFragmentNode = this._deferredFragmentNodes.get( - otherDeferredFragmentRecord, - ); - if (otherDeferredFragmentNode === undefined) { - continue; - } + for (const otherDeferredFragmentNode of this._fragmentsToNodes( + reconcilableResult.deferredGroupedFieldSetRecord + .deferredFragmentRecords, + )) { otherDeferredFragmentNode.reconcilableResults.delete( reconcilableResult, ); } } - this._removePending(deferredFragmentNode); - for (const child of deferredFragmentNode.children) { - this._newPending.add(child); - } - return reconcilableResults; + const newPending = this._promoteNonEmptyToRoot( + deferredFragmentNode.children, + ); + return { newPending, reconcilableResults }; } removeDeferredFragment( @@ -227,9 +174,11 @@ export class IncrementalGraph { this._removePending(deferredFragmentNode); this._deferredFragmentNodes.delete(deferredFragmentRecord); // TODO: add test case for an erroring deferred fragment with child defers - /* c8 ignore next 3 */ + /* c8 ignore next 5 */ for (const child of deferredFragmentNode.children) { - this.removeDeferredFragment(child.deferredFragmentRecord); + if (isDeferredFragmentNode(child)) { + this.removeDeferredFragment(child.deferredFragmentRecord); + } } return true; } @@ -239,36 +188,100 @@ export class IncrementalGraph { } private _removePending(subsequentResultNode: SubsequentResultNode): void { - this._pending.delete(subsequentResultNode); - if (this._pending.size === 0) { + this._rootNodes.delete(subsequentResultNode); + if (this._rootNodes.size === 0) { for (const resolve of this._nextQueue) { resolve({ value: undefined, done: true }); } } } - private _addDeferredGroupedFieldSetRecord( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, + private _addIncrementalDataRecords( + incrementalDataRecords: ReadonlyArray, + parents: ReadonlyArray | undefined, + initialResultChildren?: Set | undefined, ): void { - for (const deferredFragmentRecord of deferredGroupedFieldSetRecord.deferredFragmentRecords) { - const deferredFragmentNode = this._addDeferredFragmentNode( - deferredFragmentRecord, - ); - if (this._pending.has(deferredFragmentNode)) { - this._newIncrementalDataRecords.add(deferredGroupedFieldSetRecord); + for (const incrementalDataRecord of incrementalDataRecords) { + if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { + for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) { + const deferredFragmentNode = this._addDeferredFragmentNode( + deferredFragmentRecord, + initialResultChildren, + ); + deferredFragmentNode.deferredGroupedFieldSetRecords.add( + incrementalDataRecord, + ); + } + if (this._hasPendingFragment(incrementalDataRecord)) { + this._onDeferredGroupedFieldSet(incrementalDataRecord); + } + } else if (parents === undefined) { + invariant(initialResultChildren !== undefined); + initialResultChildren.add(incrementalDataRecord); + } else { + for (const parent of parents) { + const deferredFragmentNode = this._addDeferredFragmentNode( + parent, + initialResultChildren, + ); + deferredFragmentNode.children.add(incrementalDataRecord); + } } - deferredFragmentNode.deferredGroupedFieldSetRecords.add( - deferredGroupedFieldSetRecord, - ); } } - private _addStreamRecord(streamRecord: StreamRecord): void { - this._newPending.add(streamRecord); + private _promoteNonEmptyToRoot( + newPendingNodes: Set, + ): ReadonlyArray { + const newPendingResults: Array = []; + for (const node of newPendingNodes) { + if (isDeferredFragmentNode(node)) { + if (node.deferredGroupedFieldSetRecords.size > 0) { + for (const deferredGroupedFieldSetRecord of node.deferredGroupedFieldSetRecords) { + if (!this._hasPendingFragment(deferredGroupedFieldSetRecord)) { + this._onDeferredGroupedFieldSet(deferredGroupedFieldSetRecord); + } + } + this._rootNodes.add(node); + newPendingResults.push(node.deferredFragmentRecord); + continue; + } + this._deferredFragmentNodes.delete(node.deferredFragmentRecord); + for (const child of node.children) { + newPendingNodes.add(child); + } + } else { + this._rootNodes.add(node); + newPendingResults.push(node); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._onStreamItems(node); + } + } + return newPendingResults; + } + + private _hasPendingFragment( + deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, + ): boolean { + return this._fragmentsToNodes( + deferredGroupedFieldSetRecord.deferredFragmentRecords, + ).some((node) => this._rootNodes.has(node)); + } + + private _fragmentsToNodes( + deferredFragmentRecords: ReadonlyArray, + ): Array { + return deferredFragmentRecords + .map((deferredFragmentRecord) => + this._deferredFragmentNodes.get(deferredFragmentRecord), + ) + .filter(isDeferredFragmentNode); } private _addDeferredFragmentNode( deferredFragmentRecord: DeferredFragmentRecord, + initialResultChildren: Set | undefined, ): DeferredFragmentNode { let deferredFragmentNode = this._deferredFragmentNodes.get( deferredFragmentRecord, @@ -280,7 +293,7 @@ export class IncrementalGraph { deferredFragmentRecord, deferredGroupedFieldSetRecords: new Set(), reconcilableResults: new Set(), - children: [], + children: new Set(), }; this._deferredFragmentNodes.set( deferredFragmentRecord, @@ -288,21 +301,40 @@ export class IncrementalGraph { ); const parent = deferredFragmentRecord.parent; if (parent === undefined) { - this._newPending.add(deferredFragmentNode); + invariant(initialResultChildren !== undefined); + initialResultChildren.add(deferredFragmentNode); return deferredFragmentNode; } - const parentNode = this._addDeferredFragmentNode(parent); - parentNode.children.push(deferredFragmentNode); + const parentNode = this._addDeferredFragmentNode( + parent, + initialResultChildren, + ); + parentNode.children.add(deferredFragmentNode); return deferredFragmentNode; } - private async _onStreamItems( - streamRecord: StreamRecord, - streamItemQueue: Array, - ): Promise { + private _onDeferredGroupedFieldSet( + deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, + ): void { + const deferredGroupedFieldSetResult = deferredGroupedFieldSetRecord.result; + const result = + deferredGroupedFieldSetResult instanceof BoxedPromiseOrValue + ? deferredGroupedFieldSetResult.value + : deferredGroupedFieldSetResult().value; + + if (isPromise(result)) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + result.then((resolved) => this._enqueue(resolved)); + } else { + this._enqueue(result); + } + } + + private async _onStreamItems(streamRecord: StreamRecord): Promise { let items: Array = []; let errors: Array = []; let incrementalDataRecords: Array = []; + const streamItemQueue = streamRecord.streamItemQueue; let streamItemRecord: StreamItemRecord | undefined; while ((streamItemRecord = streamItemQueue.shift()) !== undefined) { let result = diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index b453bde0d3..ba07c7e413 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -74,8 +74,9 @@ class IncrementalPublisher { errors: ReadonlyArray | undefined, incrementalDataRecords: ReadonlyArray, ): ExperimentalIncrementalExecutionResults { - this._incrementalGraph.addIncrementalDataRecords(incrementalDataRecords); - const newPending = this._incrementalGraph.getNewPending(); + const newPending = this._incrementalGraph.getNewPending( + incrementalDataRecords, + ); const pending = this._pendingSourcesToResults(newPending); @@ -217,8 +218,6 @@ class IncrementalPublisher { } else { this._handleCompletedStreamItems(completedIncrementalData, context); } - const newPending = this._incrementalGraph.getNewPending(); - context.pending.push(...this._pendingSourcesToResults(newPending)); } private _handleCompletedDeferredGroupedFieldSet( @@ -252,22 +251,19 @@ class IncrementalPublisher { deferredGroupedFieldSetResult, ); - const incrementalDataRecords = - deferredGroupedFieldSetResult.incrementalDataRecords; - if (incrementalDataRecords !== undefined) { - this._incrementalGraph.addIncrementalDataRecords(incrementalDataRecords); - } - for (const deferredFragmentRecord of deferredGroupedFieldSetResult .deferredGroupedFieldSetRecord.deferredFragmentRecords) { - const reconcilableResults = - this._incrementalGraph.completeDeferredFragment(deferredFragmentRecord); - if (reconcilableResults === undefined) { + const completion = this._incrementalGraph.completeDeferredFragment( + deferredFragmentRecord, + ); + if (completion === undefined) { continue; } const id = deferredFragmentRecord.id; invariant(id !== undefined); const incremental = context.incremental; + const { newPending, reconcilableResults } = completion; + context.pending.push(...this._pendingSourcesToResults(newPending)); for (const reconcilableResult of reconcilableResults) { const { bestId, subPath } = this._getBestIdAndSubPath( id, @@ -323,10 +319,12 @@ class IncrementalPublisher { context.incremental.push(incrementalEntry); - if (streamItemsResult.incrementalDataRecords !== undefined) { - this._incrementalGraph.addIncrementalDataRecords( - streamItemsResult.incrementalDataRecords, + const incrementalDataRecords = streamItemsResult.incrementalDataRecords; + if (incrementalDataRecords !== undefined) { + const newPending = this._incrementalGraph.getNewPending( + incrementalDataRecords, ); + context.pending.push(...this._pendingSourcesToResults(newPending)); } } } From 163e96c8a66440014e3ac9c4a1c2925fa4f50e97 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 25 Jun 2024 23:04:27 +0300 Subject: [PATCH 2/7] change internal method names --- src/execution/IncrementalGraph.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 3e55b3587a..1f35df1db6 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -145,7 +145,7 @@ export class IncrementalGraph { const reconcilableResults = Array.from( deferredFragmentNode.reconcilableResults, ); - this._removePending(deferredFragmentNode); + this._removeRootNode(deferredFragmentNode); for (const reconcilableResult of reconcilableResults) { for (const otherDeferredFragmentNode of this._fragmentsToNodes( reconcilableResult.deferredGroupedFieldSetRecord @@ -171,7 +171,7 @@ export class IncrementalGraph { if (deferredFragmentNode === undefined) { return false; } - this._removePending(deferredFragmentNode); + this._removeRootNode(deferredFragmentNode); this._deferredFragmentNodes.delete(deferredFragmentRecord); // TODO: add test case for an erroring deferred fragment with child defers /* c8 ignore next 5 */ @@ -184,10 +184,10 @@ export class IncrementalGraph { } removeStream(streamRecord: StreamRecord): void { - this._removePending(streamRecord); + this._removeRootNode(streamRecord); } - private _removePending(subsequentResultNode: SubsequentResultNode): void { + private _removeRootNode(subsequentResultNode: SubsequentResultNode): void { this._rootNodes.delete(subsequentResultNode); if (this._rootNodes.size === 0) { for (const resolve of this._nextQueue) { @@ -212,7 +212,7 @@ export class IncrementalGraph { incrementalDataRecord, ); } - if (this._hasPendingFragment(incrementalDataRecord)) { + if (this._completesRootNode(incrementalDataRecord)) { this._onDeferredGroupedFieldSet(incrementalDataRecord); } } else if (parents === undefined) { @@ -231,37 +231,37 @@ export class IncrementalGraph { } private _promoteNonEmptyToRoot( - newPendingNodes: Set, + newRootNodes: Set, ): ReadonlyArray { - const newPendingResults: Array = []; - for (const node of newPendingNodes) { + const newPending: Array = []; + for (const node of newRootNodes) { if (isDeferredFragmentNode(node)) { if (node.deferredGroupedFieldSetRecords.size > 0) { for (const deferredGroupedFieldSetRecord of node.deferredGroupedFieldSetRecords) { - if (!this._hasPendingFragment(deferredGroupedFieldSetRecord)) { + if (!this._completesRootNode(deferredGroupedFieldSetRecord)) { this._onDeferredGroupedFieldSet(deferredGroupedFieldSetRecord); } } this._rootNodes.add(node); - newPendingResults.push(node.deferredFragmentRecord); + newPending.push(node.deferredFragmentRecord); continue; } this._deferredFragmentNodes.delete(node.deferredFragmentRecord); for (const child of node.children) { - newPendingNodes.add(child); + newRootNodes.add(child); } } else { this._rootNodes.add(node); - newPendingResults.push(node); + newPending.push(node); // eslint-disable-next-line @typescript-eslint/no-floating-promises this._onStreamItems(node); } } - return newPendingResults; + return newPending; } - private _hasPendingFragment( + private _completesRootNode( deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, ): boolean { return this._fragmentsToNodes( From fc849b7411e871dbf9cc29a9a0a18b067ccee6c5 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 26 Jun 2024 17:19:41 +0300 Subject: [PATCH 3/7] newPending => newRootNodes --- src/execution/IncrementalGraph.ts | 8 ++++---- src/execution/IncrementalPublisher.ts | 26 +++++++++++++------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 1f35df1db6..543e57beb1 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -54,7 +54,7 @@ export class IncrementalGraph { this._nextQueue = []; } - getNewPending( + getNewRootNodes( incrementalDataRecords: ReadonlyArray, ): ReadonlyArray { const initialResultChildren = new Set(); @@ -127,7 +127,7 @@ export class IncrementalGraph { completeDeferredFragment(deferredFragmentRecord: DeferredFragmentRecord): | { - newPending: ReadonlyArray; + newRootNodes: ReadonlyArray; reconcilableResults: ReadonlyArray; } | undefined { @@ -156,10 +156,10 @@ export class IncrementalGraph { ); } } - const newPending = this._promoteNonEmptyToRoot( + const newRootNodes = this._promoteNonEmptyToRoot( deferredFragmentNode.children, ); - return { newPending, reconcilableResults }; + return { newRootNodes, reconcilableResults }; } removeDeferredFragment( diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index ba07c7e413..a625b0e098 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -74,11 +74,11 @@ class IncrementalPublisher { errors: ReadonlyArray | undefined, incrementalDataRecords: ReadonlyArray, ): ExperimentalIncrementalExecutionResults { - const newPending = this._incrementalGraph.getNewPending( + const newRootNodes = this._incrementalGraph.getNewRootNodes( incrementalDataRecords, ); - const pending = this._pendingSourcesToResults(newPending); + const pending = this._toPendingResults(newRootNodes); const initialResult: InitialIncrementalExecutionResult = errors === undefined @@ -91,19 +91,19 @@ class IncrementalPublisher { }; } - private _pendingSourcesToResults( - newPending: ReadonlyArray, + private _toPendingResults( + newRootNodes: ReadonlyArray, ): Array { const pendingResults: Array = []; - for (const pendingSource of newPending) { + for (const node of newRootNodes) { const id = String(this._getNextId()); - pendingSource.id = id; + node.id = id; const pendingResult: PendingResult = { id, - path: pathToArray(pendingSource.path), + path: pathToArray(node.path), }; - if (pendingSource.label !== undefined) { - pendingResult.label = pendingSource.label; + if (node.label !== undefined) { + pendingResult.label = node.label; } pendingResults.push(pendingResult); } @@ -262,8 +262,8 @@ class IncrementalPublisher { const id = deferredFragmentRecord.id; invariant(id !== undefined); const incremental = context.incremental; - const { newPending, reconcilableResults } = completion; - context.pending.push(...this._pendingSourcesToResults(newPending)); + const { newRootNodes, reconcilableResults } = completion; + context.pending.push(...this._toPendingResults(newRootNodes)); for (const reconcilableResult of reconcilableResults) { const { bestId, subPath } = this._getBestIdAndSubPath( id, @@ -321,10 +321,10 @@ class IncrementalPublisher { const incrementalDataRecords = streamItemsResult.incrementalDataRecords; if (incrementalDataRecords !== undefined) { - const newPending = this._incrementalGraph.getNewPending( + const newRootNodes = this._incrementalGraph.getNewRootNodes( incrementalDataRecords, ); - context.pending.push(...this._pendingSourcesToResults(newPending)); + context.pending.push(...this._toPendingResults(newRootNodes)); } } } From f1f549c1cdbafac3a356a5438119ccce6fb8ec41 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 26 Jun 2024 17:31:01 +0300 Subject: [PATCH 4/7] fixup --- src/execution/IncrementalGraph.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 543e57beb1..76f1a32351 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -231,10 +231,10 @@ export class IncrementalGraph { } private _promoteNonEmptyToRoot( - newRootNodes: Set, + maybeEmptyNewRootNodes: Set, ): ReadonlyArray { - const newPending: Array = []; - for (const node of newRootNodes) { + const newRootNodes: Array = []; + for (const node of maybeEmptyNewRootNodes) { if (isDeferredFragmentNode(node)) { if (node.deferredGroupedFieldSetRecords.size > 0) { for (const deferredGroupedFieldSetRecord of node.deferredGroupedFieldSetRecords) { @@ -243,22 +243,22 @@ export class IncrementalGraph { } } this._rootNodes.add(node); - newPending.push(node.deferredFragmentRecord); + newRootNodes.push(node.deferredFragmentRecord); continue; } this._deferredFragmentNodes.delete(node.deferredFragmentRecord); for (const child of node.children) { - newRootNodes.add(child); + maybeEmptyNewRootNodes.add(child); } } else { this._rootNodes.add(node); - newPending.push(node); + newRootNodes.push(node); // eslint-disable-next-line @typescript-eslint/no-floating-promises this._onStreamItems(node); } } - return newPending; + return newRootNodes; } private _completesRootNode( From 3e64c5cfe6c950cb3e4cffe5143d3352dd76e79a Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 17 Jun 2024 22:25:07 +0300 Subject: [PATCH 5/7] simplify graph by allowing mutation --- src/execution/IncrementalGraph.ts | 153 +++++++++--------------------- src/execution/execute.ts | 8 +- src/execution/types.ts | 25 ++++- 3 files changed, 73 insertions(+), 113 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 76f1a32351..4778b526f6 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -15,32 +15,16 @@ import type { StreamRecord, SubsequentResultRecord, } from './types.js'; -import { isDeferredGroupedFieldSetRecord } from './types.js'; - -interface DeferredFragmentNode { - deferredFragmentRecord: DeferredFragmentRecord; - deferredGroupedFieldSetRecords: Set; - reconcilableResults: Set; - children: Set; -} - -function isDeferredFragmentNode( - node: SubsequentResultNode | undefined, -): node is DeferredFragmentNode { - return node !== undefined && 'deferredFragmentRecord' in node; -} - -type SubsequentResultNode = DeferredFragmentNode | StreamRecord; +import { + isDeferredFragmentRecord, + isDeferredGroupedFieldSetRecord, +} from './types.js'; /** * @internal */ export class IncrementalGraph { - private _rootNodes: Set; - private _deferredFragmentNodes: Map< - DeferredFragmentRecord, - DeferredFragmentNode - >; + private _rootNodes: Set; private _completedQueue: Array; private _nextQueue: Array< @@ -49,7 +33,6 @@ export class IncrementalGraph { constructor() { this._rootNodes = new Set(); - this._deferredFragmentNodes = new Map(); this._completedQueue = []; this._nextQueue = []; } @@ -57,7 +40,7 @@ export class IncrementalGraph { getNewRootNodes( incrementalDataRecords: ReadonlyArray, ): ReadonlyArray { - const initialResultChildren = new Set(); + const initialResultChildren = new Set(); this._addIncrementalDataRecords( incrementalDataRecords, undefined, @@ -69,13 +52,12 @@ export class IncrementalGraph { addCompletedReconcilableDeferredGroupedFieldSet( reconcilableResult: ReconcilableDeferredGroupedFieldSetResult, ): void { - for (const deferredFragmentNode of this._fragmentsToNodes( - reconcilableResult.deferredGroupedFieldSetRecord.deferredFragmentRecords, - )) { - deferredFragmentNode.deferredGroupedFieldSetRecords.delete( + for (const deferredFragmentRecord of reconcilableResult + .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + deferredFragmentRecord.deferredGroupedFieldSetRecords.delete( reconcilableResult.deferredGroupedFieldSetRecord, ); - deferredFragmentNode.reconcilableResults.add(reconcilableResult); + deferredFragmentRecord.reconcilableResults.add(reconcilableResult); } const incrementalDataRecords = reconcilableResult.incrementalDataRecords; @@ -131,33 +113,28 @@ export class IncrementalGraph { reconcilableResults: ReadonlyArray; } | undefined { - const deferredFragmentNode = this._deferredFragmentNodes.get( - deferredFragmentRecord, - ); // TODO: add test case? /* c8 ignore next 3 */ - if (deferredFragmentNode === undefined) { + if (!this._rootNodes.has(deferredFragmentRecord)) { return; } - if (deferredFragmentNode.deferredGroupedFieldSetRecords.size > 0) { + if (deferredFragmentRecord.deferredGroupedFieldSetRecords.size > 0) { return; } const reconcilableResults = Array.from( - deferredFragmentNode.reconcilableResults, + deferredFragmentRecord.reconcilableResults, ); - this._removeRootNode(deferredFragmentNode); + this._removeRootNode(deferredFragmentRecord); for (const reconcilableResult of reconcilableResults) { - for (const otherDeferredFragmentNode of this._fragmentsToNodes( - reconcilableResult.deferredGroupedFieldSetRecord - .deferredFragmentRecords, - )) { - otherDeferredFragmentNode.reconcilableResults.delete( + for (const otherDeferredFragmentRecord of reconcilableResult + .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + otherDeferredFragmentRecord.reconcilableResults.delete( reconcilableResult, ); } } const newRootNodes = this._promoteNonEmptyToRoot( - deferredFragmentNode.children, + deferredFragmentRecord.children, ); return { newRootNodes, reconcilableResults }; } @@ -165,21 +142,10 @@ export class IncrementalGraph { removeDeferredFragment( deferredFragmentRecord: DeferredFragmentRecord, ): boolean { - const deferredFragmentNode = this._deferredFragmentNodes.get( - deferredFragmentRecord, - ); - if (deferredFragmentNode === undefined) { + if (!this._rootNodes.has(deferredFragmentRecord)) { return false; } - this._removeRootNode(deferredFragmentNode); - this._deferredFragmentNodes.delete(deferredFragmentRecord); - // TODO: add test case for an erroring deferred fragment with child defers - /* c8 ignore next 5 */ - for (const child of deferredFragmentNode.children) { - if (isDeferredFragmentNode(child)) { - this.removeDeferredFragment(child.deferredFragmentRecord); - } - } + this._removeRootNode(deferredFragmentRecord); return true; } @@ -187,8 +153,10 @@ export class IncrementalGraph { this._removeRootNode(streamRecord); } - private _removeRootNode(subsequentResultNode: SubsequentResultNode): void { - this._rootNodes.delete(subsequentResultNode); + private _removeRootNode( + subsequentResultRecord: SubsequentResultRecord, + ): void { + this._rootNodes.delete(subsequentResultRecord); if (this._rootNodes.size === 0) { for (const resolve of this._nextQueue) { resolve({ value: undefined, done: true }); @@ -199,16 +167,16 @@ export class IncrementalGraph { private _addIncrementalDataRecords( incrementalDataRecords: ReadonlyArray, parents: ReadonlyArray | undefined, - initialResultChildren?: Set | undefined, + initialResultChildren?: Set | undefined, ): void { for (const incrementalDataRecord of incrementalDataRecords) { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) { - const deferredFragmentNode = this._addDeferredFragmentNode( + this._addDeferredFragment( deferredFragmentRecord, initialResultChildren, ); - deferredFragmentNode.deferredGroupedFieldSetRecords.add( + deferredFragmentRecord.deferredGroupedFieldSetRecords.add( incrementalDataRecord, ); } @@ -220,22 +188,19 @@ export class IncrementalGraph { initialResultChildren.add(incrementalDataRecord); } else { for (const parent of parents) { - const deferredFragmentNode = this._addDeferredFragmentNode( - parent, - initialResultChildren, - ); - deferredFragmentNode.children.add(incrementalDataRecord); + this._addDeferredFragment(parent, initialResultChildren); + parent.children.add(incrementalDataRecord); } } } } private _promoteNonEmptyToRoot( - maybeEmptyNewRootNodes: Set, + maybeEmptyNewRootNodes: Set, ): ReadonlyArray { const newRootNodes: Array = []; for (const node of maybeEmptyNewRootNodes) { - if (isDeferredFragmentNode(node)) { + if (isDeferredFragmentRecord(node)) { if (node.deferredGroupedFieldSetRecords.size > 0) { for (const deferredGroupedFieldSetRecord of node.deferredGroupedFieldSetRecords) { if (!this._completesRootNode(deferredGroupedFieldSetRecord)) { @@ -243,10 +208,9 @@ export class IncrementalGraph { } } this._rootNodes.add(node); - newRootNodes.push(node.deferredFragmentRecord); + newRootNodes.push(node); continue; } - this._deferredFragmentNodes.delete(node.deferredFragmentRecord); for (const child of node.children) { maybeEmptyNewRootNodes.add(child); } @@ -264,53 +228,26 @@ export class IncrementalGraph { private _completesRootNode( deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, ): boolean { - return this._fragmentsToNodes( - deferredGroupedFieldSetRecord.deferredFragmentRecords, - ).some((node) => this._rootNodes.has(node)); - } - - private _fragmentsToNodes( - deferredFragmentRecords: ReadonlyArray, - ): Array { - return deferredFragmentRecords - .map((deferredFragmentRecord) => - this._deferredFragmentNodes.get(deferredFragmentRecord), - ) - .filter(isDeferredFragmentNode); + return deferredGroupedFieldSetRecord.deferredFragmentRecords.some( + (deferredFragmentRecord) => this._rootNodes.has(deferredFragmentRecord), + ); } - private _addDeferredFragmentNode( + private _addDeferredFragment( deferredFragmentRecord: DeferredFragmentRecord, - initialResultChildren: Set | undefined, - ): DeferredFragmentNode { - let deferredFragmentNode = this._deferredFragmentNodes.get( - deferredFragmentRecord, - ); - if (deferredFragmentNode !== undefined) { - return deferredFragmentNode; + subsequentResultRecords: Set | undefined, + ): void { + if (this._rootNodes.has(deferredFragmentRecord)) { + return; } - deferredFragmentNode = { - deferredFragmentRecord, - deferredGroupedFieldSetRecords: new Set(), - reconcilableResults: new Set(), - children: new Set(), - }; - this._deferredFragmentNodes.set( - deferredFragmentRecord, - deferredFragmentNode, - ); const parent = deferredFragmentRecord.parent; if (parent === undefined) { - invariant(initialResultChildren !== undefined); - initialResultChildren.add(deferredFragmentNode); - return deferredFragmentNode; + invariant(subsequentResultRecords !== undefined); + subsequentResultRecords.add(deferredFragmentRecord); + return; } - const parentNode = this._addDeferredFragmentNode( - parent, - initialResultChildren, - ); - parentNode.children.add(deferredFragmentNode); - return deferredFragmentNode; + parent.children.add(deferredFragmentRecord); + this._addDeferredFragment(parent, subsequentResultRecords); } private _onDeferredGroupedFieldSet( diff --git a/src/execution/execute.ts b/src/execution/execute.ts index d01b6ee768..f32036f645 100644 --- a/src/execution/execute.ts +++ b/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); diff --git a/src/execution/types.ts b/src/execution/types.ts index 50f9a083f8..c88ae9986e 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -214,11 +214,34 @@ export interface DeferredGroupedFieldSetRecord { export type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord; -export interface DeferredFragmentRecord { +/** @internal */ +export class DeferredFragmentRecord { path: Path | undefined; label: string | undefined; id?: string | undefined; parent: DeferredFragmentRecord | undefined; + deferredGroupedFieldSetRecords: Set; + reconcilableResults: Set; + children: Set; + + constructor( + path: Path | undefined, + label: string | undefined, + parent: DeferredFragmentRecord | undefined, + ) { + this.path = path; + this.label = label; + this.parent = parent; + this.deferredGroupedFieldSetRecords = new Set(); + this.reconcilableResults = new Set(); + this.children = new Set(); + } +} + +export function isDeferredFragmentRecord( + subsequentResultRecord: SubsequentResultRecord, +): subsequentResultRecord is DeferredFragmentRecord { + return subsequentResultRecord instanceof DeferredFragmentRecord; } export interface StreamItemResult { From 7e2fe1e57729053a720e41596ac412a87f5d87fd Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 19 Jun 2024 22:50:29 +0300 Subject: [PATCH 6/7] 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 --- src/execution/IncrementalGraph.ts | 21 ++-- src/execution/__tests__/defer-test.ts | 132 +++++++++++++++++++++++++- src/execution/execute.ts | 29 ++++-- src/execution/types.ts | 21 ++++ 4 files changed, 180 insertions(+), 23 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 4778b526f6..6520f90cdb 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -8,6 +8,7 @@ import type { GraphQLError } from '../error/GraphQLError.js'; import type { DeferredFragmentRecord, DeferredGroupedFieldSetRecord, + DeferredGroupedFieldSetResult, IncrementalDataRecord, IncrementalDataRecordResult, ReconcilableDeferredGroupedFieldSetResult, @@ -113,12 +114,10 @@ export class IncrementalGraph { reconcilableResults: ReadonlyArray; } | undefined { - // TODO: add test case? - /* c8 ignore next 3 */ - if (!this._rootNodes.has(deferredFragmentRecord)) { - return; - } - if (deferredFragmentRecord.deferredGroupedFieldSetRecords.size > 0) { + if ( + !this._rootNodes.has(deferredFragmentRecord) || + deferredFragmentRecord.deferredGroupedFieldSetRecords.size > 0 + ) { return; } const reconcilableResults = Array.from( @@ -202,6 +201,7 @@ export class IncrementalGraph { for (const node of maybeEmptyNewRootNodes) { if (isDeferredFragmentRecord(node)) { if (node.deferredGroupedFieldSetRecords.size > 0) { + node.setAsPending(); for (const deferredGroupedFieldSetRecord of node.deferredGroupedFieldSetRecords) { if (!this._completesRootNode(deferredGroupedFieldSetRecord)) { this._onDeferredGroupedFieldSet(deferredGroupedFieldSetRecord); @@ -253,12 +253,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 + ).value; if (isPromise(result)) { // eslint-disable-next-line @typescript-eslint/no-floating-promises result.then((resolved) => this._enqueue(resolved)); diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts index e74aebb9ae..2853415bd0 100644 --- a/src/execution/__tests__/defer-test.ts +++ b/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 { diff --git a/src/execution/execute.ts b/src/execution/execute.ts index f32036f645..92d6977ed1 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2114,16 +2114,25 @@ 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 { + 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); } diff --git a/src/execution/types.ts b/src/execution/types.ts index c88ae9986e..76205ccaec 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -224,6 +224,9 @@ export class DeferredFragmentRecord { reconcilableResults: Set; children: Set; + private pending: boolean; + private fns: Array<() => void>; + constructor( path: Path | undefined, label: string | undefined, @@ -235,6 +238,24 @@ export class DeferredFragmentRecord { this.deferredGroupedFieldSetRecords = new Set(); this.reconcilableResults = new Set(); this.children = new Set(); + this.pending = false; + this.fns = []; + } + + onPending(fn: () => void): void { + if (this.pending) { + fn(); + } else { + this.fns.push(fn); + } + } + + setAsPending(): void { + this.pending = true; + let fn; + while ((fn = this.fns.shift()) !== undefined) { + fn(); + } } } From c4524ff03f1d358ef1560a8a434b44753fbb7106 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 20 Jun 2024 12:58:37 +0300 Subject: [PATCH 7/7] refactor(incremental): set pending result id during execution This makes id an immutable property of each pending result, reducing the need to track these ids / lazily create them within the publisher. Empty and filtered deferred fragments will lead to "missing" ids and id ordering will now depend on timing/early execution. The id by the specification are opaque strings so that this is not a concern. --- src/execution/IncrementalPublisher.ts | 18 ++--------- src/execution/__tests__/defer-test.ts | 44 +++++++++++++------------- src/execution/__tests__/stream-test.ts | 8 ++--- src/execution/execute.ts | 14 +++++++- src/execution/types.ts | 6 ++-- 5 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index a625b0e098..55d8bb2ef7 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -60,12 +60,10 @@ interface SubsequentIncrementalExecutionResultContext { */ class IncrementalPublisher { private _context: IncrementalPublisherContext; - private _nextId: number; private _incrementalGraph: IncrementalGraph; constructor(context: IncrementalPublisherContext) { this._context = context; - this._nextId = 0; this._incrementalGraph = new IncrementalGraph(); } @@ -96,10 +94,8 @@ class IncrementalPublisher { ): Array { const pendingResults: Array = []; for (const node of newRootNodes) { - const id = String(this._getNextId()); - node.id = id; const pendingResult: PendingResult = { - id, + id: node.id, path: pathToArray(node.path), }; if (node.label !== undefined) { @@ -110,10 +106,6 @@ class IncrementalPublisher { return pendingResults; } - private _getNextId(): string { - return String(this._nextId++); - } - private _subscribe(): AsyncGenerator< SubsequentIncrementalExecutionResult, void, @@ -231,16 +223,14 @@ class IncrementalPublisher { ) { for (const deferredFragmentRecord of deferredGroupedFieldSetResult .deferredGroupedFieldSetRecord.deferredFragmentRecords) { - const id = deferredFragmentRecord.id; if ( !this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord) ) { // This can occur if multiple deferred grouped field sets error for a fragment. continue; } - invariant(id !== undefined); context.completed.push({ - id, + id: deferredFragmentRecord.id, errors: deferredGroupedFieldSetResult.errors, }); } @@ -259,11 +249,10 @@ class IncrementalPublisher { if (completion === undefined) { continue; } - const id = deferredFragmentRecord.id; - invariant(id !== undefined); const incremental = context.incremental; const { newRootNodes, reconcilableResults } = completion; context.pending.push(...this._toPendingResults(newRootNodes)); + const id = deferredFragmentRecord.id; for (const reconcilableResult of reconcilableResults) { const { bestId, subPath } = this._getBestIdAndSubPath( id, @@ -289,7 +278,6 @@ class IncrementalPublisher { ): void { const streamRecord = streamItemsResult.streamRecord; const id = streamRecord.id; - invariant(id !== undefined); if (streamItemsResult.errors !== undefined) { context.completed.push({ id, diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts index 2853415bd0..b28ce39cbb 100644 --- a/src/execution/__tests__/defer-test.ts +++ b/src/execution/__tests__/defer-test.ts @@ -607,12 +607,12 @@ describe('Execute: defer directive', () => { data: { hero: {}, }, - pending: [{ id: '0', path: ['hero'] }], + pending: [{ id: '1', path: ['hero'] }], hasNext: true, }, { - incremental: [{ data: { name: 'Luke' }, id: '0' }], - completed: [{ id: '0' }], + incremental: [{ data: { name: 'Luke' }, id: '1' }], + completed: [{ id: '1' }], hasNext: false, }, ]); @@ -787,8 +787,8 @@ describe('Execute: defer directive', () => { hero: {}, }, pending: [ - { id: '0', path: ['hero'], label: 'DeferID' }, - { id: '1', path: [], label: 'DeferName' }, + { id: '1', path: ['hero'], label: 'DeferID' }, + { id: '0', path: [], label: 'DeferName' }, ], hasNext: true, }, @@ -798,17 +798,17 @@ describe('Execute: defer directive', () => { data: { id: '1', }, - id: '0', + id: '1', }, { data: { name: 'Luke', }, - id: '1', + id: '0', subPath: ['hero'], }, ], - completed: [{ id: '0' }, { id: '1' }], + completed: [{ id: '1' }, { id: '0' }], hasNext: false, }, ]); @@ -1019,18 +1019,18 @@ describe('Execute: defer directive', () => { data: { hero: { friends: [{}, {}, {}] } }, pending: [ { id: '0', path: ['hero', 'friends', 0] }, - { id: '1', path: ['hero', 'friends', 1] }, - { id: '2', path: ['hero', 'friends', 2] }, + { id: '4', path: ['hero', 'friends', 1] }, + { id: '8', path: ['hero', 'friends', 2] }, ], hasNext: true, }, { incremental: [ { data: { id: '2', name: 'Han' }, id: '0' }, - { data: { id: '3', name: 'Leia' }, id: '1' }, - { data: { id: '4', name: 'C-3PO' }, id: '2' }, + { data: { id: '3', name: 'Leia' }, id: '4' }, + { data: { id: '4', name: 'C-3PO' }, id: '8' }, ], - completed: [{ id: '0' }, { id: '1' }, { id: '2' }], + completed: [{ id: '0' }, { id: '4' }, { id: '8' }], hasNext: false, }, ]); @@ -1275,8 +1275,8 @@ describe('Execute: defer directive', () => { }, }, pending: [ - { id: '0', path: ['hero', 'nestedObject', 'deeperObject'] }, { id: '1', path: ['hero', 'nestedObject', 'deeperObject'] }, + { id: '2', path: ['hero', 'nestedObject', 'deeperObject'] }, ], hasNext: true, }, @@ -1286,16 +1286,16 @@ describe('Execute: defer directive', () => { data: { foo: 'foo', }, - id: '0', + id: '1', }, { data: { bar: 'bar', }, - id: '1', + id: '2', }, ], - completed: [{ id: '0' }, { id: '1' }], + completed: [{ id: '1' }, { id: '2' }], hasNext: false, }, ]); @@ -1351,8 +1351,8 @@ describe('Execute: defer directive', () => { }, }, pending: [ - { id: '0', path: ['a', 'b'] }, - { id: '1', path: [] }, + { id: '1', path: ['a', 'b'] }, + { id: '0', path: [] }, ], hasNext: true, }, @@ -1360,14 +1360,14 @@ describe('Execute: defer directive', () => { incremental: [ { data: { e: { f: 'f' } }, - id: '0', + id: '1', }, { data: { g: { h: 'h' } }, - id: '1', + id: '0', }, ], - completed: [{ id: '0' }, { id: '1' }], + completed: [{ id: '1' }, { id: '0' }], hasNext: false, }, ]); diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index 15ad4028a5..b8eaf695d7 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -1978,14 +1978,14 @@ describe('Execute: stream directive', () => { nestedFriendList: [], }, }, - pending: [{ id: '0', path: ['nestedObject', 'nestedFriendList'] }], + pending: [{ id: '1', path: ['nestedObject', 'nestedFriendList'] }], hasNext: true, }, { incremental: [ { items: [{ id: '1', name: 'Luke' }], - id: '0', + id: '1', }, ], hasNext: true, @@ -1994,13 +1994,13 @@ describe('Execute: stream directive', () => { incremental: [ { items: [{ id: '2', name: 'Han' }], - id: '0', + id: '1', }, ], hasNext: true, }, { - completed: [{ id: '0' }], + completed: [{ id: '1' }], hasNext: false, }, ]); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 92d6977ed1..4fab9c460f 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -142,6 +142,7 @@ export interface ExecutionContext { subscribeFieldResolver: GraphQLFieldResolver; enableEarlyExecution: boolean; errors: Array | undefined; + nextId: number; cancellableStreams: Set | undefined; } @@ -299,7 +300,11 @@ function executeOperation( const fieldPLan = buildFieldPlan(groupedFieldSet); groupedFieldSet = fieldPLan.groupedFieldSet; const newGroupedFieldSets = fieldPLan.newGroupedFieldSets; - const newDeferMap = addNewDeferredFragments(newDeferUsages, new Map()); + const newDeferMap = addNewDeferredFragments( + exeContext, + newDeferUsages, + new Map(), + ); graphqlWrappedResult = executeRootGroupedFieldSet( exeContext, @@ -505,6 +510,7 @@ export function buildExecutionContext( subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, enableEarlyExecution: enableEarlyExecution === true, errors: undefined, + nextId: 0, cancellableStreams: undefined, }; } @@ -1113,6 +1119,7 @@ async function completeAsyncIteratorValue( streamRecord = { label: streamUsage.label, path, + id: String(exeContext.nextId++), streamItemQueue, }; } else { @@ -1120,6 +1127,7 @@ async function completeAsyncIteratorValue( label: streamUsage.label, path, streamItemQueue, + id: String(exeContext.nextId++), earlyReturn: returnFn.bind(asyncIterator), }; if (exeContext.cancellableStreams === undefined) { @@ -1274,6 +1282,7 @@ function completeIterableValue( const syncStreamRecord: StreamRecord = { label: streamUsage.label, path, + id: String(exeContext.nextId++), streamItemQueue: buildSyncStreamItemQueue( item, index, @@ -1662,6 +1671,7 @@ function invalidReturnTypeError( * */ function addNewDeferredFragments( + exeContext: ExecutionContext, newDeferUsages: ReadonlyArray, newDeferMap: Map, path?: Path | undefined, @@ -1679,6 +1689,7 @@ function addNewDeferredFragments( const deferredFragmentRecord = new DeferredFragmentRecord( path, newDeferUsage.label, + String(exeContext.nextId++), parent, ); @@ -1733,6 +1744,7 @@ function collectAndExecuteSubfields( groupedFieldSet = subFieldPlan.groupedFieldSet; const newGroupedFieldSets = subFieldPlan.newGroupedFieldSets; const newDeferMap = addNewDeferredFragments( + exeContext, newDeferUsages, new Map(deferMap), path, diff --git a/src/execution/types.ts b/src/execution/types.ts index 76205ccaec..8eec6c121a 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -218,7 +218,7 @@ export type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord; export class DeferredFragmentRecord { path: Path | undefined; label: string | undefined; - id?: string | undefined; + id: string; parent: DeferredFragmentRecord | undefined; deferredGroupedFieldSetRecords: Set; reconcilableResults: Set; @@ -230,10 +230,12 @@ export class DeferredFragmentRecord { constructor( path: Path | undefined, label: string | undefined, + id: string, parent: DeferredFragmentRecord | undefined, ) { this.path = path; this.label = label; + this.id = id; this.parent = parent; this.deferredGroupedFieldSetRecords = new Set(); this.reconcilableResults = new Set(); @@ -276,7 +278,7 @@ export type StreamItemRecord = ThunkIncrementalResult; export interface StreamRecord { path: Path; label: string | undefined; - id?: string | undefined; + id: string; streamItemQueue: Array; }