From b97128ff03cba8fb109f73212208cc7bd2822826 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 7 Apr 2024 12:34:45 +0300 Subject: [PATCH 01/27] incremental: introduce GraphQLResult type to avoid filtering --- src/execution/IncrementalPublisher.ts | 992 +++++++++--------- src/execution/__tests__/defer-test.ts | 50 +- src/execution/__tests__/executor-test.ts | 51 + src/execution/__tests__/stream-test.ts | 96 +- src/execution/buildFieldPlan.ts | 45 +- src/execution/execute.ts | 1192 ++++++++++++---------- src/jsutils/promiseForObject.ts | 7 +- 7 files changed, 1272 insertions(+), 1161 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 1ca31acb88..08208a1b58 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -1,6 +1,8 @@ +import { isPromise } from '../jsutils/isPromise.js'; import type { ObjMap } from '../jsutils/ObjMap.js'; import type { Path } from '../jsutils/Path.js'; import { pathToArray } from '../jsutils/Path.js'; +import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; import { promiseWithResolvers } from '../jsutils/promiseWithResolvers.js'; import type { @@ -8,13 +10,7 @@ import type { GraphQLFormattedError, } from '../error/GraphQLError.js'; -import type { GroupedFieldSet } from './buildFieldPlan.js'; - -interface IncrementalUpdate> { - pending: ReadonlyArray; - incremental: ReadonlyArray>; - completed: ReadonlyArray; -} +import type { DeferUsageSet } from './buildFieldPlan.js'; /** * The result of GraphQL execution. @@ -78,7 +74,10 @@ export interface FormattedInitialIncrementalExecutionResult< export interface SubsequentIncrementalExecutionResult< TData = unknown, TExtensions = ObjMap, -> extends Partial> { +> { + pending?: ReadonlyArray; + incremental?: ReadonlyArray>; + completed?: ReadonlyArray; hasNext: boolean; extensions?: TExtensions; } @@ -94,12 +93,15 @@ export interface FormattedSubsequentIncrementalExecutionResult< extensions?: TExtensions; } +interface BareDeferredGroupedFieldSetResult> { + errors?: ReadonlyArray; + data: TData; +} + export interface IncrementalDeferResult< TData = ObjMap, TExtensions = ObjMap, -> { - errors?: ReadonlyArray; - data: TData; +> extends BareDeferredGroupedFieldSetResult { id: string; subPath?: ReadonlyArray; extensions?: TExtensions; @@ -116,12 +118,15 @@ export interface FormattedIncrementalDeferResult< extensions?: TExtensions; } -export interface IncrementalStreamResult< - TData = Array, - TExtensions = ObjMap, -> { +interface BareStreamItemsResult> { errors?: ReadonlyArray; items: TData; +} + +export interface IncrementalStreamResult< + TData = ReadonlyArray, + TExtensions = ObjMap, +> extends BareStreamItemsResult { id: string; subPath?: ReadonlyArray; extensions?: TExtensions; @@ -166,214 +171,204 @@ export interface FormattedCompletedResult { errors?: ReadonlyArray; } +export function buildIncrementalResponse( + context: IncrementalPublisherContext, + result: ObjMap, + errors: ReadonlyArray, + incrementalDataRecords: ReadonlyArray, +): ExperimentalIncrementalExecutionResults { + const incrementalPublisher = new IncrementalPublisher(context); + return incrementalPublisher.buildResponse( + result, + errors, + incrementalDataRecords, + ); +} + +interface IncrementalPublisherContext { + cancellableStreams: Set; +} + /** * This class is used to publish incremental results to the client, enabling semi-concurrent * execution while preserving result order. * - * The internal publishing state is managed as follows: - * - * '_released': the set of Subsequent Result records that are ready to be sent to the client, - * i.e. their parents have completed and they have also completed. - * - * `_pending`: the set of Subsequent Result records that are definitely pending, i.e. their - * parents have completed so that they can no longer be filtered. This includes all Subsequent - * Result records in `released`, as well as the records that have not yet completed. - * * @internal */ -export class IncrementalPublisher { - private _nextId = 0; - private _released: Set; +class IncrementalPublisher { + private _context: IncrementalPublisherContext; + private _nextId: number; private _pending: Set; - + private _completedResultQueue: Array; + private _newPending: Set; + private _incremental: Array; + private _completed: Array; // these are assigned within the Promise executor called synchronously within the constructor private _signalled!: Promise; private _resolve!: () => void; - constructor() { - this._released = new Set(); + constructor(context: IncrementalPublisherContext) { + this._context = context; + this._nextId = 0; this._pending = new Set(); + this._completedResultQueue = []; + this._newPending = new Set(); + this._incremental = []; + this._completed = []; this._reset(); } - reportNewDeferFragmentRecord( - deferredFragmentRecord: DeferredFragmentRecord, - parentIncrementalResultRecord: - | InitialResultRecord - | DeferredFragmentRecord - | StreamItemsRecord, - ): void { - parentIncrementalResultRecord.children.add(deferredFragmentRecord); - } + buildResponse( + data: ObjMap, + errors: ReadonlyArray, + incrementalDataRecords: ReadonlyArray, + ): ExperimentalIncrementalExecutionResults { + this._addIncrementalDataRecords(incrementalDataRecords); + this._pruneEmpty(); - reportNewDeferredGroupedFieldSetRecord( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, - ): void { - for (const deferredFragmentRecord of deferredGroupedFieldSetRecord.deferredFragmentRecords) { - deferredFragmentRecord._pending.add(deferredGroupedFieldSetRecord); - deferredFragmentRecord.deferredGroupedFieldSetRecords.add( - deferredGroupedFieldSetRecord, - ); - } + const pending = this._pendingSourcesToResults(); + + const initialResult: InitialIncrementalExecutionResult = + errors.length === 0 + ? { data, pending, hasNext: true } + : { errors, data, pending, hasNext: true }; + + return { + initialResult, + subsequentResults: this._subscribe(), + }; } - reportNewStreamItemsRecord( - streamItemsRecord: StreamItemsRecord, - parentIncrementalDataRecord: IncrementalDataRecord, + private _addIncrementalDataRecords( + incrementalDataRecords: ReadonlyArray, ): void { - if (isDeferredGroupedFieldSetRecord(parentIncrementalDataRecord)) { - for (const parent of parentIncrementalDataRecord.deferredFragmentRecords) { - parent.children.add(streamItemsRecord); + for (const incrementalDataRecord of incrementalDataRecords) { + if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { + for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) { + this._addDeferredFragmentRecord(deferredFragmentRecord); + } + + const result = incrementalDataRecord.result; + if (isPromise(result)) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + result.then((resolved) => { + this._enqueueCompletedDeferredGroupedFieldSet(resolved); + }); + } else { + this._enqueueCompletedDeferredGroupedFieldSet(result); + } + + continue; } - } else { - parentIncrementalDataRecord.children.add(streamItemsRecord); - } - } - completeDeferredGroupedFieldSet( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, - data: ObjMap, - ): void { - deferredGroupedFieldSetRecord.data = data; - for (const deferredFragmentRecord of deferredGroupedFieldSetRecord.deferredFragmentRecords) { - deferredFragmentRecord._pending.delete(deferredGroupedFieldSetRecord); - if (deferredFragmentRecord._pending.size === 0) { - this.completeDeferredFragmentRecord(deferredFragmentRecord); + const streamRecord = incrementalDataRecord.streamRecord; + if (streamRecord.id === undefined) { + this._newPending.add(streamRecord); } - } - } - markErroredDeferredGroupedFieldSet( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, - error: GraphQLError, - ): void { - for (const deferredFragmentRecord of deferredGroupedFieldSetRecord.deferredFragmentRecords) { - deferredFragmentRecord.errors.push(error); - this.completeDeferredFragmentRecord(deferredFragmentRecord); + const result = incrementalDataRecord.getResult(); + if (isPromise(result)) { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + result.then((resolved) => { + this._enqueueCompletedStreamItems(resolved); + }); + } else { + this._enqueueCompletedStreamItems(result); + } } } - completeDeferredFragmentRecord( + private _addDeferredFragmentRecord( deferredFragmentRecord: DeferredFragmentRecord, ): void { - this._release(deferredFragmentRecord); - } - - completeStreamItemsRecord( - streamItemsRecord: StreamItemsRecord, - items: Array, - ) { - streamItemsRecord.items = items; - streamItemsRecord.isCompleted = true; - this._release(streamItemsRecord); - } + const parent = deferredFragmentRecord.parent; + if (parent === undefined) { + if (deferredFragmentRecord.id !== undefined) { + return; + } - markErroredStreamItemsRecord( - streamItemsRecord: StreamItemsRecord, - error: GraphQLError, - ) { - streamItemsRecord.streamRecord.errors.push(error); - this.setIsFinalRecord(streamItemsRecord); - streamItemsRecord.isCompleted = true; - streamItemsRecord.streamRecord.earlyReturn?.().catch(() => { - // ignore error - }); - this._release(streamItemsRecord); - } + this._newPending.add(deferredFragmentRecord); + return; + } - setIsFinalRecord(streamItemsRecord: StreamItemsRecord) { - streamItemsRecord.isFinalRecord = true; - } + if (parent.children.has(deferredFragmentRecord)) { + return; + } - setIsCompletedAsyncIterator(streamItemsRecord: StreamItemsRecord) { - streamItemsRecord.isCompletedAsyncIterator = true; - this.setIsFinalRecord(streamItemsRecord); - } + parent.children.add(deferredFragmentRecord); - addFieldError( - incrementalDataRecord: IncrementalDataRecord, - error: GraphQLError, - ) { - incrementalDataRecord.errors.push(error); + this._addDeferredFragmentRecord(parent); } - buildDataResponse( - initialResultRecord: InitialResultRecord, - data: ObjMap | null, - ): ExecutionResult | ExperimentalIncrementalExecutionResults { - const pendingSources = this._publish(initialResultRecord.children); - - const errors = initialResultRecord.errors; - const initialResult = errors.length === 0 ? { data } : { errors, data }; - if (pendingSources.size > 0) { - return { - initialResult: { - ...initialResult, - pending: this._pendingSourcesToResults(pendingSources), - hasNext: true, - }, - subsequentResults: this._subscribe(), - }; + private _pruneEmpty() { + const maybeEmptyNewPending = this._newPending; + this._newPending = new Set(); + for (const node of maybeEmptyNewPending) { + if (isDeferredFragmentRecord(node)) { + if (node.deferredGroupedFieldSetRecords.length > 0) { + this._newPending.add(node); + continue; + } + for (const child of node.children) { + this._addNonEmptyNewPending(child); + } + } else { + this._newPending.add(node); + } } - return initialResult; } - buildErrorResponse( - initialResultRecord: InitialResultRecord, - error: GraphQLError, - ): ExecutionResult { - const errors = initialResultRecord.errors; - errors.push(error); - return { data: null, errors }; + private _addNonEmptyNewPending( + deferredFragmentRecord: DeferredFragmentRecord, + ): void { + if (deferredFragmentRecord.deferredGroupedFieldSetRecords.length > 0) { + this._newPending.add(deferredFragmentRecord); + return; + } + /* c8 ignore next 5 */ + // TODO: add test case for this, if when skipping an empty deferred fragment, the empty fragment has nested children. + for (const child of deferredFragmentRecord.children) { + this._addNonEmptyNewPending(child); + } } - filter( - nullPath: Path | undefined, - erroringIncrementalDataRecord: IncrementalDataRecord, + private _enqueueCompletedDeferredGroupedFieldSet( + result: DeferredGroupedFieldSetResult, ): void { - const nullPathArray = pathToArray(nullPath); - - const streams = new Set(); - - const children = this._getChildren(erroringIncrementalDataRecord); - const descendants = this._getDescendants(children); - - for (const child of descendants) { - if (!this._nullsChildSubsequentResultRecord(child, nullPathArray)) { - continue; - } - - child.filtered = true; - - if (isStreamItemsRecord(child)) { - streams.add(child.streamRecord); + let hasPendingParent = false; + for (const deferredFragmentRecord of result.deferredFragmentRecords) { + if (deferredFragmentRecord.id !== undefined) { + hasPendingParent = true; } + deferredFragmentRecord.results.push(result); + } + if (hasPendingParent) { + this._completedResultQueue.push(result); + this._trigger(); } + } - streams.forEach((stream) => { - stream.earlyReturn?.().catch(() => { - // ignore error - }); - }); + private _enqueueCompletedStreamItems(result: StreamItemsResult): void { + this._completedResultQueue.push(result); + this._trigger(); } - private _pendingSourcesToResults( - pendingSources: ReadonlySet, - ): Array { + private _pendingSourcesToResults(): Array { const pendingResults: Array = []; - for (const pendingSource of pendingSources) { - pendingSource.pendingSent = true; - const id = this._getNextId(); + for (const pendingSource of this._newPending) { + const id = String(this._getNextId()); + this._pending.add(pendingSource); pendingSource.id = id; const pendingResult: PendingResult = { id, - path: pendingSource.path, + path: pathToArray(pendingSource.path), }; if (pendingSource.label !== undefined) { pendingResult.label = pendingSource.label; } pendingResults.push(pendingResult); } + this._newPending.clear(); return pendingResults; } @@ -391,47 +386,67 @@ export class IncrementalPublisher { const _next = async (): Promise< IteratorResult > => { - // eslint-disable-next-line no-constant-condition - while (true) { - if (isDone) { - return { value: undefined, done: true }; - } + while (!isDone) { + let pending: Array = []; + + let completedResult: IncrementalDataRecordResult | undefined; + while ( + (completedResult = this._completedResultQueue.shift()) !== undefined + ) { + if (isDeferredGroupedFieldSetResult(completedResult)) { + this._handleCompletedDeferredGroupedFieldSet(completedResult); + } else { + this._handleCompletedStreamItems(completedResult); + } - for (const item of this._released) { - this._pending.delete(item); + pending = [...pending, ...this._pendingSourcesToResults()]; } - const released = this._released; - this._released = new Set(); - const result = this._getIncrementalResult(released); + if (this._incremental.length > 0 || this._completed.length > 0) { + const hasNext = this._pending.size > 0; - if (this._pending.size === 0) { - isDone = true; - } + if (!hasNext) { + isDone = true; + } - if (result !== undefined) { - return { value: result, done: false }; + const subsequentIncrementalExecutionResult: SubsequentIncrementalExecutionResult = + { hasNext }; + + if (pending.length > 0) { + subsequentIncrementalExecutionResult.pending = pending; + } + if (this._incremental.length > 0) { + subsequentIncrementalExecutionResult.incremental = + this._incremental; + } + if (this._completed.length > 0) { + subsequentIncrementalExecutionResult.completed = this._completed; + } + + this._incremental = []; + this._completed = []; + + return { value: subsequentIncrementalExecutionResult, done: false }; } // eslint-disable-next-line no-await-in-loop await this._signalled; } + + await returnStreamIterators().catch(() => { + // ignore errors + }); + + return { value: undefined, done: true }; }; const returnStreamIterators = async (): Promise => { - const streams = new Set(); - const descendants = this._getDescendants(this._pending); - for (const subsequentResultRecord of descendants) { - if (isStreamItemsRecord(subsequentResultRecord)) { - streams.add(subsequentResultRecord.streamRecord); - } - } const promises: Array> = []; - streams.forEach((streamRecord) => { - if (streamRecord.earlyReturn) { + for (const streamRecord of this._context.cancellableStreams) { + if (streamRecord.earlyReturn !== undefined) { promises.push(streamRecord.earlyReturn()); } - }); + } await Promise.all(promises); }; @@ -475,385 +490,380 @@ export class IncrementalPublisher { this._signalled = signalled; } - private _introduce(item: SubsequentResultRecord) { - this._pending.add(item); - } - - private _release(item: SubsequentResultRecord): void { - if (this._pending.has(item)) { - this._released.add(item); - this._trigger(); + private _handleCompletedDeferredGroupedFieldSet( + deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, + ): void { + if ( + isNonReconcilableDeferredGroupedFieldSetResult( + deferredGroupedFieldSetResult, + ) + ) { + for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) { + const id = deferredFragmentRecord.id; + if (id !== undefined) { + this._completed.push({ + id, + errors: deferredGroupedFieldSetResult.errors, + }); + this._pending.delete(deferredFragmentRecord); + } + } + return; } - } - - private _push(item: SubsequentResultRecord): void { - this._released.add(item); - this._pending.add(item); - this._trigger(); - } - - private _getIncrementalResult( - completedRecords: ReadonlySet, - ): SubsequentIncrementalExecutionResult | undefined { - const { pending, incremental, completed } = - this._processPending(completedRecords); - - const hasNext = this._pending.size > 0; - if (incremental.length === 0 && completed.length === 0 && hasNext) { - return undefined; + for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) { + deferredFragmentRecord.reconcilableResults.push( + deferredGroupedFieldSetResult, + ); } - const result: SubsequentIncrementalExecutionResult = { hasNext }; - if (pending.length) { - result.pending = pending; - } - if (incremental.length) { - result.incremental = incremental; - } - if (completed.length) { - result.completed = completed; + if (deferredGroupedFieldSetResult.incrementalDataRecords.length > 0) { + this._addIncrementalDataRecords( + deferredGroupedFieldSetResult.incrementalDataRecords, + ); } - return result; - } - - private _processPending( - completedRecords: ReadonlySet, - ): IncrementalUpdate { - const newPendingSources = new Set(); - const incrementalResults: Array = []; - const completedResults: Array = []; - for (const subsequentResultRecord of completedRecords) { - this._publish(subsequentResultRecord.children, newPendingSources); - if (isStreamItemsRecord(subsequentResultRecord)) { - if (subsequentResultRecord.isFinalRecord) { - newPendingSources.delete(subsequentResultRecord.streamRecord); - completedResults.push( - this._completedRecordToResult(subsequentResultRecord.streamRecord), - ); - } - if (subsequentResultRecord.isCompletedAsyncIterator) { - // async iterable resolver just finished but there may be pending payloads - continue; - } - if (subsequentResultRecord.streamRecord.errors.length > 0) { + for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) { + const id = deferredFragmentRecord.id; + // TODO: add test case for this. + // Presumably, this can occur if an error causes a fragment to be completed early, + // while an asynchronous deferred grouped field set result is enqueued. + /* c8 ignore next 3 */ + if (id === undefined) { + continue; + } + const fragmentResults = deferredFragmentRecord.reconcilableResults; + if ( + deferredFragmentRecord.deferredGroupedFieldSetRecords.length !== + fragmentResults.length + ) { + continue; + } + for (const fragmentResult of fragmentResults) { + if (fragmentResult.sent) { continue; } - const incrementalResult: IncrementalStreamResult = { - // safe because `items` is always defined when the record is completed - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - items: subsequentResultRecord.items!, - // safe because `id` is defined once the stream has been released as pending - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: subsequentResultRecord.streamRecord.id!, - }; - if (subsequentResultRecord.errors.length > 0) { - incrementalResult.errors = subsequentResultRecord.errors; - } - incrementalResults.push(incrementalResult); - } else { - newPendingSources.delete(subsequentResultRecord); - completedResults.push( - this._completedRecordToResult(subsequentResultRecord), + fragmentResult.sent = true; + const { bestId, subPath } = this._getBestIdAndSubPath( + id, + deferredFragmentRecord, + fragmentResult, ); - if (subsequentResultRecord.errors.length > 0) { - continue; + const incrementalEntry: IncrementalDeferResult = { + ...fragmentResult.result, + id: bestId, + }; + if (subPath !== undefined) { + incrementalEntry.subPath = subPath; } - for (const deferredGroupedFieldSetRecord of subsequentResultRecord.deferredGroupedFieldSetRecords) { - if (!deferredGroupedFieldSetRecord.sent) { - deferredGroupedFieldSetRecord.sent = true; - const incrementalResult: IncrementalDeferResult = - this._getIncrementalDeferResult(deferredGroupedFieldSetRecord); - if (deferredGroupedFieldSetRecord.errors.length > 0) { - incrementalResult.errors = deferredGroupedFieldSetRecord.errors; - } - incrementalResults.push(incrementalResult); + this._incremental.push(incrementalEntry); + } + this._completed.push({ id }); + this._pending.delete(deferredFragmentRecord); + for (const child of deferredFragmentRecord.children) { + this._newPending.add(child); + for (const childResult of child.results) { + if (!isPromise(childResult)) { + this._completedResultQueue.push(childResult); } } } } - return { - pending: this._pendingSourcesToResults(newPendingSources), - incremental: incrementalResults, - completed: completedResults, - }; + this._pruneEmpty(); } - private _getIncrementalDeferResult( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, - ): IncrementalDeferResult { - const { data, deferredFragmentRecords } = deferredGroupedFieldSetRecord; - let maxLength: number | undefined; - let idWithLongestPath: string | undefined; - for (const deferredFragmentRecord of deferredFragmentRecords) { - const id = deferredFragmentRecord.id; - // TODO: add test - /* c8 ignore next 3 */ - if (id === undefined) { - continue; - } - const length = deferredFragmentRecord.path.length; - if (maxLength === undefined || length > maxLength) { - maxLength = length; - idWithLongestPath = id; - } - } - const subPath = deferredGroupedFieldSetRecord.path.slice(maxLength); - const incrementalDeferResult: IncrementalDeferResult = { - // safe because `data``is always defined when the record is completed - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - data: data!, - // safe because `id` is always defined once the fragment has been released - // as pending and at least one fragment has been completed, so must have been - // released as pending - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: idWithLongestPath!, - }; - - if (subPath.length > 0) { - incrementalDeferResult.subPath = subPath; + private _handleCompletedStreamItems( + streamItemsResult: StreamItemsResult, + ): void { + const streamRecord = streamItemsResult.streamRecord; + const id = streamRecord.id; + // TODO: Consider adding invariant or non-null assertion, as this should never happen. Since the stream is converted into a linked list + // for ordering purposes, if an entry errors, additional entries will not be processed. + /* c8 ignore next 3 */ + if (id === undefined) { + return; } + if (streamItemsResult.result === undefined) { + this._completed.push({ id }); + this._pending.delete(streamRecord); + this._context.cancellableStreams.delete(streamRecord); + } else if (streamItemsResult.result === null) { + this._completed.push({ + id, + errors: streamItemsResult.errors, + }); + this._pending.delete(streamRecord); + this._context.cancellableStreams.delete(streamRecord); + streamRecord.earlyReturn?.().catch(() => { + /* c8 ignore next 1 */ + // ignore error + }); + } else { + const incrementalEntry: IncrementalStreamResult = { + id, + ...streamItemsResult.result, + }; - return incrementalDeferResult; - } + this._incremental.push(incrementalEntry); - private _completedRecordToResult( - completedRecord: DeferredFragmentRecord | StreamRecord, - ): CompletedResult { - const result: CompletedResult = { - // safe because `id` is defined once the stream has been released as pending - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: completedRecord.id!, - }; - if (completedRecord.errors.length > 0) { - result.errors = completedRecord.errors; + if (streamItemsResult.incrementalDataRecords.length > 0) { + this._addIncrementalDataRecords( + streamItemsResult.incrementalDataRecords, + ); + this._pruneEmpty(); + } } - return result; } - private _publish( - subsequentResultRecords: ReadonlySet, - pendingSources = new Set(), - ): Set { - const emptyRecords: Array = []; + private _getBestIdAndSubPath( + initialId: string, + initialDeferredFragmentRecord: DeferredFragmentRecord, + deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, + ): { bestId: string; subPath: ReadonlyArray | undefined } { + let maxLength = pathToArray(initialDeferredFragmentRecord.path).length; + let bestId = initialId; - for (const subsequentResultRecord of subsequentResultRecords) { - if (subsequentResultRecord.filtered) { + for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) { + if (deferredFragmentRecord === initialDeferredFragmentRecord) { continue; } - if (isStreamItemsRecord(subsequentResultRecord)) { - if (subsequentResultRecord.isCompleted) { - this._push(subsequentResultRecord); - } else { - this._introduce(subsequentResultRecord); - } - - const stream = subsequentResultRecord.streamRecord; - if (!stream.pendingSent) { - pendingSources.add(stream); - } - continue; - } - - if (subsequentResultRecord._pending.size > 0) { - this._introduce(subsequentResultRecord); - } else if ( - subsequentResultRecord.deferredGroupedFieldSetRecords.size === 0 - ) { - emptyRecords.push(subsequentResultRecord); + const id = deferredFragmentRecord.id; + // TODO: add test case for when an fragment has not been released, but might be processed for the shortest path. + /* c8 ignore next 3 */ + if (id === undefined) { continue; - } else { - this._push(subsequentResultRecord); - } - - if (!subsequentResultRecord.pendingSent) { - pendingSources.add(subsequentResultRecord); } - } - - for (const emptyRecord of emptyRecords) { - this._publish(emptyRecord.children, pendingSources); - } - - return pendingSources; - } - - private _getChildren( - erroringIncrementalDataRecord: IncrementalDataRecord, - ): ReadonlySet { - const children = new Set(); - if (isDeferredGroupedFieldSetRecord(erroringIncrementalDataRecord)) { - for (const erroringIncrementalResultRecord of erroringIncrementalDataRecord.deferredFragmentRecords) { - for (const child of erroringIncrementalResultRecord.children) { - children.add(child); - } - } - } else { - for (const child of erroringIncrementalDataRecord.children) { - children.add(child); + const fragmentPath = pathToArray(deferredFragmentRecord.path); + const length = fragmentPath.length; + if (length > maxLength) { + maxLength = length; + bestId = id; } } - return children; + const subPath = deferredGroupedFieldSetResult.path.slice(maxLength); + return { + bestId, + subPath: subPath.length > 0 ? subPath : undefined, + }; } +} - private _getDescendants( - children: ReadonlySet, - descendants = new Set(), - ): ReadonlySet { - for (const child of children) { - descendants.add(child); - this._getDescendants(child.children, descendants); - } - return descendants; - } +function isDeferredFragmentRecord( + subsequentResultRecord: SubsequentResultRecord, +): subsequentResultRecord is DeferredFragmentRecord { + return subsequentResultRecord instanceof DeferredFragmentRecord; +} - private _nullsChildSubsequentResultRecord( - subsequentResultRecord: SubsequentResultRecord, - nullPath: ReadonlyArray, - ): boolean { - const incrementalDataRecords = isStreamItemsRecord(subsequentResultRecord) - ? [subsequentResultRecord] - : subsequentResultRecord.deferredGroupedFieldSetRecords; +function isDeferredGroupedFieldSetRecord( + incrementalDataRecord: IncrementalDataRecord, +): incrementalDataRecord is DeferredGroupedFieldSetRecord { + return incrementalDataRecord instanceof DeferredGroupedFieldSetRecord; +} - for (const incrementalDataRecord of incrementalDataRecords) { - if (this._matchesPath(incrementalDataRecord.path, nullPath)) { - return true; - } - } +export interface IncrementalContext { + deferUsageSet: DeferUsageSet | undefined; + path: Path | undefined; + errors: Array; +} - return false; - } +export type DeferredGroupedFieldSetResult = + | ReconcilableDeferredGroupedFieldSetResult + | NonReconcilableDeferredGroupedFieldSetResult; - private _matchesPath( - testPath: ReadonlyArray, - basePath: ReadonlyArray, - ): boolean { - for (let i = 0; i < basePath.length; i++) { - if (basePath[i] !== testPath[i]) { - // testPath points to a path unaffected at basePath - return false; - } - } - return true; - } +export function isDeferredGroupedFieldSetResult( + subsequentResult: DeferredGroupedFieldSetResult | StreamItemsResult, +): subsequentResult is DeferredGroupedFieldSetResult { + return 'deferredFragmentRecords' in subsequentResult; } -function isDeferredGroupedFieldSetRecord( - incrementalDataRecord: unknown, -): incrementalDataRecord is DeferredGroupedFieldSetRecord { - return incrementalDataRecord instanceof DeferredGroupedFieldSetRecord; +interface ReconcilableDeferredGroupedFieldSetResult { + deferredFragmentRecords: ReadonlyArray; + path: Array; + result: BareDeferredGroupedFieldSetResult; + incrementalDataRecords: ReadonlyArray; + sent?: true | undefined; } -function isStreamItemsRecord( - subsequentResultRecord: unknown, -): subsequentResultRecord is StreamItemsRecord { - return subsequentResultRecord instanceof StreamItemsRecord; +interface NonReconcilableDeferredGroupedFieldSetResult { + result: null; + errors: ReadonlyArray; + deferredFragmentRecords: ReadonlyArray; + path: Array; } -/** @internal */ -export class InitialResultRecord { - errors: Array; - children: Set; - constructor() { - this.errors = []; - this.children = new Set(); - } +export function isNonReconcilableDeferredGroupedFieldSetResult( + deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, +): deferredGroupedFieldSetResult is NonReconcilableDeferredGroupedFieldSetResult { + return deferredGroupedFieldSetResult.result === null; } /** @internal */ export class DeferredGroupedFieldSetRecord { - path: ReadonlyArray; + path: Path | undefined; deferredFragmentRecords: ReadonlyArray; - groupedFieldSet: GroupedFieldSet; - shouldInitiateDefer: boolean; - errors: Array; - data: ObjMap | undefined; - sent: boolean; + result: PromiseOrValue; constructor(opts: { path: Path | undefined; + deferUsageSet: DeferUsageSet; deferredFragmentRecords: ReadonlyArray; - groupedFieldSet: GroupedFieldSet; - shouldInitiateDefer: boolean; + executor: ( + incrementalContext: IncrementalContext, + ) => PromiseOrValue; }) { - this.path = pathToArray(opts.path); - this.deferredFragmentRecords = opts.deferredFragmentRecords; - this.groupedFieldSet = opts.groupedFieldSet; - this.shouldInitiateDefer = opts.shouldInitiateDefer; - this.errors = []; - this.sent = false; + const { path, deferUsageSet, deferredFragmentRecords, executor } = opts; + this.path = path; + this.deferredFragmentRecords = deferredFragmentRecords; + + const incrementalContext: IncrementalContext = { + deferUsageSet, + path, + errors: [], + }; + + for (const deferredFragmentRecord of deferredFragmentRecords) { + deferredFragmentRecord.deferredGroupedFieldSetRecords.push(this); + } + + this.result = deferredFragmentRecords.some( + (deferredFragmentRecord) => deferredFragmentRecord.id !== undefined, + ) + ? executor(incrementalContext) + : Promise.resolve().then(() => executor(incrementalContext)); } } /** @internal */ export class DeferredFragmentRecord { - path: ReadonlyArray; + path: Path | undefined; label: string | undefined; - id: string | undefined; - children: Set; - deferredGroupedFieldSetRecords: Set; - errors: Array; - filtered: boolean; - pendingSent?: boolean; - _pending: Set; + deferredGroupedFieldSetRecords: Array; + results: Array; + reconcilableResults: Array; + parent: DeferredFragmentRecord | undefined; + children: Set; + id?: string | undefined; - constructor(opts: { path: Path | undefined; label: string | undefined }) { - this.path = pathToArray(opts.path); + constructor(opts: { + path: Path | undefined; + label: string | undefined; + parent: DeferredFragmentRecord | undefined; + }) { + this.path = opts.path; this.label = opts.label; + this.deferredGroupedFieldSetRecords = []; + this.results = []; + this.reconcilableResults = []; + this.parent = opts.parent; this.children = new Set(); - this.filtered = false; - this.deferredGroupedFieldSetRecords = new Set(); - this.errors = []; - this._pending = new Set(); } } /** @internal */ export class StreamRecord { label: string | undefined; - path: ReadonlyArray; - id: string | undefined; - errors: Array; - earlyReturn?: (() => Promise) | undefined; - pendingSent?: boolean; + path: Path; + earlyReturn: (() => Promise) | undefined; + id?: string | undefined; constructor(opts: { label: string | undefined; path: Path; earlyReturn?: (() => Promise) | undefined; }) { - this.label = opts.label; - this.path = pathToArray(opts.path); - this.errors = []; - this.earlyReturn = opts.earlyReturn; + const { label, path, earlyReturn } = opts; + this.label = label; + this.path = path; + this.earlyReturn = earlyReturn; } } +interface NonReconcilableStreamItemsResult { + streamRecord: StreamRecord; + result: null; + errors: ReadonlyArray; +} + +interface NonTerminatingStreamItemsResult { + streamRecord: StreamRecord; + result: BareStreamItemsResult; + incrementalDataRecords: ReadonlyArray; +} + +interface TerminatingStreamItemsResult { + streamRecord: StreamRecord; + result?: never; + incrementalDataRecords?: never; + errors?: never; +} + +export type StreamItemsResult = + | NonReconcilableStreamItemsResult + | NonTerminatingStreamItemsResult + | TerminatingStreamItemsResult; + +export function isNonTerminatingStreamItemsResult( + streamItemsResult: StreamItemsResult, +): streamItemsResult is NonTerminatingStreamItemsResult { + return streamItemsResult.result != null; +} + /** @internal */ export class StreamItemsRecord { - errors: Array; streamRecord: StreamRecord; - path: ReadonlyArray; - items: Array | undefined; - children: Set; - isFinalRecord?: boolean; - isCompletedAsyncIterator?: boolean; - isCompleted: boolean; - filtered: boolean; - - constructor(opts: { streamRecord: StreamRecord; path: Path | undefined }) { - this.streamRecord = opts.streamRecord; - this.path = pathToArray(opts.path); - this.children = new Set(); - this.errors = []; - this.isCompleted = false; - this.filtered = false; + nextStreamItems: StreamItemsRecord | undefined; + + private _result: PromiseOrValue; + + constructor(opts: { + streamRecord: StreamRecord; + itemPath?: Path | undefined; + executor: ( + incrementalContext: IncrementalContext, + ) => PromiseOrValue; + }) { + const { streamRecord, itemPath, executor } = opts; + this.streamRecord = streamRecord; + const incrementalContext: IncrementalContext = { + deferUsageSet: undefined, + path: itemPath, + errors: [], + }; + + this._result = executor(incrementalContext); + } + + getResult(): PromiseOrValue { + if (isPromise(this._result)) { + return this._result.then((resolved) => + this._prependNextStreamItems(resolved), + ); + } + + return this._prependNextStreamItems(this._result); + } + + private _prependNextStreamItems( + result: StreamItemsResult, + ): StreamItemsResult { + return isNonTerminatingStreamItemsResult(result) && + this.nextStreamItems !== undefined + ? { + ...result, + incrementalDataRecords: + result.incrementalDataRecords.length === 0 + ? [this.nextStreamItems] + : [this.nextStreamItems, ...result.incrementalDataRecords], + } + : result; } } export type IncrementalDataRecord = - | InitialResultRecord | DeferredGroupedFieldSetRecord | StreamItemsRecord; -type SubsequentResultRecord = DeferredFragmentRecord | StreamItemsRecord; +export type IncrementalDataRecordResult = + | DeferredGroupedFieldSetResult + | StreamItemsResult; + +type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord; diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts index d03570270a..03bf8126c6 100644 --- a/src/execution/__tests__/defer-test.ts +++ b/src/execution/__tests__/defer-test.ts @@ -367,12 +367,6 @@ describe('Execute: defer directive', () => { }, id: '0', }, - ], - completed: [{ id: '0' }], - hasNext: true, - }, - { - incremental: [ { data: { friends: [{ name: 'Han' }, { name: 'Leia' }, { name: 'C-3PO' }], @@ -380,7 +374,7 @@ describe('Execute: defer directive', () => { id: '1', }, ], - completed: [{ id: '1' }], + completed: [{ id: '0' }, { id: '1' }], hasNext: false, }, ]); @@ -674,8 +668,8 @@ describe('Execute: defer directive', () => { hero: {}, }, pending: [ - { id: '0', path: [], label: 'DeferName' }, - { id: '1', path: ['hero'], label: 'DeferID' }, + { id: '0', path: ['hero'], label: 'DeferID' }, + { id: '1', path: [], label: 'DeferName' }, ], hasNext: true, }, @@ -685,17 +679,17 @@ describe('Execute: defer directive', () => { data: { id: '1', }, - id: '1', + id: '0', }, { data: { name: 'Luke', }, - id: '0', + id: '1', subPath: ['hero'], }, ], - completed: [{ id: '1' }, { id: '0' }], + completed: [{ id: '0' }, { id: '1' }], hasNext: false, }, ]); @@ -983,37 +977,27 @@ describe('Execute: defer directive', () => { hasNext: true, }, { - pending: [{ id: '1', path: ['hero', 'nestedObject'] }], + pending: [ + { id: '1', path: ['hero', 'nestedObject'] }, + { id: '2', path: ['hero', 'nestedObject', 'deeperObject'] }, + ], incremental: [ { data: { bar: 'bar' }, id: '0', subPath: ['nestedObject', 'deeperObject'], }, - ], - completed: [{ id: '0' }], - hasNext: true, - }, - { - pending: [{ id: '2', path: ['hero', 'nestedObject', 'deeperObject'] }], - incremental: [ { data: { baz: 'baz' }, id: '1', subPath: ['deeperObject'], }, - ], - hasNext: true, - completed: [{ id: '1' }], - }, - { - incremental: [ { data: { bak: 'bak' }, id: '2', }, ], - completed: [{ id: '2' }], + completed: [{ id: '0' }, { id: '1' }, { id: '2' }], hasNext: false, }, ]); @@ -1132,8 +1116,8 @@ describe('Execute: defer directive', () => { }, }, pending: [ - { id: '0', path: [] }, - { id: '1', path: ['a', 'b'] }, + { id: '0', path: ['a', 'b'] }, + { id: '1', path: [] }, ], hasNext: true, }, @@ -1141,14 +1125,14 @@ describe('Execute: defer directive', () => { incremental: [ { data: { e: { f: 'f' } }, - id: '1', + id: '0', }, { data: { g: { h: 'h' } }, - id: '0', + id: '1', }, ], - completed: [{ id: '1' }, { id: '0' }], + completed: [{ id: '0' }, { id: '1' }], hasNext: false, }, ]); @@ -1277,6 +1261,7 @@ describe('Execute: defer directive', () => { }, ], completed: [ + { id: '0' }, { id: '1', errors: [ @@ -1288,7 +1273,6 @@ describe('Execute: defer directive', () => { }, ], }, - { id: '0' }, ], hasNext: false, }, diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index c29b4ae60d..de33f8c91b 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -635,6 +635,57 @@ describe('Execute: Handles basic execution tasks', () => { expect(isAsyncResolverFinished).to.equal(true); }); + it('handles async bubbling errors combined with non-bubbling errors', async () => { + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + asyncNonNullError: { + type: new GraphQLNonNull(GraphQLString), + async resolve() { + await resolveOnNextTick(); + return null; + }, + }, + asyncError: { + type: GraphQLString, + async resolve() { + await resolveOnNextTick(); + throw new Error('Oops'); + }, + }, + }, + }), + }); + + // Order is important here, as the nullable error should resolve first + const document = parse(` + { + asyncError + asyncNonNullError + } + `); + + const result = execute({ schema, document }); + + expectJSON(await result).toDeepEqual({ + data: null, + errors: [ + { + message: 'Oops', + locations: [{ line: 3, column: 9 }], + path: ['asyncError'], + }, + { + message: + 'Cannot return null for non-nullable field Query.asyncNonNullError.', + locations: [{ line: 4, column: 9 }], + path: ['asyncNonNullError'], + }, + ], + }); + }); + it('Full response path is included for non-nullable fields', () => { const A: GraphQLObjectType = new GraphQLObjectType({ name: 'A', diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index 6e1928f945..9c0a7ed22b 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -146,11 +146,10 @@ describe('Execute: stream directive', () => { hasNext: true, }, { - incremental: [{ items: ['banana'], id: '0' }], - hasNext: true, - }, - { - incremental: [{ items: ['coconut'], id: '0' }], + incremental: [ + { items: ['banana'], id: '0' }, + { items: ['coconut'], id: '0' }, + ], completed: [{ id: '0' }], hasNext: false, }, @@ -170,15 +169,11 @@ describe('Execute: stream directive', () => { hasNext: true, }, { - incremental: [{ items: ['apple'], id: '0' }], - hasNext: true, - }, - { - incremental: [{ items: ['banana'], id: '0' }], - hasNext: true, - }, - { - incremental: [{ items: ['coconut'], id: '0' }], + incremental: [ + { items: ['apple'], id: '0' }, + { items: ['banana'], id: '0' }, + { items: ['coconut'], id: '0' }, + ], completed: [{ id: '0' }], hasNext: false, }, @@ -228,11 +223,6 @@ describe('Execute: stream directive', () => { items: ['banana'], id: '0', }, - ], - hasNext: true, - }, - { - incremental: [ { items: ['coconut'], id: '0', @@ -297,11 +287,6 @@ describe('Execute: stream directive', () => { items: [['banana', 'banana', 'banana']], id: '0', }, - ], - hasNext: true, - }, - { - incremental: [ { items: [['coconut', 'coconut', 'coconut']], id: '0', @@ -1470,12 +1455,17 @@ describe('Execute: stream directive', () => { }, ], }, + ], + completed: [{ id: '0' }], + hasNext: true, + }, + { + incremental: [ { items: [{ name: 'Luke' }], id: '1', }, ], - completed: [{ id: '0' }], hasNext: true, }, { @@ -1954,40 +1944,49 @@ describe('Execute: stream directive', () => { hasNext: true, }); - const result2Promise = iterator.next(); - resolveIterableCompletion(null); - const result2 = await result2Promise; + const result2 = await iterator.next(); expectJSON(result2).toDeepEqual({ value: { - pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], incremental: [ { data: { name: 'Luke' }, id: '0', }, + ], + completed: [{ id: '0' }], + hasNext: true, + }, + done: false, + }); + + const result3 = await iterator.next(); + expectJSON(result3).toDeepEqual({ + value: { + pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], + incremental: [ { items: [{ id: '2' }], id: '1', }, ], - completed: [{ id: '0' }], hasNext: true, }, done: false, }); - - const result3Promise = iterator.next(); - resolveSlowField('Han'); - const result3 = await result3Promise; - expectJSON(result3).toDeepEqual({ + const result4Promise = iterator.next(); + resolveIterableCompletion(null); + const result4 = await result4Promise; + expectJSON(result4).toDeepEqual({ value: { completed: [{ id: '1' }], hasNext: true, }, done: false, }); - const result4 = await iterator.next(); - expectJSON(result4).toDeepEqual({ + const result5Promise = iterator.next(); + resolveSlowField('Han'); + const result5 = await result5Promise; + expectJSON(result5).toDeepEqual({ value: { incremental: [ { @@ -2000,8 +1999,8 @@ describe('Execute: stream directive', () => { }, done: false, }); - const result5 = await iterator.next(); - expectJSON(result5).toDeepEqual({ + const result6 = await iterator.next(); + expectJSON(result6).toDeepEqual({ value: undefined, done: true, }); @@ -2060,16 +2059,11 @@ describe('Execute: stream directive', () => { const result2 = await result2Promise; expectJSON(result2).toDeepEqual({ value: { - pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], incremental: [ { data: { name: 'Luke' }, id: '0', }, - { - items: [{ id: '2' }], - id: '1', - }, ], completed: [{ id: '0' }], hasNext: true, @@ -2080,13 +2074,13 @@ describe('Execute: stream directive', () => { const result3 = await iterator.next(); expectJSON(result3).toDeepEqual({ value: { + pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], incremental: [ { - data: { name: 'Han' }, - id: '2', + items: [{ id: '2' }], + id: '1', }, ], - completed: [{ id: '2' }], hasNext: true, }, done: false, @@ -2096,7 +2090,13 @@ describe('Execute: stream directive', () => { const result4 = await result4Promise; expectJSON(result4).toDeepEqual({ value: { - completed: [{ id: '1' }], + incremental: [ + { + data: { name: 'Han' }, + id: '2', + }, + ], + completed: [{ id: '2' }, { id: '1' }], hasNext: false, }, done: false, diff --git a/src/execution/buildFieldPlan.ts b/src/execution/buildFieldPlan.ts index 390e2cf813..55ac0d5dc1 100644 --- a/src/execution/buildFieldPlan.ts +++ b/src/execution/buildFieldPlan.ts @@ -22,7 +22,7 @@ export function buildFieldPlan( parentDeferUsages: DeferUsageSet = new Set(), ): { groupedFieldSet: GroupedFieldSet; - newGroupedFieldSetDetailsMap: Map; + newGroupedFieldSets: Map; } { const groupedFieldSet = new Map< string, @@ -32,18 +32,15 @@ export function buildFieldPlan( } >(); - const newGroupedFieldSetDetailsMap = new Map< + const newGroupedFieldSets = new Map< DeferUsageSet, - { - groupedFieldSet: Map< - string, - { - fields: Array; - deferUsages: DeferUsageSet; - } - >; - shouldInitiateDefer: boolean; - } + Map< + string, + { + fields: Array; + deferUsages: DeferUsageSet; + } + > >(); const map = new Map< @@ -94,12 +91,8 @@ export function buildFieldPlan( continue; } - let newGroupedFieldSetDetails = getBySet( - newGroupedFieldSetDetailsMap, - deferUsageSet, - ); - let newGroupedFieldSet; - if (newGroupedFieldSetDetails === undefined) { + let newGroupedFieldSet = getBySet(newGroupedFieldSets, deferUsageSet); + if (newGroupedFieldSet === undefined) { newGroupedFieldSet = new Map< string, { @@ -108,19 +101,7 @@ export function buildFieldPlan( knownDeferUsages: DeferUsageSet; } >(); - - newGroupedFieldSetDetails = { - groupedFieldSet: newGroupedFieldSet, - shouldInitiateDefer: Array.from(deferUsageSet).some( - (deferUsage) => !parentDeferUsages.has(deferUsage), - ), - }; - newGroupedFieldSetDetailsMap.set( - deferUsageSet, - newGroupedFieldSetDetails, - ); - } else { - newGroupedFieldSet = newGroupedFieldSetDetails.groupedFieldSet; + newGroupedFieldSets.set(deferUsageSet, newGroupedFieldSet); } let fieldGroup = newGroupedFieldSet.get(responseKey); if (fieldGroup === undefined) { @@ -135,7 +116,7 @@ export function buildFieldPlan( return { groupedFieldSet, - newGroupedFieldSetDetailsMap, + newGroupedFieldSets, }; } diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 767a3f77d1..6b2f0f5f15 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -51,21 +51,22 @@ import type { DeferUsageSet, FieldGroup, GroupedFieldSet, - NewGroupedFieldSetDetails, } from './buildFieldPlan.js'; import { buildFieldPlan } from './buildFieldPlan.js'; import type { DeferUsage, FieldDetails } from './collectFields.js'; import { collectFields, collectSubfields } from './collectFields.js'; import type { + DeferredGroupedFieldSetResult, ExecutionResult, ExperimentalIncrementalExecutionResults, + IncrementalContext, IncrementalDataRecord, + StreamItemsResult, } from './IncrementalPublisher.js'; import { + buildIncrementalResponse, DeferredFragmentRecord, DeferredGroupedFieldSetRecord, - IncrementalPublisher, - InitialResultRecord, StreamItemsRecord, StreamRecord, } from './IncrementalPublisher.js'; @@ -142,7 +143,8 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; - incrementalPublisher: IncrementalPublisher; + errors: Array; + cancellableStreams: Set; } export interface ExecutionArgs { @@ -163,6 +165,8 @@ export interface StreamUsage { fieldGroup: FieldGroup; } +type GraphQLResult = [T, ReadonlyArray]; + const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = 'The provided schema unexpectedly contains experimental directives (@defer or @stream). These directives may only be utilized if experimental execution features are explicitly enabled.'; @@ -255,16 +259,9 @@ export function experimentalExecuteIncrementally( function executeOperation( exeContext: ExecutionContext, ): PromiseOrValue { - const initialResultRecord = new InitialResultRecord(); try { - const { - operation, - schema, - fragments, - variableValues, - rootValue, - incrementalPublisher, - } = exeContext; + const { operation, schema, fragments, variableValues, rootValue } = + exeContext; const rootType = schema.getRootType(operation.operation); if (rootType == null) { throw new GraphQLError( @@ -280,60 +277,99 @@ function executeOperation( rootType, operation, ); - const { groupedFieldSet, newGroupedFieldSetDetailsMap } = - buildFieldPlan(fields); - - const newDeferMap = addNewDeferredFragments( - incrementalPublisher, - newDeferUsages, - initialResultRecord, - ); + const { groupedFieldSet, newGroupedFieldSets } = buildFieldPlan(fields); - const path = undefined; + const newDeferMap = addNewDeferredFragments(newDeferUsages, new Map()); - const newDeferredGroupedFieldSetRecords = addNewDeferredGroupedFieldSets( - incrementalPublisher, - newGroupedFieldSetDetailsMap, - newDeferMap, - path, - ); - - const result = executeRootGroupedFieldSet( + let acc = executeRootGroupedFieldSet( exeContext, operation.operation, rootType, rootValue, groupedFieldSet, - initialResultRecord, newDeferMap, ); - executeDeferredGroupedFieldSets( + const newDeferredGroupedFieldSetRecords = executeDeferredGroupedFieldSets( exeContext, rootType, rootValue, - path, - newDeferredGroupedFieldSetRecords, + undefined, + newGroupedFieldSets, newDeferMap, ); - if (isPromise(result)) { - return result.then( - (resolved) => - incrementalPublisher.buildDataResponse(initialResultRecord, resolved), - (error) => - incrementalPublisher.buildErrorResponse(initialResultRecord, error), + acc = withNewDeferredGroupedFieldSets( + acc, + newDeferredGroupedFieldSetRecords, + ); + if (isPromise(acc)) { + return acc.then( + (resolved) => buildDataResponse(exeContext, resolved[0], resolved[1]), + (error) => ({ + data: null, + errors: withError(exeContext.errors, error), + }), ); } - return incrementalPublisher.buildDataResponse(initialResultRecord, result); + return buildDataResponse(exeContext, acc[0], acc[1]); } catch (error) { - return exeContext.incrementalPublisher.buildErrorResponse( - initialResultRecord, - error, - ); + return { data: null, errors: withError(exeContext.errors, error) }; } } +function withNewDeferredGroupedFieldSets( + result: PromiseOrValue>>, + newDeferredGroupedFieldSetRecords: ReadonlyArray, +): PromiseOrValue>> { + if (isPromise(result)) { + return result.then((resolved) => { + appendNewIncrementalDataRecords( + resolved, + newDeferredGroupedFieldSetRecords, + ); + return resolved; + }); + } + + appendNewIncrementalDataRecords(result, newDeferredGroupedFieldSetRecords); + return result; +} + +function appendNewIncrementalDataRecords( + acc: GraphQLResult, + newRecords: ReadonlyArray, +): void { + if (newRecords.length > 0) { + acc[1] = acc[1].length === 0 ? newRecords : [...acc[1], ...newRecords]; + } +} + +function withError( + errors: Array, + error: GraphQLError, +): ReadonlyArray { + return errors.length === 0 ? [error] : [...errors, error]; +} + +function buildDataResponse( + exeContext: ExecutionContext, + data: ObjMap, + incrementalDataRecords: ReadonlyArray, +): ExecutionResult | ExperimentalIncrementalExecutionResults { + const { errors } = exeContext; + if (incrementalDataRecords.length === 0) { + return errors.length > 0 ? { errors, data } : { data }; + } + + return buildIncrementalResponse( + exeContext, + data, + errors, + incrementalDataRecords, + ); +} + /** * Also implements the "Executing requests" section of the GraphQL specification. * However, it guarantees to complete synchronously (or throw an error) assuming @@ -435,7 +471,8 @@ export function buildExecutionContext( fieldResolver: fieldResolver ?? defaultFieldResolver, typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, - incrementalPublisher: new IncrementalPublisher(), + errors: [], + cancellableStreams: new Set(), }; } @@ -446,6 +483,7 @@ function buildPerEventExecutionContext( return { ...exeContext, rootValue: payload, + errors: [], }; } @@ -455,9 +493,8 @@ function executeRootGroupedFieldSet( rootType: GraphQLObjectType, rootValue: unknown, groupedFieldSet: GroupedFieldSet, - initialResultRecord: InitialResultRecord, - newDeferMap: ReadonlyMap, -): PromiseOrValue> { + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { switch (operation) { case OperationTypeNode.QUERY: return executeFields( @@ -466,8 +503,8 @@ function executeRootGroupedFieldSet( rootValue, undefined, groupedFieldSet, - initialResultRecord, - newDeferMap, + undefined, + deferMap, ); case OperationTypeNode.MUTATION: return executeFieldsSerially( @@ -476,8 +513,7 @@ function executeRootGroupedFieldSet( rootValue, undefined, groupedFieldSet, - initialResultRecord, - newDeferMap, + deferMap, ); case OperationTypeNode.SUBSCRIPTION: // TODO: deprecate `subscribe` and move all logic here @@ -488,8 +524,8 @@ function executeRootGroupedFieldSet( rootValue, undefined, groupedFieldSet, - initialResultRecord, - newDeferMap, + undefined, + deferMap, ); } } @@ -504,12 +540,11 @@ function executeFieldsSerially( sourceValue: unknown, path: Path | undefined, groupedFieldSet: GroupedFieldSet, - incrementalDataRecord: InitialResultRecord, - deferMap: ReadonlyMap, -): PromiseOrValue> { + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { return promiseReduce( groupedFieldSet, - (results, [responseName, fieldGroup]) => { + (acc, [responseName, fieldGroup]) => { const fieldPath = addPath(path, responseName, parentType.name); const result = executeField( exeContext, @@ -517,22 +552,24 @@ function executeFieldsSerially( sourceValue, fieldGroup, fieldPath, - incrementalDataRecord, + undefined, deferMap, ); if (result === undefined) { - return results; + return acc; } if (isPromise(result)) { - return result.then((resolvedResult) => { - results[responseName] = resolvedResult; - return results; + return result.then((resolved) => { + acc[0][responseName] = resolved[0]; + appendNewIncrementalDataRecords(acc, resolved[1]); + return acc; }); } - results[responseName] = result; - return results; + acc[0][responseName] = result[0]; + appendNewIncrementalDataRecords(acc, result[1]); + return acc; }, - Object.create(null), + [Object.create(null), []] as GraphQLResult>, ); } @@ -546,10 +583,11 @@ function executeFields( sourceValue: unknown, path: Path | undefined, groupedFieldSet: GroupedFieldSet, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue> { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { const results = Object.create(null); + const acc: GraphQLResult> = [results, []]; let containsPromise = false; try { @@ -561,36 +599,44 @@ function executeFields( sourceValue, fieldGroup, fieldPath, - incrementalDataRecord, + incrementalContext, deferMap, ); if (result !== undefined) { - results[responseName] = result; if (isPromise(result)) { + results[responseName] = result.then((resolved) => { + appendNewIncrementalDataRecords(acc, resolved[1]); + return resolved[0]; + }); containsPromise = true; + } else { + results[responseName] = result[0]; + appendNewIncrementalDataRecords(acc, result[1]); } } } } catch (error) { if (containsPromise) { // Ensure that any promises returned by other fields are handled, as they may also reject. - return promiseForObject(results).finally(() => { + return promiseForObject(results, () => { + /* noop */ + }).finally(() => { throw error; - }); + }) as never; } throw error; } - // If there are no promises, we can just return the object + // If there are no promises, we can just return the object and any incrementalDataRecords if (!containsPromise) { - return results; + return acc; } // Otherwise, results is a map from field name to the result of resolving that // field, which is possibly a promise. Return a promise that will return this // same map, but with any promises replaced with the values they resolved to. - return promiseForObject(results); + return promiseForObject(results, (resolved) => [resolved, acc[1]]); } function toNodes(fieldGroup: FieldGroup): ReadonlyArray { @@ -609,9 +655,9 @@ function executeField( source: unknown, fieldGroup: FieldGroup, path: Path, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue> | undefined { const fieldName = fieldGroup.fields[0].node.name.value; const fieldDef = exeContext.schema.getField(parentType, fieldName); if (!fieldDef) { @@ -655,7 +701,7 @@ function executeField( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -667,7 +713,7 @@ function executeField( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); @@ -681,10 +727,9 @@ function executeField( returnType, fieldGroup, path, - incrementalDataRecord, + incrementalContext, ); - exeContext.incrementalPublisher.filter(path, incrementalDataRecord); - return null; + return [null, []]; }); } return completed; @@ -695,10 +740,9 @@ function executeField( returnType, fieldGroup, path, - incrementalDataRecord, + incrementalContext, ); - exeContext.incrementalPublisher.filter(path, incrementalDataRecord); - return null; + return [null, []]; } } @@ -735,7 +779,7 @@ function handleFieldError( returnType: GraphQLOutputType, fieldGroup: FieldGroup, path: Path, - incrementalDataRecord: IncrementalDataRecord, + incrementalContext: IncrementalContext | undefined, ): void { const error = locatedError(rawError, toNodes(fieldGroup), pathToArray(path)); @@ -747,7 +791,7 @@ function handleFieldError( // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. - exeContext.incrementalPublisher.addFieldError(incrementalDataRecord, error); + (incrementalContext ?? exeContext).errors.push(error); } /** @@ -778,9 +822,9 @@ function completeValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue> { // If result is an Error, throw a located error. if (result instanceof Error) { throw result; @@ -796,10 +840,10 @@ function completeValue( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); - if (completed === null) { + if ((completed as GraphQLResult)[0] === null) { throw new Error( `Cannot return null for non-nullable field ${info.parentType.name}.${info.fieldName}.`, ); @@ -809,7 +853,7 @@ function completeValue( // If result value is null or undefined then return null. if (result == null) { - return null; + return [null, []]; } // If field type is List, complete each item in the list with the inner type @@ -821,7 +865,7 @@ function completeValue( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -829,7 +873,7 @@ function completeValue( // If field type is a leaf type, Scalar or Enum, serialize to a valid value, // returning null if serialization is not possible. if (isLeafType(returnType)) { - return completeLeafValue(returnType, result); + return [completeLeafValue(returnType, result), []]; } // If field type is an abstract type, Interface or Union, determine the @@ -842,7 +886,7 @@ function completeValue( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -856,7 +900,7 @@ function completeValue( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -875,9 +919,9 @@ async function completePromisedValue( info: GraphQLResolveInfo, path: Path, result: Promise, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): Promise { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): Promise> { try { const resolved = await result; let completed = completeValue( @@ -887,9 +931,10 @@ async function completePromisedValue( info, path, resolved, - incrementalDataRecord, + incrementalContext, deferMap, ); + if (isPromise(completed)) { completed = await completed; } @@ -901,10 +946,9 @@ async function completePromisedValue( returnType, fieldGroup, path, - incrementalDataRecord, + incrementalContext, ); - exeContext.incrementalPublisher.filter(path, incrementalDataRecord); - return null; + return [null, []]; } } @@ -982,6 +1026,7 @@ function getStreamUsage( return streamUsage; } + /** * Complete a async iterator value by completing the result and calling * recursively until all the results are completed. @@ -993,37 +1038,45 @@ async function completeAsyncIteratorValue( info: GraphQLResolveInfo, path: Path, asyncIterator: AsyncIterator, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): Promise> { - const streamUsage = getStreamUsage(exeContext, fieldGroup, path); + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): Promise>> { let containsPromise = false; const completedResults: Array = []; + const acc: GraphQLResult> = [completedResults, []]; let index = 0; + const streamUsage = getStreamUsage(exeContext, fieldGroup, path); // eslint-disable-next-line no-constant-condition while (true) { if (streamUsage && index >= streamUsage.initialCount) { - const earlyReturn = asyncIterator.return; const streamRecord = new StreamRecord({ label: streamUsage.label, path, - earlyReturn: - earlyReturn === undefined - ? undefined - : earlyReturn.bind(asyncIterator), + earlyReturn: asyncIterator.return?.bind(asyncIterator), }); - // eslint-disable-next-line @typescript-eslint/no-floating-promises - executeStreamAsyncIterator( + + exeContext.cancellableStreams.add(streamRecord); + + const firstStreamItems = firstAsyncStreamItems( + streamRecord, + path, index, + toNodes(fieldGroup), asyncIterator, - exeContext, - streamUsage.fieldGroup, - info, - itemType, - path, - incrementalDataRecord, - streamRecord, + (currentItemPath, currentItem, currentIncrementalContext) => + completeStreamItems( + streamRecord, + currentItemPath, + currentItem, + exeContext, + currentIncrementalContext, + streamUsage.fieldGroup, + info, + itemType, + ), ); + + appendNewIncrementalDataRecords(acc, [firstStreamItems]); break; } @@ -1032,31 +1085,65 @@ async function completeAsyncIteratorValue( try { // eslint-disable-next-line no-await-in-loop iteration = await asyncIterator.next(); - if (iteration.done) { - break; - } } catch (rawError) { throw locatedError(rawError, toNodes(fieldGroup), pathToArray(path)); } - if ( + // TODO: add test case for stream returning done before initialCount + /* c8 ignore next 3 */ + if (iteration.done) { + break; + } + + const item = iteration.value; + // TODO: add tests for stream backed by asyncIterator that returns a promise + /* c8 ignore start */ + if (isPromise(item)) { + completedResults.push( + completePromisedValue( + exeContext, + itemType, + fieldGroup, + info, + itemPath, + item, + incrementalContext, + deferMap, + ).then((resolved) => { + appendNewIncrementalDataRecords(acc, resolved[1]); + return resolved[0]; + }), + ); + containsPromise = true; + } else if ( + /* c8 ignore stop */ completeListItemValue( - iteration.value, + item, completedResults, + acc, exeContext, itemType, fieldGroup, info, itemPath, - incrementalDataRecord, + incrementalContext, deferMap, ) + // TODO: add tests for stream backed by asyncIterator that completes to a promise + /* c8 ignore start */ ) { containsPromise = true; } - index += 1; + /* c8 ignore stop */ + index++; } - return containsPromise ? Promise.all(completedResults) : completedResults; + + return containsPromise + ? /* c8 ignore start */ Promise.all(completedResults).then((resolved) => [ + resolved, + acc[1], + ]) + : /* c8 ignore stop */ acc; } /** @@ -1070,9 +1157,9 @@ function completeListValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue> { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { const itemType = returnType.ofType; if (isAsyncIterable(result)) { @@ -1085,7 +1172,7 @@ function completeListValue( info, path, asyncIterator, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -1096,65 +1183,91 @@ function completeListValue( ); } - const streamUsage = getStreamUsage(exeContext, fieldGroup, path); - // This is specified as a simple map, however we're optimizing the path // where the list contains no Promises by avoiding creating another Promise. let containsPromise = false; - let currentParents = incrementalDataRecord; const completedResults: Array = []; + const acc: GraphQLResult> = [completedResults, []]; let index = 0; - let streamRecord: StreamRecord | undefined; - for (const item of result) { - // No need to modify the info object containing the path, - // since from here on it is not ever accessed by resolver functions. - const itemPath = addPath(path, index, undefined); + const streamUsage = getStreamUsage(exeContext, fieldGroup, path); + const iterator = result[Symbol.iterator](); + let iteration = iterator.next(); + while (!iteration.done) { + const item = iteration.value; if (streamUsage && index >= streamUsage.initialCount) { - if (streamRecord === undefined) { - streamRecord = new StreamRecord({ label: streamUsage.label, path }); - } - currentParents = executeStreamField( + const streamRecord = new StreamRecord({ + label: streamUsage.label, path, - itemPath, - item, - exeContext, - streamUsage.fieldGroup, - info, - itemType, - currentParents, + }); + + const firstStreamItems = firstSyncStreamItems( streamRecord, + item, + index, + iterator, + (currentItemPath, currentItem, currentIncrementalContext) => + completeStreamItems( + streamRecord, + currentItemPath, + currentItem, + exeContext, + currentIncrementalContext, + streamUsage.fieldGroup, + info, + itemType, + ), ); - index++; - continue; + + appendNewIncrementalDataRecords(acc, [firstStreamItems]); + break; } - if ( + // No need to modify the info object containing the path, + // since from here on it is not ever accessed by resolver functions. + const itemPath = addPath(path, index, undefined); + + if (isPromise(item)) { + completedResults.push( + completePromisedValue( + exeContext, + itemType, + fieldGroup, + info, + itemPath, + item, + incrementalContext, + deferMap, + ).then((resolved) => { + appendNewIncrementalDataRecords(acc, resolved[1]); + return resolved[0]; + }), + ); + containsPromise = true; + } else if ( completeListItemValue( item, completedResults, + acc, exeContext, itemType, fieldGroup, info, itemPath, - incrementalDataRecord, + incrementalContext, deferMap, ) ) { containsPromise = true; } - index++; - } - if (streamRecord !== undefined) { - exeContext.incrementalPublisher.setIsFinalRecord( - currentParents as StreamItemsRecord, - ); + iteration = iterator.next(); } - return containsPromise ? Promise.all(completedResults) : completedResults; + return containsPromise + ? Promise.all(completedResults).then((resolved) => [resolved, acc[1]]) + : acc; } /** @@ -1165,31 +1278,15 @@ function completeListValue( function completeListItemValue( item: unknown, completedResults: Array, + parent: GraphQLResult>, exeContext: ExecutionContext, itemType: GraphQLOutputType, fieldGroup: FieldGroup, info: GraphQLResolveInfo, itemPath: Path, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, ): boolean { - if (isPromise(item)) { - completedResults.push( - completePromisedValue( - exeContext, - itemType, - fieldGroup, - info, - itemPath, - item, - incrementalDataRecord, - deferMap, - ), - ); - - return true; - } - try { const completedItem = completeValue( exeContext, @@ -1198,7 +1295,7 @@ function completeListItemValue( info, itemPath, item, - incrementalDataRecord, + incrementalContext, deferMap, ); @@ -1206,27 +1303,29 @@ function completeListItemValue( // Note: we don't rely on a `catch` method, but we do expect "thenable" // to take a second callback for the error case. completedResults.push( - completedItem.then(undefined, (rawError) => { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - incrementalDataRecord, - ); - exeContext.incrementalPublisher.filter( - itemPath, - incrementalDataRecord, - ); - return null; - }), + completedItem.then( + (resolved) => { + appendNewIncrementalDataRecords(parent, resolved[1]); + return resolved[0]; + }, + (rawError) => { + handleFieldError( + rawError, + exeContext, + itemType, + fieldGroup, + itemPath, + incrementalContext, + ); + return null; + }, + ), ); - return true; } - completedResults.push(completedItem); + completedResults.push(completedItem[0]); + appendNewIncrementalDataRecords(parent, completedItem[1]); } catch (rawError) { handleFieldError( rawError, @@ -1234,12 +1333,10 @@ function completeListItemValue( itemType, fieldGroup, itemPath, - incrementalDataRecord, + incrementalContext, ); - exeContext.incrementalPublisher.filter(itemPath, incrementalDataRecord); completedResults.push(null); } - return false; } @@ -1272,9 +1369,9 @@ function completeAbstractValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue> { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { const resolveTypeFn = returnType.resolveType ?? exeContext.typeResolver; const contextValue = exeContext.contextValue; const runtimeType = resolveTypeFn(result, contextValue, info, returnType); @@ -1295,7 +1392,7 @@ function completeAbstractValue( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ), ); @@ -1315,7 +1412,7 @@ function completeAbstractValue( info, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -1385,9 +1482,9 @@ function completeObjectValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue> { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { // If there is an isTypeOf predicate function, call it with the // current result. If isTypeOf returns false, then raise an error rather // than continuing execution. @@ -1405,7 +1502,7 @@ function completeObjectValue( fieldGroup, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); }); @@ -1422,7 +1519,7 @@ function completeObjectValue( fieldGroup, path, result, - incrementalDataRecord, + incrementalContext, deferMap, ); } @@ -1456,46 +1553,25 @@ function invalidReturnTypeError( * */ function addNewDeferredFragments( - incrementalPublisher: IncrementalPublisher, newDeferUsages: ReadonlyArray, - incrementalDataRecord: IncrementalDataRecord, - deferMap?: ReadonlyMap, + newDeferMap: Map, path?: Path | undefined, ): ReadonlyMap { - if (newDeferUsages.length === 0) { - // Given no DeferUsages, return the existing map, creating one if necessary. - return deferMap ?? new Map(); - } - - // Create a copy of the old map. - const newDeferMap = - deferMap === undefined - ? new Map() - : new Map(deferMap); - // For each new deferUsage object: for (const newDeferUsage of newDeferUsages) { const parentDeferUsage = newDeferUsage.parentDeferUsage; - // If the parent defer usage is not defined, the parent result record is either: - // - the InitialResultRecord, or - // - a StreamItemsRecord, as `@defer` may be nested under `@stream`. const parent = parentDeferUsage === undefined - ? (incrementalDataRecord as InitialResultRecord | StreamItemsRecord) + ? undefined : deferredFragmentRecordFromDeferUsage(parentDeferUsage, newDeferMap); // Instantiate the new record. const deferredFragmentRecord = new DeferredFragmentRecord({ path, label: newDeferUsage.label, - }); - - // Report the new record to the Incremental Publisher. - incrementalPublisher.reportNewDeferFragmentRecord( - deferredFragmentRecord, parent, - ); + }); // Update the map. newDeferMap.set(newDeferUsage, deferredFragmentRecord); @@ -1512,74 +1588,22 @@ function deferredFragmentRecordFromDeferUsage( return deferMap.get(deferUsage)!; } -function addNewDeferredGroupedFieldSets( - incrementalPublisher: IncrementalPublisher, - newGroupedFieldSetDetailsMap: Map, - deferMap: ReadonlyMap, - path?: Path | undefined, -): ReadonlyArray { - const newDeferredGroupedFieldSetRecords: Array = - []; - - for (const [ - deferUsageSet, - { groupedFieldSet, shouldInitiateDefer }, - ] of newGroupedFieldSetDetailsMap) { - const deferredFragmentRecords = getDeferredFragmentRecords( - deferUsageSet, - deferMap, - ); - const deferredGroupedFieldSetRecord = new DeferredGroupedFieldSetRecord({ - path, - deferredFragmentRecords, - groupedFieldSet, - shouldInitiateDefer, - }); - incrementalPublisher.reportNewDeferredGroupedFieldSetRecord( - deferredGroupedFieldSetRecord, - ); - newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord); - } - - return newDeferredGroupedFieldSetRecords; -} - -function getDeferredFragmentRecords( - deferUsages: DeferUsageSet, - deferMap: ReadonlyMap, -): ReadonlyArray { - return Array.from(deferUsages).map((deferUsage) => - deferredFragmentRecordFromDeferUsage(deferUsage, deferMap), - ); -} - function collectAndExecuteSubfields( exeContext: ExecutionContext, returnType: GraphQLObjectType, fieldGroup: FieldGroup, path: Path, result: unknown, - incrementalDataRecord: IncrementalDataRecord, - deferMap: ReadonlyMap, -): PromiseOrValue> { + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { // Collect sub-fields to execute to complete this value. - const { groupedFieldSet, newGroupedFieldSetDetailsMap, newDeferUsages } = + const { groupedFieldSet, newGroupedFieldSets, newDeferUsages } = buildSubFieldPlan(exeContext, returnType, fieldGroup); - const incrementalPublisher = exeContext.incrementalPublisher; - const newDeferMap = addNewDeferredFragments( - incrementalPublisher, newDeferUsages, - incrementalDataRecord, - deferMap, - path, - ); - - const newDeferredGroupedFieldSetRecords = addNewDeferredGroupedFieldSets( - incrementalPublisher, - newGroupedFieldSetDetailsMap, - newDeferMap, + new Map(deferMap), path, ); @@ -1589,20 +1613,23 @@ function collectAndExecuteSubfields( result, path, groupedFieldSet, - incrementalDataRecord, + incrementalContext, newDeferMap, ); - executeDeferredGroupedFieldSets( + const newDeferredGroupedFieldSetRecords = executeDeferredGroupedFieldSets( exeContext, returnType, result, path, - newDeferredGroupedFieldSetRecords, + newGroupedFieldSets, newDeferMap, ); - return subFields; + return withNewDeferredGroupedFieldSets( + subFields, + newDeferredGroupedFieldSetRecords, + ); } /** @@ -1902,142 +1929,333 @@ function executeDeferredGroupedFieldSets( parentType: GraphQLObjectType, sourceValue: unknown, path: Path | undefined, - newDeferredGroupedFieldSetRecords: ReadonlyArray, + newGroupedFieldSets: Map, deferMap: ReadonlyMap, -): void { - for (const deferredGroupedFieldSetRecord of newDeferredGroupedFieldSetRecords) { - if (deferredGroupedFieldSetRecord.shouldInitiateDefer) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Promise.resolve().then(() => +): ReadonlyArray { + const newDeferredGroupedFieldSetRecords: Array = + []; + + for (const [deferUsageSet, groupedFieldSet] of newGroupedFieldSets) { + const deferredFragmentRecords = getDeferredFragmentRecords( + deferUsageSet, + deferMap, + ); + + const deferredGroupedFieldSetRecord = new DeferredGroupedFieldSetRecord({ + path, + deferUsageSet, + deferredFragmentRecords, + executor: (incrementalContext) => executeDeferredGroupedFieldSet( + deferredFragmentRecords, exeContext, parentType, sourceValue, path, - deferredGroupedFieldSetRecord, + groupedFieldSet, + incrementalContext, deferMap, ), - ); - continue; - } + }); - executeDeferredGroupedFieldSet( - exeContext, - parentType, - sourceValue, - path, - deferredGroupedFieldSetRecord, - deferMap, - ); + newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord); } + + return newDeferredGroupedFieldSetRecords; } function executeDeferredGroupedFieldSet( + deferredFragmentRecords: ReadonlyArray, exeContext: ExecutionContext, parentType: GraphQLObjectType, sourceValue: unknown, path: Path | undefined, - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, + groupedFieldSet: GroupedFieldSet, + incrementalContext: IncrementalContext, deferMap: ReadonlyMap, -): void { +): PromiseOrValue { + let result; try { - const incrementalResult = executeFields( + result = executeFields( exeContext, parentType, sourceValue, path, - deferredGroupedFieldSetRecord.groupedFieldSet, - deferredGroupedFieldSetRecord, + groupedFieldSet, + incrementalContext, deferMap, ); + } catch (error) { + return { + deferredFragmentRecords, + path: pathToArray(path), + result: null, + errors: withError(incrementalContext.errors, error), + }; + } - if (isPromise(incrementalResult)) { - incrementalResult.then( - (resolved) => - exeContext.incrementalPublisher.completeDeferredGroupedFieldSet( - deferredGroupedFieldSetRecord, - resolved, - ), - (error) => - exeContext.incrementalPublisher.markErroredDeferredGroupedFieldSet( - deferredGroupedFieldSetRecord, - error, - ), - ); - return; - } - - exeContext.incrementalPublisher.completeDeferredGroupedFieldSet( - deferredGroupedFieldSetRecord, - incrementalResult, + if (isPromise(result)) { + return result.then( + (resolved) => + buildDeferredGroupedFieldSetResult( + incrementalContext, + deferredFragmentRecords, + path, + resolved, + ), + (error) => ({ + deferredFragmentRecords, + path: pathToArray(path), + result: null, + errors: withError(incrementalContext.errors, error), + }), ); + } + + return buildDeferredGroupedFieldSetResult( + incrementalContext, + deferredFragmentRecords, + path, + result, + ); +} + +function buildDeferredGroupedFieldSetResult( + incrementalContext: IncrementalContext, + deferredFragmentRecords: ReadonlyArray, + path: Path | undefined, + result: GraphQLResult>, +): DeferredGroupedFieldSetResult { + const { errors } = incrementalContext; + return { + deferredFragmentRecords, + path: pathToArray(path), + result: + errors.length === 0 ? { data: result[0] } : { data: result[0], errors }, + incrementalDataRecords: result[1], + }; +} + +function getDeferredFragmentRecords( + deferUsages: DeferUsageSet, + deferMap: ReadonlyMap, +): ReadonlyArray { + return Array.from(deferUsages).map((deferUsage) => + deferredFragmentRecordFromDeferUsage(deferUsage, deferMap), + ); +} + +function firstSyncStreamItems( + streamRecord: StreamRecord, + initialItem: PromiseOrValue, + initialIndex: number, + iterator: Iterator, + executor: ( + itemPath: Path, + item: PromiseOrValue, + incrementalContext: IncrementalContext, + ) => PromiseOrValue, +): StreamItemsRecord { + const path = streamRecord.path; + const initialPath = addPath(path, initialIndex, undefined); + + const firstStreamItems = new StreamItemsRecord({ + streamRecord, + itemPath: initialPath, + executor: (incrementalContext) => + Promise.resolve().then(() => { + const firstResult = executor( + initialPath, + initialItem, + incrementalContext, + ); + let currentIndex = initialIndex; + let currentStreamItems = firstStreamItems; + let iteration = iterator.next(); + while (!iteration.done) { + const item = iteration.value; + currentIndex++; + + const currentPath = addPath(path, currentIndex, undefined); + + const nextStreamItems = new StreamItemsRecord({ + streamRecord, + itemPath: currentPath, + executor: (nextIncrementalContext) => + executor(currentPath, item, nextIncrementalContext), + }); + + currentStreamItems.nextStreamItems = nextStreamItems; + currentStreamItems = nextStreamItems; + iteration = iterator.next(); + } + + currentStreamItems.nextStreamItems = new StreamItemsRecord({ + streamRecord, + executor: () => ({ streamRecord }), + }); + + return firstResult; + }), + }); + return firstStreamItems; +} + +function firstAsyncStreamItems( + streamRecord: StreamRecord, + path: Path, + initialIndex: number, + nodes: ReadonlyArray, + asyncIterator: AsyncIterator, + executor: ( + itemPath: Path, + item: PromiseOrValue, + incrementalContext: IncrementalContext, + ) => PromiseOrValue, +): StreamItemsRecord { + const initialPath = addPath(path, initialIndex, undefined); + const firstStreamItems: StreamItemsRecord = new StreamItemsRecord({ + streamRecord, + itemPath: initialPath, + executor: (incrementalContext) => + Promise.resolve().then(() => + getNextAsyncStreamItemsResult( + streamRecord, + firstStreamItems, + path, + initialIndex, + incrementalContext, + nodes, + asyncIterator, + executor, + ), + ), + }); + return firstStreamItems; +} + +async function getNextAsyncStreamItemsResult( + streamRecord: StreamRecord, + initialStreamItemsRecord: StreamItemsRecord, + path: Path, + index: number, + incrementalContext: IncrementalContext, + nodes: ReadonlyArray, + asyncIterator: AsyncIterator, + executor: ( + itemPath: Path, + item: PromiseOrValue, + incrementalContext: IncrementalContext, + ) => PromiseOrValue, +): Promise { + let iteration; + try { + iteration = await asyncIterator.next(); } catch (error) { - exeContext.incrementalPublisher.markErroredDeferredGroupedFieldSet( - deferredGroupedFieldSetRecord, - error, - ); + return { + streamRecord, + result: null, + errors: [locatedError(error, nodes, pathToArray(path))], + }; } + + if (iteration.done) { + return { streamRecord }; + } + + const itemPath = addPath(path, index, undefined); + + const result = executor(itemPath, iteration.value, incrementalContext); + + const nextStreamItems: StreamItemsRecord = nextAsyncStreamItems( + streamRecord, + path, + itemPath, + index, + nodes, + asyncIterator, + executor, + ); + initialStreamItemsRecord.nextStreamItems = nextStreamItems; + + return result; } -function executeStreamField( +function nextAsyncStreamItems( + streamRecord: StreamRecord, path: Path, + initialPath: Path, + initialIndex: number, + nodes: ReadonlyArray, + asyncIterator: AsyncIterator, + executor: ( + itemPath: Path, + item: PromiseOrValue, + incrementalContext: IncrementalContext, + ) => PromiseOrValue, +): StreamItemsRecord { + const nextStreamItems: StreamItemsRecord = new StreamItemsRecord({ + streamRecord, + itemPath: initialPath, + executor: (incrementalContext) => + Promise.resolve().then(() => + getNextAsyncStreamItemsResult( + streamRecord, + nextStreamItems, + path, + initialIndex + 1, + incrementalContext, + nodes, + asyncIterator, + executor, + ), + ), + }); + return nextStreamItems; +} + +function completeStreamItems( + streamRecord: StreamRecord, itemPath: Path, - item: PromiseOrValue, + item: unknown, exeContext: ExecutionContext, + incrementalContext: IncrementalContext, fieldGroup: FieldGroup, info: GraphQLResolveInfo, itemType: GraphQLOutputType, - incrementalDataRecord: IncrementalDataRecord, - streamRecord: StreamRecord, -): StreamItemsRecord { - const incrementalPublisher = exeContext.incrementalPublisher; - const streamItemsRecord = new StreamItemsRecord({ - streamRecord, - path: itemPath, - }); - incrementalPublisher.reportNewStreamItemsRecord( - streamItemsRecord, - incrementalDataRecord, - ); - +): PromiseOrValue { if (isPromise(item)) { - completePromisedValue( + return completePromisedValue( exeContext, itemType, fieldGroup, info, itemPath, item, - streamItemsRecord, + incrementalContext, new Map(), ).then( - (value) => - incrementalPublisher.completeStreamItemsRecord(streamItemsRecord, [ - value, - ]), - (error) => { - incrementalPublisher.filter(path, streamItemsRecord); - incrementalPublisher.markErroredStreamItemsRecord( - streamItemsRecord, - error, - ); - }, + (resolvedItem) => + buildStreamItemsResult(incrementalContext, streamRecord, resolvedItem), + (error) => ({ + streamRecord, + result: null, + errors: withError(incrementalContext.errors, error), + }), ); - - return streamItemsRecord; } - let completedItem: PromiseOrValue; + let result: PromiseOrValue>; try { try { - completedItem = completeValue( + result = completeValue( exeContext, itemType, fieldGroup, info, itemPath, item, - streamItemsRecord, + incrementalContext, new Map(), ); } catch (rawError) { @@ -2047,19 +2265,20 @@ function executeStreamField( itemType, fieldGroup, itemPath, - streamItemsRecord, + incrementalContext, ); - completedItem = null; - incrementalPublisher.filter(itemPath, streamItemsRecord); + result = [null, []]; } } catch (error) { - incrementalPublisher.filter(path, streamItemsRecord); - incrementalPublisher.markErroredStreamItemsRecord(streamItemsRecord, error); - return streamItemsRecord; + return { + streamRecord, + result: null, + errors: withError(incrementalContext.errors, error), + }; } - if (isPromise(completedItem)) { - completedItem + if (isPromise(result)) { + return result .then(undefined, (rawError) => { handleFieldError( rawError, @@ -2067,178 +2286,43 @@ function executeStreamField( itemType, fieldGroup, itemPath, - streamItemsRecord, + incrementalContext, ); - incrementalPublisher.filter(itemPath, streamItemsRecord); - return null; + return [null, []] as GraphQLResult; }) .then( - (value) => - incrementalPublisher.completeStreamItemsRecord(streamItemsRecord, [ - value, - ]), - (error) => { - incrementalPublisher.filter(path, streamItemsRecord); - incrementalPublisher.markErroredStreamItemsRecord( - streamItemsRecord, - error, - ); - }, + (resolvedItem) => + buildStreamItemsResult( + incrementalContext, + streamRecord, + resolvedItem, + ), + (error) => ({ + streamRecord, + result: null, + errors: withError(incrementalContext.errors, error), + }), ); - - return streamItemsRecord; } - incrementalPublisher.completeStreamItemsRecord(streamItemsRecord, [ - completedItem, - ]); - return streamItemsRecord; + return buildStreamItemsResult(incrementalContext, streamRecord, result); } -async function executeStreamAsyncIteratorItem( - asyncIterator: AsyncIterator, - exeContext: ExecutionContext, - fieldGroup: FieldGroup, - info: GraphQLResolveInfo, - itemType: GraphQLOutputType, - streamItemsRecord: StreamItemsRecord, - itemPath: Path, -): Promise> { - let item; - try { - const iteration = await asyncIterator.next(); - if (streamItemsRecord.streamRecord.errors.length > 0) { - return { done: true, value: undefined }; - } - if (iteration.done) { - exeContext.incrementalPublisher.setIsCompletedAsyncIterator( - streamItemsRecord, - ); - return { done: true, value: undefined }; - } - item = iteration.value; - } catch (rawError) { - throw locatedError( - rawError, - toNodes(fieldGroup), - streamItemsRecord.streamRecord.path, - ); - } - let completedItem; - try { - completedItem = completeValue( - exeContext, - itemType, - fieldGroup, - info, - itemPath, - item, - streamItemsRecord, - new Map(), - ); - - if (isPromise(completedItem)) { - completedItem = completedItem.then(undefined, (rawError) => { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - streamItemsRecord, - ); - exeContext.incrementalPublisher.filter(itemPath, streamItemsRecord); - return null; - }); - } - return { done: false, value: completedItem }; - } catch (rawError) { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - streamItemsRecord, - ); - exeContext.incrementalPublisher.filter(itemPath, streamItemsRecord); - return { done: false, value: null }; - } -} - -async function executeStreamAsyncIterator( - initialIndex: number, - asyncIterator: AsyncIterator, - exeContext: ExecutionContext, - fieldGroup: FieldGroup, - info: GraphQLResolveInfo, - itemType: GraphQLOutputType, - path: Path, - incrementalDataRecord: IncrementalDataRecord, +function buildStreamItemsResult( + incrementalContext: IncrementalContext, streamRecord: StreamRecord, -): Promise { - const incrementalPublisher = exeContext.incrementalPublisher; - let index = initialIndex; - let currentIncrementalDataRecord = incrementalDataRecord; - // eslint-disable-next-line no-constant-condition - while (true) { - const itemPath = addPath(path, index, undefined); - const streamItemsRecord = new StreamItemsRecord({ - streamRecord, - path: itemPath, - }); - incrementalPublisher.reportNewStreamItemsRecord( - streamItemsRecord, - currentIncrementalDataRecord, - ); - - let iteration; - try { - // eslint-disable-next-line no-await-in-loop - iteration = await executeStreamAsyncIteratorItem( - asyncIterator, - exeContext, - fieldGroup, - info, - itemType, - streamItemsRecord, - itemPath, - ); - } catch (error) { - incrementalPublisher.filter(path, streamItemsRecord); - incrementalPublisher.markErroredStreamItemsRecord( - streamItemsRecord, - error, - ); - return; - } - - const { done, value: completedItem } = iteration; - - if (isPromise(completedItem)) { - completedItem.then( - (value) => - incrementalPublisher.completeStreamItemsRecord(streamItemsRecord, [ - value, - ]), - (error) => { - incrementalPublisher.filter(path, streamItemsRecord); - incrementalPublisher.markErroredStreamItemsRecord( - streamItemsRecord, - error, - ); - }, - ); - } else { - incrementalPublisher.completeStreamItemsRecord(streamItemsRecord, [ - completedItem, - ]); - } - - if (done) { - break; - } - currentIncrementalDataRecord = streamItemsRecord; - index++; - } + result: GraphQLResult, +): StreamItemsResult { + const { errors } = incrementalContext; + return { + streamRecord, + result: + errors.length === 0 + ? { items: [result[0]] } + : { + items: [result[0]], + errors: [...errors], + }, + incrementalDataRecords: result[1], + }; } diff --git a/src/jsutils/promiseForObject.ts b/src/jsutils/promiseForObject.ts index ff48d9f218..25b3413923 100644 --- a/src/jsutils/promiseForObject.ts +++ b/src/jsutils/promiseForObject.ts @@ -7,9 +7,10 @@ import type { ObjMap } from './ObjMap.js'; * This is akin to bluebird's `Promise.props`, but implemented only using * `Promise.all` so it will work with any implementation of ES6 promises. */ -export async function promiseForObject( +export async function promiseForObject( object: ObjMap>, -): Promise> { + callback: (object: ObjMap) => U, +): Promise { const keys = Object.keys(object); const values = Object.values(object); @@ -18,5 +19,5 @@ export async function promiseForObject( for (let i = 0; i < keys.length; ++i) { resolvedObject[keys[i]] = resolvedValues[i]; } - return resolvedObject; + return callback(resolvedObject); } From bd529751942345ef646712bb98e32fc9b78a9a57 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Apr 2024 22:42:54 +0300 Subject: [PATCH 02/27] remove unnecessary members from incrementalContext turns out we just need errors! --- src/execution/IncrementalPublisher.ts | 38 +---- src/execution/execute.ts | 233 +++++++++----------------- 2 files changed, 90 insertions(+), 181 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 08208a1b58..41fa3d85c0 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -10,8 +10,6 @@ import type { GraphQLFormattedError, } from '../error/GraphQLError.js'; -import type { DeferUsageSet } from './buildFieldPlan.js'; - /** * The result of GraphQL execution. * @@ -660,12 +658,6 @@ function isDeferredGroupedFieldSetRecord( return incrementalDataRecord instanceof DeferredGroupedFieldSetRecord; } -export interface IncrementalContext { - deferUsageSet: DeferUsageSet | undefined; - path: Path | undefined; - errors: Array; -} - export type DeferredGroupedFieldSetResult = | ReconcilableDeferredGroupedFieldSetResult | NonReconcilableDeferredGroupedFieldSetResult; @@ -699,28 +691,18 @@ export function isNonReconcilableDeferredGroupedFieldSetResult( /** @internal */ export class DeferredGroupedFieldSetRecord { - path: Path | undefined; deferredFragmentRecords: ReadonlyArray; result: PromiseOrValue; constructor(opts: { - path: Path | undefined; - deferUsageSet: DeferUsageSet; deferredFragmentRecords: ReadonlyArray; executor: ( - incrementalContext: IncrementalContext, + errors: Array, ) => PromiseOrValue; }) { - const { path, deferUsageSet, deferredFragmentRecords, executor } = opts; - this.path = path; + const { deferredFragmentRecords, executor } = opts; this.deferredFragmentRecords = deferredFragmentRecords; - const incrementalContext: IncrementalContext = { - deferUsageSet, - path, - errors: [], - }; - for (const deferredFragmentRecord of deferredFragmentRecords) { deferredFragmentRecord.deferredGroupedFieldSetRecords.push(this); } @@ -728,8 +710,8 @@ export class DeferredGroupedFieldSetRecord { this.result = deferredFragmentRecords.some( (deferredFragmentRecord) => deferredFragmentRecord.id !== undefined, ) - ? executor(incrementalContext) - : Promise.resolve().then(() => executor(incrementalContext)); + ? executor([]) + : Promise.resolve().then(() => executor([])); } } @@ -816,20 +798,14 @@ export class StreamItemsRecord { constructor(opts: { streamRecord: StreamRecord; - itemPath?: Path | undefined; executor: ( - incrementalContext: IncrementalContext, + errors: Array, ) => PromiseOrValue; }) { - const { streamRecord, itemPath, executor } = opts; + const { streamRecord, executor } = opts; this.streamRecord = streamRecord; - const incrementalContext: IncrementalContext = { - deferUsageSet: undefined, - path: itemPath, - errors: [], - }; - this._result = executor(incrementalContext); + this._result = executor([]); } getResult(): PromiseOrValue { diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 6b2f0f5f15..c0325d331d 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -59,7 +59,6 @@ import type { DeferredGroupedFieldSetResult, ExecutionResult, ExperimentalIncrementalExecutionResults, - IncrementalContext, IncrementalDataRecord, StreamItemsResult, } from './IncrementalPublisher.js'; @@ -143,7 +142,6 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; - errors: Array; cancellableStreams: Set; } @@ -259,6 +257,7 @@ export function experimentalExecuteIncrementally( function executeOperation( exeContext: ExecutionContext, ): PromiseOrValue { + const errors: Array = []; try { const { operation, schema, fragments, variableValues, rootValue } = exeContext; @@ -287,6 +286,7 @@ function executeOperation( rootType, rootValue, groupedFieldSet, + errors, newDeferMap, ); @@ -305,16 +305,17 @@ function executeOperation( ); if (isPromise(acc)) { return acc.then( - (resolved) => buildDataResponse(exeContext, resolved[0], resolved[1]), + (resolved) => + buildDataResponse(exeContext, resolved[0], errors, resolved[1]), (error) => ({ data: null, - errors: withError(exeContext.errors, error), + errors: withError(errors, error), }), ); } - return buildDataResponse(exeContext, acc[0], acc[1]); + return buildDataResponse(exeContext, acc[0], errors, acc[1]); } catch (error) { - return { data: null, errors: withError(exeContext.errors, error) }; + return { data: null, errors: withError(errors, error) }; } } @@ -355,9 +356,9 @@ function withError( function buildDataResponse( exeContext: ExecutionContext, data: ObjMap, + errors: ReadonlyArray, incrementalDataRecords: ReadonlyArray, ): ExecutionResult | ExperimentalIncrementalExecutionResults { - const { errors } = exeContext; if (incrementalDataRecords.length === 0) { return errors.length > 0 ? { errors, data } : { data }; } @@ -471,7 +472,6 @@ export function buildExecutionContext( fieldResolver: fieldResolver ?? defaultFieldResolver, typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, - errors: [], cancellableStreams: new Set(), }; } @@ -483,7 +483,6 @@ function buildPerEventExecutionContext( return { ...exeContext, rootValue: payload, - errors: [], }; } @@ -493,6 +492,7 @@ function executeRootGroupedFieldSet( rootType: GraphQLObjectType, rootValue: unknown, groupedFieldSet: GroupedFieldSet, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { switch (operation) { @@ -503,7 +503,7 @@ function executeRootGroupedFieldSet( rootValue, undefined, groupedFieldSet, - undefined, + errors, deferMap, ); case OperationTypeNode.MUTATION: @@ -513,6 +513,7 @@ function executeRootGroupedFieldSet( rootValue, undefined, groupedFieldSet, + errors, deferMap, ); case OperationTypeNode.SUBSCRIPTION: @@ -524,7 +525,7 @@ function executeRootGroupedFieldSet( rootValue, undefined, groupedFieldSet, - undefined, + errors, deferMap, ); } @@ -540,6 +541,7 @@ function executeFieldsSerially( sourceValue: unknown, path: Path | undefined, groupedFieldSet: GroupedFieldSet, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { return promiseReduce( @@ -552,7 +554,7 @@ function executeFieldsSerially( sourceValue, fieldGroup, fieldPath, - undefined, + errors, deferMap, ); if (result === undefined) { @@ -583,7 +585,7 @@ function executeFields( sourceValue: unknown, path: Path | undefined, groupedFieldSet: GroupedFieldSet, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const results = Object.create(null); @@ -599,7 +601,7 @@ function executeFields( sourceValue, fieldGroup, fieldPath, - incrementalContext, + errors, deferMap, ); @@ -655,7 +657,7 @@ function executeField( source: unknown, fieldGroup: FieldGroup, path: Path, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue> | undefined { const fieldName = fieldGroup.fields[0].node.name.value; @@ -701,7 +703,7 @@ function executeField( info, path, result, - incrementalContext, + errors, deferMap, ); } @@ -713,7 +715,7 @@ function executeField( info, path, result, - incrementalContext, + errors, deferMap, ); @@ -721,27 +723,13 @@ function executeField( // Note: we don't rely on a `catch` method, but we do expect "thenable" // to take a second callback for the error case. return completed.then(undefined, (rawError) => { - handleFieldError( - rawError, - exeContext, - returnType, - fieldGroup, - path, - incrementalContext, - ); + handleFieldError(rawError, returnType, fieldGroup, path, errors); return [null, []]; }); } return completed; } catch (rawError) { - handleFieldError( - rawError, - exeContext, - returnType, - fieldGroup, - path, - incrementalContext, - ); + handleFieldError(rawError, returnType, fieldGroup, path, errors); return [null, []]; } } @@ -775,11 +763,10 @@ export function buildResolveInfo( function handleFieldError( rawError: unknown, - exeContext: ExecutionContext, returnType: GraphQLOutputType, fieldGroup: FieldGroup, path: Path, - incrementalContext: IncrementalContext | undefined, + errors: Array, ): void { const error = locatedError(rawError, toNodes(fieldGroup), pathToArray(path)); @@ -791,7 +778,7 @@ function handleFieldError( // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. - (incrementalContext ?? exeContext).errors.push(error); + errors.push(error); } /** @@ -822,7 +809,7 @@ function completeValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue> { // If result is an Error, throw a located error. @@ -840,7 +827,7 @@ function completeValue( info, path, result, - incrementalContext, + errors, deferMap, ); if ((completed as GraphQLResult)[0] === null) { @@ -865,7 +852,7 @@ function completeValue( info, path, result, - incrementalContext, + errors, deferMap, ); } @@ -886,7 +873,7 @@ function completeValue( info, path, result, - incrementalContext, + errors, deferMap, ); } @@ -900,7 +887,7 @@ function completeValue( info, path, result, - incrementalContext, + errors, deferMap, ); } @@ -919,7 +906,7 @@ async function completePromisedValue( info: GraphQLResolveInfo, path: Path, result: Promise, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): Promise> { try { @@ -931,7 +918,7 @@ async function completePromisedValue( info, path, resolved, - incrementalContext, + errors, deferMap, ); @@ -940,14 +927,7 @@ async function completePromisedValue( } return completed; } catch (rawError) { - handleFieldError( - rawError, - exeContext, - returnType, - fieldGroup, - path, - incrementalContext, - ); + handleFieldError(rawError, returnType, fieldGroup, path, errors); return [null, []]; } } @@ -1038,7 +1018,7 @@ async function completeAsyncIteratorValue( info: GraphQLResolveInfo, path: Path, asyncIterator: AsyncIterator, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): Promise>> { let containsPromise = false; @@ -1107,7 +1087,7 @@ async function completeAsyncIteratorValue( info, itemPath, item, - incrementalContext, + errors, deferMap, ).then((resolved) => { appendNewIncrementalDataRecords(acc, resolved[1]); @@ -1126,7 +1106,7 @@ async function completeAsyncIteratorValue( fieldGroup, info, itemPath, - incrementalContext, + errors, deferMap, ) // TODO: add tests for stream backed by asyncIterator that completes to a promise @@ -1157,7 +1137,7 @@ function completeListValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const itemType = returnType.ofType; @@ -1172,7 +1152,7 @@ function completeListValue( info, path, asyncIterator, - incrementalContext, + errors, deferMap, ); } @@ -1236,7 +1216,7 @@ function completeListValue( info, itemPath, item, - incrementalContext, + errors, deferMap, ).then((resolved) => { appendNewIncrementalDataRecords(acc, resolved[1]); @@ -1254,7 +1234,7 @@ function completeListValue( fieldGroup, info, itemPath, - incrementalContext, + errors, deferMap, ) ) { @@ -1284,7 +1264,7 @@ function completeListItemValue( fieldGroup: FieldGroup, info: GraphQLResolveInfo, itemPath: Path, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): boolean { try { @@ -1295,7 +1275,7 @@ function completeListItemValue( info, itemPath, item, - incrementalContext, + errors, deferMap, ); @@ -1309,14 +1289,7 @@ function completeListItemValue( return resolved[0]; }, (rawError) => { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - incrementalContext, - ); + handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); return null; }, ), @@ -1327,14 +1300,7 @@ function completeListItemValue( completedResults.push(completedItem[0]); appendNewIncrementalDataRecords(parent, completedItem[1]); } catch (rawError) { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - incrementalContext, - ); + handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); completedResults.push(null); } return false; @@ -1369,7 +1335,7 @@ function completeAbstractValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const resolveTypeFn = returnType.resolveType ?? exeContext.typeResolver; @@ -1392,7 +1358,7 @@ function completeAbstractValue( info, path, result, - incrementalContext, + errors, deferMap, ), ); @@ -1412,7 +1378,7 @@ function completeAbstractValue( info, path, result, - incrementalContext, + errors, deferMap, ); } @@ -1482,7 +1448,7 @@ function completeObjectValue( info: GraphQLResolveInfo, path: Path, result: unknown, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { // If there is an isTypeOf predicate function, call it with the @@ -1502,7 +1468,7 @@ function completeObjectValue( fieldGroup, path, result, - incrementalContext, + errors, deferMap, ); }); @@ -1519,7 +1485,7 @@ function completeObjectValue( fieldGroup, path, result, - incrementalContext, + errors, deferMap, ); } @@ -1594,7 +1560,7 @@ function collectAndExecuteSubfields( fieldGroup: FieldGroup, path: Path, result: unknown, - incrementalContext: IncrementalContext | undefined, + errors: Array, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { // Collect sub-fields to execute to complete this value. @@ -1613,7 +1579,7 @@ function collectAndExecuteSubfields( result, path, groupedFieldSet, - incrementalContext, + errors, newDeferMap, ); @@ -1942,10 +1908,8 @@ function executeDeferredGroupedFieldSets( ); const deferredGroupedFieldSetRecord = new DeferredGroupedFieldSetRecord({ - path, - deferUsageSet, deferredFragmentRecords, - executor: (incrementalContext) => + executor: (errors) => executeDeferredGroupedFieldSet( deferredFragmentRecords, exeContext, @@ -1953,7 +1917,7 @@ function executeDeferredGroupedFieldSets( sourceValue, path, groupedFieldSet, - incrementalContext, + errors, deferMap, ), }); @@ -1971,7 +1935,7 @@ function executeDeferredGroupedFieldSet( sourceValue: unknown, path: Path | undefined, groupedFieldSet: GroupedFieldSet, - incrementalContext: IncrementalContext, + errors: Array, deferMap: ReadonlyMap, ): PromiseOrValue { let result; @@ -1982,7 +1946,7 @@ function executeDeferredGroupedFieldSet( sourceValue, path, groupedFieldSet, - incrementalContext, + errors, deferMap, ); } catch (error) { @@ -1990,7 +1954,7 @@ function executeDeferredGroupedFieldSet( deferredFragmentRecords, path: pathToArray(path), result: null, - errors: withError(incrementalContext.errors, error), + errors: withError(errors, error), }; } @@ -1998,7 +1962,7 @@ function executeDeferredGroupedFieldSet( return result.then( (resolved) => buildDeferredGroupedFieldSetResult( - incrementalContext, + errors, deferredFragmentRecords, path, resolved, @@ -2007,13 +1971,13 @@ function executeDeferredGroupedFieldSet( deferredFragmentRecords, path: pathToArray(path), result: null, - errors: withError(incrementalContext.errors, error), + errors: withError(errors, error), }), ); } return buildDeferredGroupedFieldSetResult( - incrementalContext, + errors, deferredFragmentRecords, path, result, @@ -2021,12 +1985,11 @@ function executeDeferredGroupedFieldSet( } function buildDeferredGroupedFieldSetResult( - incrementalContext: IncrementalContext, + errors: ReadonlyArray, deferredFragmentRecords: ReadonlyArray, path: Path | undefined, result: GraphQLResult>, ): DeferredGroupedFieldSetResult { - const { errors } = incrementalContext; return { deferredFragmentRecords, path: pathToArray(path), @@ -2053,7 +2016,7 @@ function firstSyncStreamItems( executor: ( itemPath: Path, item: PromiseOrValue, - incrementalContext: IncrementalContext, + errors: Array, ) => PromiseOrValue, ): StreamItemsRecord { const path = streamRecord.path; @@ -2061,14 +2024,9 @@ function firstSyncStreamItems( const firstStreamItems = new StreamItemsRecord({ streamRecord, - itemPath: initialPath, - executor: (incrementalContext) => + executor: (errors) => Promise.resolve().then(() => { - const firstResult = executor( - initialPath, - initialItem, - incrementalContext, - ); + const firstResult = executor(initialPath, initialItem, errors); let currentIndex = initialIndex; let currentStreamItems = firstStreamItems; let iteration = iterator.next(); @@ -2080,7 +2038,6 @@ function firstSyncStreamItems( const nextStreamItems = new StreamItemsRecord({ streamRecord, - itemPath: currentPath, executor: (nextIncrementalContext) => executor(currentPath, item, nextIncrementalContext), }); @@ -2110,21 +2067,19 @@ function firstAsyncStreamItems( executor: ( itemPath: Path, item: PromiseOrValue, - incrementalContext: IncrementalContext, + errors: Array, ) => PromiseOrValue, ): StreamItemsRecord { - const initialPath = addPath(path, initialIndex, undefined); const firstStreamItems: StreamItemsRecord = new StreamItemsRecord({ streamRecord, - itemPath: initialPath, - executor: (incrementalContext) => + executor: (errors) => Promise.resolve().then(() => getNextAsyncStreamItemsResult( streamRecord, firstStreamItems, path, initialIndex, - incrementalContext, + errors, nodes, asyncIterator, executor, @@ -2139,13 +2094,13 @@ async function getNextAsyncStreamItemsResult( initialStreamItemsRecord: StreamItemsRecord, path: Path, index: number, - incrementalContext: IncrementalContext, + errors: Array, nodes: ReadonlyArray, asyncIterator: AsyncIterator, executor: ( itemPath: Path, item: PromiseOrValue, - incrementalContext: IncrementalContext, + errors: Array, ) => PromiseOrValue, ): Promise { let iteration; @@ -2165,12 +2120,11 @@ async function getNextAsyncStreamItemsResult( const itemPath = addPath(path, index, undefined); - const result = executor(itemPath, iteration.value, incrementalContext); + const result = executor(itemPath, iteration.value, errors); const nextStreamItems: StreamItemsRecord = nextAsyncStreamItems( streamRecord, path, - itemPath, index, nodes, asyncIterator, @@ -2184,27 +2138,25 @@ async function getNextAsyncStreamItemsResult( function nextAsyncStreamItems( streamRecord: StreamRecord, path: Path, - initialPath: Path, initialIndex: number, nodes: ReadonlyArray, asyncIterator: AsyncIterator, executor: ( itemPath: Path, item: PromiseOrValue, - incrementalContext: IncrementalContext, + errors: Array, ) => PromiseOrValue, ): StreamItemsRecord { const nextStreamItems: StreamItemsRecord = new StreamItemsRecord({ streamRecord, - itemPath: initialPath, - executor: (incrementalContext) => + executor: (errors) => Promise.resolve().then(() => getNextAsyncStreamItemsResult( streamRecord, nextStreamItems, path, initialIndex + 1, - incrementalContext, + errors, nodes, asyncIterator, executor, @@ -2219,7 +2171,7 @@ function completeStreamItems( itemPath: Path, item: unknown, exeContext: ExecutionContext, - incrementalContext: IncrementalContext, + errors: Array, fieldGroup: FieldGroup, info: GraphQLResolveInfo, itemType: GraphQLOutputType, @@ -2232,15 +2184,15 @@ function completeStreamItems( info, itemPath, item, - incrementalContext, + errors, new Map(), ).then( (resolvedItem) => - buildStreamItemsResult(incrementalContext, streamRecord, resolvedItem), + buildStreamItemsResult(errors, streamRecord, resolvedItem), (error) => ({ streamRecord, result: null, - errors: withError(incrementalContext.errors, error), + errors: withError(errors, error), }), ); } @@ -2255,65 +2207,46 @@ function completeStreamItems( info, itemPath, item, - incrementalContext, + errors, new Map(), ); } catch (rawError) { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - incrementalContext, - ); + handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); result = [null, []]; } } catch (error) { return { streamRecord, result: null, - errors: withError(incrementalContext.errors, error), + errors: withError(errors, error), }; } if (isPromise(result)) { return result .then(undefined, (rawError) => { - handleFieldError( - rawError, - exeContext, - itemType, - fieldGroup, - itemPath, - incrementalContext, - ); + handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); return [null, []] as GraphQLResult; }) .then( (resolvedItem) => - buildStreamItemsResult( - incrementalContext, - streamRecord, - resolvedItem, - ), + buildStreamItemsResult(errors, streamRecord, resolvedItem), (error) => ({ streamRecord, result: null, - errors: withError(incrementalContext.errors, error), + errors: withError(errors, error), }), ); } - return buildStreamItemsResult(incrementalContext, streamRecord, result); + return buildStreamItemsResult(errors, streamRecord, result); } function buildStreamItemsResult( - incrementalContext: IncrementalContext, + errors: ReadonlyArray, streamRecord: StreamRecord, result: GraphQLResult, ): StreamItemsResult { - const { errors } = incrementalContext; return { streamRecord, result: From 25af8dc46d5ca054375041a61f3005544b2a8fea Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Apr 2024 22:46:04 +0300 Subject: [PATCH 03/27] add comment explaining strategy for when we add an extra tick --- src/execution/IncrementalPublisher.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 41fa3d85c0..53daf45cca 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -707,6 +707,13 @@ export class DeferredGroupedFieldSetRecord { deferredFragmentRecord.deferredGroupedFieldSetRecords.push(this); } + // If any of the deferred fragments for this deferred grouped field set + // have an id, then they have been released to the client as pending + // and it is safe to execute the deferred grouped field set synchronously. + + // This can occur, for example, when deferred fragments have overlapping + // fields, and a new deferred grouped field set has been created for the + // non-overlapping fields. this.result = deferredFragmentRecords.some( (deferredFragmentRecord) => deferredFragmentRecord.id !== undefined, ) From 056c344d978d9e1c87f3e7917b9a5c0982bf927f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Apr 2024 22:51:41 +0300 Subject: [PATCH 04/27] add comment --- src/execution/IncrementalPublisher.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 53daf45cca..d742708919 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -281,6 +281,9 @@ class IncrementalPublisher { ): void { const parent = deferredFragmentRecord.parent; if (parent === undefined) { + // Below is equivalent and slightly faster version of: + // if (this._pending.has(deferredFragmentRecord)) { ... } + // as all released deferredFragmentRecords have ids. if (deferredFragmentRecord.id !== undefined) { return; } From 1105ee16a96db34c9388600c2983e5d050387966 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 10 Apr 2024 22:52:52 +0300 Subject: [PATCH 05/27] drop check for simpllicity --- src/execution/IncrementalPublisher.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index d742708919..74d3765ed2 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -517,11 +517,9 @@ class IncrementalPublisher { ); } - if (deferredGroupedFieldSetResult.incrementalDataRecords.length > 0) { - this._addIncrementalDataRecords( - deferredGroupedFieldSetResult.incrementalDataRecords, - ); - } + this._addIncrementalDataRecords( + deferredGroupedFieldSetResult.incrementalDataRecords, + ); for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) { const id = deferredFragmentRecord.id; From 6cfd1f08b2c336c25eb46d4fabc71816a16adec1 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 14 Apr 2024 15:50:45 +0300 Subject: [PATCH 06/27] remove last traces of IncrementalContext --- src/execution/IncrementalPublisher.ts | 14 +++++------- src/execution/execute.ts | 32 ++++++++++----------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 74d3765ed2..8d459fc1bf 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -697,9 +697,7 @@ export class DeferredGroupedFieldSetRecord { constructor(opts: { deferredFragmentRecords: ReadonlyArray; - executor: ( - errors: Array, - ) => PromiseOrValue; + executor: () => PromiseOrValue; }) { const { deferredFragmentRecords, executor } = opts; this.deferredFragmentRecords = deferredFragmentRecords; @@ -718,8 +716,8 @@ export class DeferredGroupedFieldSetRecord { this.result = deferredFragmentRecords.some( (deferredFragmentRecord) => deferredFragmentRecord.id !== undefined, ) - ? executor([]) - : Promise.resolve().then(() => executor([])); + ? executor() + : Promise.resolve().then(executor); } } @@ -806,14 +804,12 @@ export class StreamItemsRecord { constructor(opts: { streamRecord: StreamRecord; - executor: ( - errors: Array, - ) => PromiseOrValue; + executor: () => PromiseOrValue; }) { const { streamRecord, executor } = opts; this.streamRecord = streamRecord; - this._result = executor([]); + this._result = executor(); } getResult(): PromiseOrValue { diff --git a/src/execution/execute.ts b/src/execution/execute.ts index c0325d331d..7100ca246b 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1043,13 +1043,13 @@ async function completeAsyncIteratorValue( index, toNodes(fieldGroup), asyncIterator, - (currentItemPath, currentItem, currentIncrementalContext) => + (currentItemPath, currentItem) => completeStreamItems( streamRecord, currentItemPath, currentItem, exeContext, - currentIncrementalContext, + [], streamUsage.fieldGroup, info, itemType, @@ -1186,13 +1186,13 @@ function completeListValue( item, index, iterator, - (currentItemPath, currentItem, currentIncrementalContext) => + (currentItemPath, currentItem) => completeStreamItems( streamRecord, currentItemPath, currentItem, exeContext, - currentIncrementalContext, + [], streamUsage.fieldGroup, info, itemType, @@ -1909,7 +1909,7 @@ function executeDeferredGroupedFieldSets( const deferredGroupedFieldSetRecord = new DeferredGroupedFieldSetRecord({ deferredFragmentRecords, - executor: (errors) => + executor: () => executeDeferredGroupedFieldSet( deferredFragmentRecords, exeContext, @@ -1917,7 +1917,7 @@ function executeDeferredGroupedFieldSets( sourceValue, path, groupedFieldSet, - errors, + [], deferMap, ), }); @@ -2016,7 +2016,6 @@ function firstSyncStreamItems( executor: ( itemPath: Path, item: PromiseOrValue, - errors: Array, ) => PromiseOrValue, ): StreamItemsRecord { const path = streamRecord.path; @@ -2024,9 +2023,9 @@ function firstSyncStreamItems( const firstStreamItems = new StreamItemsRecord({ streamRecord, - executor: (errors) => + executor: () => Promise.resolve().then(() => { - const firstResult = executor(initialPath, initialItem, errors); + const firstResult = executor(initialPath, initialItem); let currentIndex = initialIndex; let currentStreamItems = firstStreamItems; let iteration = iterator.next(); @@ -2038,8 +2037,7 @@ function firstSyncStreamItems( const nextStreamItems = new StreamItemsRecord({ streamRecord, - executor: (nextIncrementalContext) => - executor(currentPath, item, nextIncrementalContext), + executor: () => executor(currentPath, item), }); currentStreamItems.nextStreamItems = nextStreamItems; @@ -2067,19 +2065,17 @@ function firstAsyncStreamItems( executor: ( itemPath: Path, item: PromiseOrValue, - errors: Array, ) => PromiseOrValue, ): StreamItemsRecord { const firstStreamItems: StreamItemsRecord = new StreamItemsRecord({ streamRecord, - executor: (errors) => + executor: () => Promise.resolve().then(() => getNextAsyncStreamItemsResult( streamRecord, firstStreamItems, path, initialIndex, - errors, nodes, asyncIterator, executor, @@ -2094,13 +2090,11 @@ async function getNextAsyncStreamItemsResult( initialStreamItemsRecord: StreamItemsRecord, path: Path, index: number, - errors: Array, nodes: ReadonlyArray, asyncIterator: AsyncIterator, executor: ( itemPath: Path, item: PromiseOrValue, - errors: Array, ) => PromiseOrValue, ): Promise { let iteration; @@ -2120,7 +2114,7 @@ async function getNextAsyncStreamItemsResult( const itemPath = addPath(path, index, undefined); - const result = executor(itemPath, iteration.value, errors); + const result = executor(itemPath, iteration.value); const nextStreamItems: StreamItemsRecord = nextAsyncStreamItems( streamRecord, @@ -2144,19 +2138,17 @@ function nextAsyncStreamItems( executor: ( itemPath: Path, item: PromiseOrValue, - errors: Array, ) => PromiseOrValue, ): StreamItemsRecord { const nextStreamItems: StreamItemsRecord = new StreamItemsRecord({ streamRecord, - executor: (errors) => + executor: () => Promise.resolve().then(() => getNextAsyncStreamItemsResult( streamRecord, nextStreamItems, path, initialIndex + 1, - errors, nodes, asyncIterator, executor, From 3edba1a9d3cade48239332675a53e48d659af911 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 14 Apr 2024 22:30:05 +0300 Subject: [PATCH 07/27] simplify stream handling --- src/execution/IncrementalPublisher.ts | 31 ++--------- src/execution/__tests__/stream-test.ts | 62 +++++++++------------ src/execution/execute.ts | 75 +++++++++++++++++++------- 3 files changed, 82 insertions(+), 86 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 8d459fc1bf..42bb023b7c 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -264,7 +264,7 @@ class IncrementalPublisher { this._newPending.add(streamRecord); } - const result = incrementalDataRecord.getResult(); + const result = incrementalDataRecord.result; if (isPromise(result)) { // eslint-disable-next-line @typescript-eslint/no-floating-promises result.then((resolved) => { @@ -800,7 +800,7 @@ export class StreamItemsRecord { streamRecord: StreamRecord; nextStreamItems: StreamItemsRecord | undefined; - private _result: PromiseOrValue; + result: PromiseOrValue; constructor(opts: { streamRecord: StreamRecord; @@ -809,32 +809,7 @@ export class StreamItemsRecord { const { streamRecord, executor } = opts; this.streamRecord = streamRecord; - this._result = executor(); - } - - getResult(): PromiseOrValue { - if (isPromise(this._result)) { - return this._result.then((resolved) => - this._prependNextStreamItems(resolved), - ); - } - - return this._prependNextStreamItems(this._result); - } - - private _prependNextStreamItems( - result: StreamItemsResult, - ): StreamItemsResult { - return isNonTerminatingStreamItemsResult(result) && - this.nextStreamItems !== undefined - ? { - ...result, - incrementalDataRecords: - result.incrementalDataRecords.length === 0 - ? [this.nextStreamItems] - : [this.nextStreamItems, ...result.incrementalDataRecords], - } - : result; + this.result = executor(); } } diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index 9c0a7ed22b..bf0383ce48 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -1455,17 +1455,12 @@ describe('Execute: stream directive', () => { }, ], }, - ], - completed: [{ id: '0' }], - hasNext: true, - }, - { - incremental: [ { items: [{ name: 'Luke' }], id: '1', }, ], + completed: [{ id: '0' }], hasNext: true, }, { @@ -1947,46 +1942,37 @@ describe('Execute: stream directive', () => { const result2 = await iterator.next(); expectJSON(result2).toDeepEqual({ value: { + pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], incremental: [ { data: { name: 'Luke' }, id: '0', }, - ], - completed: [{ id: '0' }], - hasNext: true, - }, - done: false, - }); - - const result3 = await iterator.next(); - expectJSON(result3).toDeepEqual({ - value: { - pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], - incremental: [ { items: [{ id: '2' }], id: '1', }, ], + completed: [{ id: '0' }], hasNext: true, }, done: false, }); - const result4Promise = iterator.next(); + + const result3Promise = iterator.next(); resolveIterableCompletion(null); - const result4 = await result4Promise; - expectJSON(result4).toDeepEqual({ + const result3 = await result3Promise; + expectJSON(result3).toDeepEqual({ value: { completed: [{ id: '1' }], hasNext: true, }, done: false, }); - const result5Promise = iterator.next(); + const result4Promise = iterator.next(); resolveSlowField('Han'); - const result5 = await result5Promise; - expectJSON(result5).toDeepEqual({ + const result4 = await result4Promise; + expectJSON(result4).toDeepEqual({ value: { incremental: [ { @@ -1999,8 +1985,8 @@ describe('Execute: stream directive', () => { }, done: false, }); - const result6 = await iterator.next(); - expectJSON(result6).toDeepEqual({ + const result5 = await iterator.next(); + expectJSON(result5).toDeepEqual({ value: undefined, done: true, }); @@ -2059,11 +2045,16 @@ describe('Execute: stream directive', () => { const result2 = await result2Promise; expectJSON(result2).toDeepEqual({ value: { + pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], incremental: [ { data: { name: 'Luke' }, id: '0', }, + { + items: [{ id: '2' }], + id: '1', + }, ], completed: [{ id: '0' }], hasNext: true, @@ -2071,23 +2062,18 @@ describe('Execute: stream directive', () => { done: false, }); - const result3 = await iterator.next(); + const result3Promise = iterator.next(); + resolveIterableCompletion(null); + const result3 = await result3Promise; expectJSON(result3).toDeepEqual({ value: { - pending: [{ id: '2', path: ['friendList', 1], label: 'DeferName' }], - incremental: [ - { - items: [{ id: '2' }], - id: '1', - }, - ], + completed: [{ id: '1' }], hasNext: true, }, done: false, }); - const result4Promise = iterator.next(); - resolveIterableCompletion(null); - const result4 = await result4Promise; + + const result4 = await iterator.next(); expectJSON(result4).toDeepEqual({ value: { incremental: [ @@ -2096,7 +2082,7 @@ describe('Execute: stream directive', () => { id: '2', }, ], - completed: [{ id: '2' }, { id: '1' }], + completed: [{ id: '2' }], hasNext: false, }, done: false, diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 7100ca246b..e82dea5c7b 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -66,6 +66,7 @@ import { buildIncrementalResponse, DeferredFragmentRecord, DeferredGroupedFieldSetRecord, + isNonTerminatingStreamItemsResult, StreamItemsRecord, StreamRecord, } from './IncrementalPublisher.js'; @@ -2025,37 +2026,75 @@ function firstSyncStreamItems( streamRecord, executor: () => Promise.resolve().then(() => { - const firstResult = executor(initialPath, initialItem); + const results: Array> = [ + executor(initialPath, initialItem), + ]; let currentIndex = initialIndex; - let currentStreamItems = firstStreamItems; let iteration = iterator.next(); while (!iteration.done) { const item = iteration.value; currentIndex++; - const currentPath = addPath(path, currentIndex, undefined); + results.push(executor(currentPath, item)); + iteration = iterator.next(); + } - const nextStreamItems = new StreamItemsRecord({ + let i = results.length - 1; + let currentResult = prependNextStreamItems( + results[i], + new StreamItemsRecord({ streamRecord, - executor: () => executor(currentPath, item), - }); + executor: () => ({ streamRecord }), + }), + ); - currentStreamItems.nextStreamItems = nextStreamItems; - currentStreamItems = nextStreamItems; - iteration = iterator.next(); + while (i > 0) { + const index = i--; + currentResult = prependNextStreamItems( + results[index - 1], + new StreamItemsRecord({ + streamRecord, + // safe as this function is executed immediately + // eslint-disable-next-line @typescript-eslint/no-loop-func + executor: () => currentResult, + }), + ); } - currentStreamItems.nextStreamItems = new StreamItemsRecord({ - streamRecord, - executor: () => ({ streamRecord }), - }); - - return firstResult; + return currentResult; }), }); return firstStreamItems; } +function prependNextStreamItems( + result: PromiseOrValue, + nextStreamItems: StreamItemsRecord, +): PromiseOrValue { + if (isPromise(result)) { + return result.then((resolved) => + isNonTerminatingStreamItemsResult(resolved) + ? { + ...resolved, + incrementalDataRecords: [ + nextStreamItems, + ...resolved.incrementalDataRecords, + ], + } + : resolved, + ); + } + return isNonTerminatingStreamItemsResult(result) + ? { + ...result, + incrementalDataRecords: [ + nextStreamItems, + ...result.incrementalDataRecords, + ], + } + : result; +} + function firstAsyncStreamItems( streamRecord: StreamRecord, path: Path, @@ -2073,7 +2112,6 @@ function firstAsyncStreamItems( Promise.resolve().then(() => getNextAsyncStreamItemsResult( streamRecord, - firstStreamItems, path, initialIndex, nodes, @@ -2087,7 +2125,6 @@ function firstAsyncStreamItems( async function getNextAsyncStreamItemsResult( streamRecord: StreamRecord, - initialStreamItemsRecord: StreamItemsRecord, path: Path, index: number, nodes: ReadonlyArray, @@ -2124,9 +2161,8 @@ async function getNextAsyncStreamItemsResult( asyncIterator, executor, ); - initialStreamItemsRecord.nextStreamItems = nextStreamItems; - return result; + return prependNextStreamItems(result, nextStreamItems); } function nextAsyncStreamItems( @@ -2146,7 +2182,6 @@ function nextAsyncStreamItems( Promise.resolve().then(() => getNextAsyncStreamItemsResult( streamRecord, - nextStreamItems, path, initialIndex + 1, nodes, From b39435aa6b3b4e28f81828a84ff67f2d29896f70 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 18:13:01 +0300 Subject: [PATCH 08/27] remove unnecessary type --- src/execution/buildFieldPlan.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/execution/buildFieldPlan.ts b/src/execution/buildFieldPlan.ts index 55ac0d5dc1..970b8d5c46 100644 --- a/src/execution/buildFieldPlan.ts +++ b/src/execution/buildFieldPlan.ts @@ -12,11 +12,6 @@ export interface FieldGroup { export type GroupedFieldSet = Map; -export interface NewGroupedFieldSetDetails { - groupedFieldSet: GroupedFieldSet; - shouldInitiateDefer: boolean; -} - export function buildFieldPlan( fields: Map>, parentDeferUsages: DeferUsageSet = new Set(), From 537ce5f670f3c04a53f16d8d81658247dbbd6954 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 18:14:13 +0300 Subject: [PATCH 09/27] refactor classes into interfaces --- src/execution/IncrementalPublisher.ts | 91 ++++---------- src/execution/execute.ts | 171 ++++++++++++++------------ 2 files changed, 111 insertions(+), 151 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 42bb023b7c..f0c1feac39 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -243,6 +243,8 @@ class IncrementalPublisher { for (const incrementalDataRecord of incrementalDataRecords) { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) { + deferredFragmentRecord.expectedReconcilableResults++; + this._addDeferredFragmentRecord(deferredFragmentRecord); } @@ -306,7 +308,7 @@ class IncrementalPublisher { this._newPending = new Set(); for (const node of maybeEmptyNewPending) { if (isDeferredFragmentRecord(node)) { - if (node.deferredGroupedFieldSetRecords.length > 0) { + if (node.expectedReconcilableResults) { this._newPending.add(node); continue; } @@ -322,7 +324,7 @@ class IncrementalPublisher { private _addNonEmptyNewPending( deferredFragmentRecord: DeferredFragmentRecord, ): void { - if (deferredFragmentRecord.deferredGroupedFieldSetRecords.length > 0) { + if (deferredFragmentRecord.expectedReconcilableResults) { this._newPending.add(deferredFragmentRecord); return; } @@ -532,7 +534,7 @@ class IncrementalPublisher { } const fragmentResults = deferredFragmentRecord.reconcilableResults; if ( - deferredFragmentRecord.deferredGroupedFieldSetRecords.length !== + deferredFragmentRecord.expectedReconcilableResults !== fragmentResults.length ) { continue; @@ -650,13 +652,13 @@ class IncrementalPublisher { function isDeferredFragmentRecord( subsequentResultRecord: SubsequentResultRecord, ): subsequentResultRecord is DeferredFragmentRecord { - return subsequentResultRecord instanceof DeferredFragmentRecord; + return 'parent' in subsequentResultRecord; } function isDeferredGroupedFieldSetRecord( incrementalDataRecord: IncrementalDataRecord, ): incrementalDataRecord is DeferredGroupedFieldSetRecord { - return incrementalDataRecord instanceof DeferredGroupedFieldSetRecord; + return 'deferredFragmentRecords' in incrementalDataRecord; } export type DeferredGroupedFieldSetResult = @@ -690,47 +692,27 @@ export function isNonReconcilableDeferredGroupedFieldSetResult( return deferredGroupedFieldSetResult.result === null; } -/** @internal */ -export class DeferredGroupedFieldSetRecord { +export interface DeferredGroupedFieldSetRecord { deferredFragmentRecords: ReadonlyArray; result: PromiseOrValue; +} - constructor(opts: { - deferredFragmentRecords: ReadonlyArray; - executor: () => PromiseOrValue; - }) { - const { deferredFragmentRecords, executor } = opts; - this.deferredFragmentRecords = deferredFragmentRecords; - - for (const deferredFragmentRecord of deferredFragmentRecords) { - deferredFragmentRecord.deferredGroupedFieldSetRecords.push(this); - } - - // If any of the deferred fragments for this deferred grouped field set - // have an id, then they have been released to the client as pending - // and it is safe to execute the deferred grouped field set synchronously. - - // This can occur, for example, when deferred fragments have overlapping - // fields, and a new deferred grouped field set has been created for the - // non-overlapping fields. - this.result = deferredFragmentRecords.some( - (deferredFragmentRecord) => deferredFragmentRecord.id !== undefined, - ) - ? executor() - : Promise.resolve().then(executor); - } +interface SubsequentResultRecord { + path: Path | undefined; + label: string | undefined; + id?: string | undefined; } /** @internal */ -export class DeferredFragmentRecord { +export class DeferredFragmentRecord implements SubsequentResultRecord { path: Path | undefined; label: string | undefined; - deferredGroupedFieldSetRecords: Array; + id?: string | undefined; + parent: DeferredFragmentRecord | undefined; + expectedReconcilableResults: number; results: Array; reconcilableResults: Array; - parent: DeferredFragmentRecord | undefined; children: Set; - id?: string | undefined; constructor(opts: { path: Path | undefined; @@ -739,30 +721,16 @@ export class DeferredFragmentRecord { }) { this.path = opts.path; this.label = opts.label; - this.deferredGroupedFieldSetRecords = []; + this.parent = opts.parent; + this.expectedReconcilableResults = 0; this.results = []; this.reconcilableResults = []; - this.parent = opts.parent; this.children = new Set(); } } -/** @internal */ -export class StreamRecord { - label: string | undefined; - path: Path; - earlyReturn: (() => Promise) | undefined; - id?: string | undefined; - constructor(opts: { - label: string | undefined; - path: Path; - earlyReturn?: (() => Promise) | undefined; - }) { - const { label, path, earlyReturn } = opts; - this.label = label; - this.path = path; - this.earlyReturn = earlyReturn; - } +export interface StreamRecord extends SubsequentResultRecord { + earlyReturn?: (() => Promise) | undefined; } interface NonReconcilableStreamItemsResult { @@ -795,22 +763,9 @@ export function isNonTerminatingStreamItemsResult( return streamItemsResult.result != null; } -/** @internal */ -export class StreamItemsRecord { +export interface StreamItemsRecord { streamRecord: StreamRecord; - nextStreamItems: StreamItemsRecord | undefined; - result: PromiseOrValue; - - constructor(opts: { - streamRecord: StreamRecord; - executor: () => PromiseOrValue; - }) { - const { streamRecord, executor } = opts; - this.streamRecord = streamRecord; - - this.result = executor(); - } } export type IncrementalDataRecord = @@ -820,5 +775,3 @@ export type IncrementalDataRecord = export type IncrementalDataRecordResult = | DeferredGroupedFieldSetResult | StreamItemsResult; - -type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index e82dea5c7b..cda1551602 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -56,19 +56,19 @@ import { buildFieldPlan } from './buildFieldPlan.js'; import type { DeferUsage, FieldDetails } from './collectFields.js'; import { collectFields, collectSubfields } from './collectFields.js'; import type { + DeferredGroupedFieldSetRecord, DeferredGroupedFieldSetResult, ExecutionResult, ExperimentalIncrementalExecutionResults, IncrementalDataRecord, + StreamItemsRecord, StreamItemsResult, + StreamRecord, } from './IncrementalPublisher.js'; import { buildIncrementalResponse, DeferredFragmentRecord, - DeferredGroupedFieldSetRecord, isNonTerminatingStreamItemsResult, - StreamItemsRecord, - StreamRecord, } from './IncrementalPublisher.js'; import { mapAsyncIterable } from './mapAsyncIterable.js'; import { @@ -1030,11 +1030,11 @@ async function completeAsyncIteratorValue( // eslint-disable-next-line no-constant-condition while (true) { if (streamUsage && index >= streamUsage.initialCount) { - const streamRecord = new StreamRecord({ + const streamRecord: StreamRecord = { label: streamUsage.label, path, earlyReturn: asyncIterator.return?.bind(asyncIterator), - }); + }; exeContext.cancellableStreams.add(streamRecord); @@ -1177,10 +1177,10 @@ function completeListValue( const item = iteration.value; if (streamUsage && index >= streamUsage.initialCount) { - const streamRecord = new StreamRecord({ + const streamRecord: StreamRecord = { label: streamUsage.label, path, - }); + }; const firstStreamItems = firstSyncStreamItems( streamRecord, @@ -1908,20 +1908,24 @@ function executeDeferredGroupedFieldSets( deferMap, ); - const deferredGroupedFieldSetRecord = new DeferredGroupedFieldSetRecord({ + const executor = () => + executeDeferredGroupedFieldSet( + deferredFragmentRecords, + exeContext, + parentType, + sourceValue, + path, + groupedFieldSet, + [], + deferMap, + ); + + const deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord = { deferredFragmentRecords, - executor: () => - executeDeferredGroupedFieldSet( - deferredFragmentRecords, - exeContext, - parentType, - sourceValue, - path, - groupedFieldSet, - [], - deferMap, - ), - }); + result: shouldDefer(deferredFragmentRecords) + ? Promise.resolve().then(executor) + : executor(), + }; newDeferredGroupedFieldSetRecords.push(deferredGroupedFieldSetRecord); } @@ -1929,6 +1933,21 @@ function executeDeferredGroupedFieldSets( return newDeferredGroupedFieldSetRecords; } +function shouldDefer( + deferredFragmentRecords: ReadonlyArray, +): boolean { + // If any of the deferred fragments for this deferred grouped field set + // have an id, then they have been released to the client as pending + // and it is safe to execute the deferred grouped field set synchronously. + + // This can occur, for example, when deferred fragments have overlapping + // fields, and a new deferred grouped field set has been created for the + // non-overlapping fields. + return deferredFragmentRecords.every( + (deferredFragmentRecord) => deferredFragmentRecord.id === undefined, + ); +} + function executeDeferredGroupedFieldSet( deferredFragmentRecords: ReadonlyArray, exeContext: ExecutionContext, @@ -2022,48 +2041,38 @@ function firstSyncStreamItems( const path = streamRecord.path; const initialPath = addPath(path, initialIndex, undefined); - const firstStreamItems = new StreamItemsRecord({ + const firstStreamItems: StreamItemsRecord = { streamRecord, - executor: () => - Promise.resolve().then(() => { - const results: Array> = [ - executor(initialPath, initialItem), - ]; - let currentIndex = initialIndex; - let iteration = iterator.next(); - while (!iteration.done) { - const item = iteration.value; - currentIndex++; - const currentPath = addPath(path, currentIndex, undefined); - results.push(executor(currentPath, item)); - iteration = iterator.next(); - } + result: Promise.resolve().then(() => { + const results: Array> = [ + executor(initialPath, initialItem), + ]; + let currentIndex = initialIndex; + let iteration = iterator.next(); + while (!iteration.done) { + const item = iteration.value; + currentIndex++; + const currentPath = addPath(path, currentIndex, undefined); + results.push(executor(currentPath, item)); + iteration = iterator.next(); + } - let i = results.length - 1; - let currentResult = prependNextStreamItems( - results[i], - new StreamItemsRecord({ - streamRecord, - executor: () => ({ streamRecord }), - }), - ); + currentIndex = results.length - 1; + let currentResult = prependNextStreamItems(results[currentIndex], { + streamRecord, + result: { streamRecord }, + }); - while (i > 0) { - const index = i--; - currentResult = prependNextStreamItems( - results[index - 1], - new StreamItemsRecord({ - streamRecord, - // safe as this function is executed immediately - // eslint-disable-next-line @typescript-eslint/no-loop-func - executor: () => currentResult, - }), - ); - } + while (currentIndex-- > 0) { + currentResult = prependNextStreamItems(results[currentIndex], { + streamRecord, + result: currentResult, + }); + } - return currentResult; - }), - }); + return currentResult; + }), + }; return firstStreamItems; } @@ -2106,20 +2115,19 @@ function firstAsyncStreamItems( item: PromiseOrValue, ) => PromiseOrValue, ): StreamItemsRecord { - const firstStreamItems: StreamItemsRecord = new StreamItemsRecord({ + const firstStreamItems: StreamItemsRecord = { streamRecord, - executor: () => - Promise.resolve().then(() => - getNextAsyncStreamItemsResult( - streamRecord, - path, - initialIndex, - nodes, - asyncIterator, - executor, - ), + result: Promise.resolve().then(() => + getNextAsyncStreamItemsResult( + streamRecord, + path, + initialIndex, + nodes, + asyncIterator, + executor, ), - }); + ), + }; return firstStreamItems; } @@ -2176,20 +2184,19 @@ function nextAsyncStreamItems( item: PromiseOrValue, ) => PromiseOrValue, ): StreamItemsRecord { - const nextStreamItems: StreamItemsRecord = new StreamItemsRecord({ + const nextStreamItems: StreamItemsRecord = { streamRecord, - executor: () => - Promise.resolve().then(() => - getNextAsyncStreamItemsResult( - streamRecord, - path, - initialIndex + 1, - nodes, - asyncIterator, - executor, - ), + result: Promise.resolve().then(() => + getNextAsyncStreamItemsResult( + streamRecord, + path, + initialIndex + 1, + nodes, + asyncIterator, + executor, ), - }); + ), + }; return nextStreamItems; } From 26f089d3f85c9da3cd7b751196d9cf4986a86b3c Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:05:49 +0300 Subject: [PATCH 10/27] change logic to reduce extra ticks --- src/execution/execute.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index cda1551602..22b67b3584 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -296,6 +296,7 @@ function executeOperation( rootType, rootValue, undefined, + undefined, newGroupedFieldSets, newDeferMap, ); @@ -1589,6 +1590,7 @@ function collectAndExecuteSubfields( returnType, result, path, + fieldGroup.deferUsages, newGroupedFieldSets, newDeferMap, ); @@ -1896,6 +1898,7 @@ function executeDeferredGroupedFieldSets( parentType: GraphQLObjectType, sourceValue: unknown, path: Path | undefined, + parentDeferUsages: DeferUsageSet | undefined, newGroupedFieldSets: Map, deferMap: ReadonlyMap, ): ReadonlyArray { @@ -1922,7 +1925,7 @@ function executeDeferredGroupedFieldSets( const deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord = { deferredFragmentRecords, - result: shouldDefer(deferredFragmentRecords) + result: shouldDefer(parentDeferUsages, deferUsageSet) ? Promise.resolve().then(executor) : executor(), }; @@ -1934,17 +1937,18 @@ function executeDeferredGroupedFieldSets( } function shouldDefer( - deferredFragmentRecords: ReadonlyArray, + parentDeferUsages: undefined | DeferUsageSet, + deferUsages: DeferUsageSet, ): boolean { - // If any of the deferred fragments for this deferred grouped field set - // have an id, then they have been released to the client as pending - // and it is safe to execute the deferred grouped field set synchronously. - - // This can occur, for example, when deferred fragments have overlapping - // fields, and a new deferred grouped field set has been created for the - // non-overlapping fields. - return deferredFragmentRecords.every( - (deferredFragmentRecord) => deferredFragmentRecord.id === undefined, + // If we have a new child defer usage, defer. + // Otherwise, this defer usage was already deferred when it was initially + // encountered, and is now in the midst of executing early, so the new + // deferred grouped fields set can be executed immediately. + return ( + parentDeferUsages === undefined || + !Array.from(deferUsages).every((deferUsage) => + parentDeferUsages.has(deferUsage), + ) ); } From 0202ba6df01242ff558a6f0edf307b39b23adc8b Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:16:39 +0300 Subject: [PATCH 11/27] use errors as discriminator rather than null --- src/execution/IncrementalPublisher.ts | 20 +++++++++++--------- src/execution/execute.ts | 6 ------ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index f0c1feac39..d0ef4f7377 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -584,11 +584,7 @@ class IncrementalPublisher { if (id === undefined) { return; } - if (streamItemsResult.result === undefined) { - this._completed.push({ id }); - this._pending.delete(streamRecord); - this._context.cancellableStreams.delete(streamRecord); - } else if (streamItemsResult.result === null) { + if (streamItemsResult.errors !== undefined) { this._completed.push({ id, errors: streamItemsResult.errors, @@ -599,6 +595,10 @@ class IncrementalPublisher { /* c8 ignore next 1 */ // ignore error }); + } else if (streamItemsResult.result === undefined) { + this._completed.push({ id }); + this._pending.delete(streamRecord); + this._context.cancellableStreams.delete(streamRecord); } else { const incrementalEntry: IncrementalStreamResult = { id, @@ -677,19 +677,20 @@ interface ReconcilableDeferredGroupedFieldSetResult { result: BareDeferredGroupedFieldSetResult; incrementalDataRecords: ReadonlyArray; sent?: true | undefined; + errors?: never; } interface NonReconcilableDeferredGroupedFieldSetResult { - result: null; errors: ReadonlyArray; deferredFragmentRecords: ReadonlyArray; path: Array; + result?: never; } export function isNonReconcilableDeferredGroupedFieldSetResult( deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, ): deferredGroupedFieldSetResult is NonReconcilableDeferredGroupedFieldSetResult { - return deferredGroupedFieldSetResult.result === null; + return deferredGroupedFieldSetResult.errors !== undefined; } export interface DeferredGroupedFieldSetRecord { @@ -735,14 +736,15 @@ export interface StreamRecord extends SubsequentResultRecord { interface NonReconcilableStreamItemsResult { streamRecord: StreamRecord; - result: null; errors: ReadonlyArray; + result?: never; } interface NonTerminatingStreamItemsResult { streamRecord: StreamRecord; result: BareStreamItemsResult; incrementalDataRecords: ReadonlyArray; + errors?: never; } interface TerminatingStreamItemsResult { @@ -760,7 +762,7 @@ export type StreamItemsResult = export function isNonTerminatingStreamItemsResult( streamItemsResult: StreamItemsResult, ): streamItemsResult is NonTerminatingStreamItemsResult { - return streamItemsResult.result != null; + return streamItemsResult.result !== undefined; } export interface StreamItemsRecord { diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 22b67b3584..eb0a56a5ec 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1977,7 +1977,6 @@ function executeDeferredGroupedFieldSet( return { deferredFragmentRecords, path: pathToArray(path), - result: null, errors: withError(errors, error), }; } @@ -1994,7 +1993,6 @@ function executeDeferredGroupedFieldSet( (error) => ({ deferredFragmentRecords, path: pathToArray(path), - result: null, errors: withError(errors, error), }), ); @@ -2152,7 +2150,6 @@ async function getNextAsyncStreamItemsResult( } catch (error) { return { streamRecord, - result: null, errors: [locatedError(error, nodes, pathToArray(path))], }; } @@ -2229,7 +2226,6 @@ function completeStreamItems( buildStreamItemsResult(errors, streamRecord, resolvedItem), (error) => ({ streamRecord, - result: null, errors: withError(errors, error), }), ); @@ -2255,7 +2251,6 @@ function completeStreamItems( } catch (error) { return { streamRecord, - result: null, errors: withError(errors, error), }; } @@ -2271,7 +2266,6 @@ function completeStreamItems( buildStreamItemsResult(errors, streamRecord, resolvedItem), (error) => ({ streamRecord, - result: null, errors: withError(errors, error), }), ); From facaf19e0b68bf2db31861601522a7128735993f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:19:05 +0300 Subject: [PATCH 12/27] remove unnecessary exports --- src/execution/IncrementalPublisher.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index d0ef4f7377..2951264707 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -665,7 +665,7 @@ export type DeferredGroupedFieldSetResult = | ReconcilableDeferredGroupedFieldSetResult | NonReconcilableDeferredGroupedFieldSetResult; -export function isDeferredGroupedFieldSetResult( +function isDeferredGroupedFieldSetResult( subsequentResult: DeferredGroupedFieldSetResult | StreamItemsResult, ): subsequentResult is DeferredGroupedFieldSetResult { return 'deferredFragmentRecords' in subsequentResult; @@ -687,7 +687,7 @@ interface NonReconcilableDeferredGroupedFieldSetResult { result?: never; } -export function isNonReconcilableDeferredGroupedFieldSetResult( +function isNonReconcilableDeferredGroupedFieldSetResult( deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, ): deferredGroupedFieldSetResult is NonReconcilableDeferredGroupedFieldSetResult { return deferredGroupedFieldSetResult.errors !== undefined; From 26974f49a8264c4f08984bb6823d36d57228b6d5 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:32:58 +0300 Subject: [PATCH 13/27] remove unnecessary guard --- src/execution/IncrementalPublisher.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 2951264707..6f059ede9c 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -563,9 +563,7 @@ class IncrementalPublisher { for (const child of deferredFragmentRecord.children) { this._newPending.add(child); for (const childResult of child.results) { - if (!isPromise(childResult)) { - this._completedResultQueue.push(childResult); - } + this._completedResultQueue.push(childResult); } } } From 47b988cd7d6cdaac4d53eaebdd24c54593603580 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:33:29 +0300 Subject: [PATCH 14/27] remove unnecessary loop --- src/execution/IncrementalPublisher.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 6f059ede9c..4713369428 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -562,9 +562,7 @@ class IncrementalPublisher { this._pending.delete(deferredFragmentRecord); for (const child of deferredFragmentRecord.children) { this._newPending.add(child); - for (const childResult of child.results) { - this._completedResultQueue.push(childResult); - } + this._completedResultQueue.push(...child.results); } } From 679aeb32a89f216cbd0b595d4fdd3c7d13ff774a Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:34:09 +0300 Subject: [PATCH 15/27] rename fragmentResult --- src/execution/IncrementalPublisher.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 4713369428..9943a688fa 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -532,25 +532,25 @@ class IncrementalPublisher { if (id === undefined) { continue; } - const fragmentResults = deferredFragmentRecord.reconcilableResults; + const reconcilableResults = deferredFragmentRecord.reconcilableResults; if ( deferredFragmentRecord.expectedReconcilableResults !== - fragmentResults.length + reconcilableResults.length ) { continue; } - for (const fragmentResult of fragmentResults) { - if (fragmentResult.sent) { + for (const reconcilableResult of reconcilableResults) { + if (reconcilableResult.sent) { continue; } - fragmentResult.sent = true; + reconcilableResult.sent = true; const { bestId, subPath } = this._getBestIdAndSubPath( id, deferredFragmentRecord, - fragmentResult, + reconcilableResult, ); const incrementalEntry: IncrementalDeferResult = { - ...fragmentResult.result, + ...reconcilableResult.result, id: bestId, }; if (subPath !== undefined) { From 1b83ea54dd1406b46fcb0af7d4d3792b834d07b7 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:46:19 +0300 Subject: [PATCH 16/27] add better typing for CancellableStreamRecord --- src/execution/IncrementalPublisher.ts | 38 ++++++++++++++++--------- src/execution/execute.ts | 41 ++++++++++++++++----------- 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 9943a688fa..9d32a349b8 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -184,7 +184,7 @@ export function buildIncrementalResponse( } interface IncrementalPublisherContext { - cancellableStreams: Set; + cancellableStreams: Set; } /** @@ -586,15 +586,19 @@ class IncrementalPublisher { errors: streamItemsResult.errors, }); this._pending.delete(streamRecord); - this._context.cancellableStreams.delete(streamRecord); - streamRecord.earlyReturn?.().catch(() => { - /* c8 ignore next 1 */ - // ignore error - }); + if (isCancellableStreamRecord(streamRecord)) { + this._context.cancellableStreams.delete(streamRecord); + streamRecord.earlyReturn().catch(() => { + /* c8 ignore next 1 */ + // ignore error + }); + } } else if (streamItemsResult.result === undefined) { this._completed.push({ id }); this._pending.delete(streamRecord); - this._context.cancellableStreams.delete(streamRecord); + if (isCancellableStreamRecord(streamRecord)) { + this._context.cancellableStreams.delete(streamRecord); + } } else { const incrementalEntry: IncrementalStreamResult = { id, @@ -694,7 +698,7 @@ export interface DeferredGroupedFieldSetRecord { result: PromiseOrValue; } -interface SubsequentResultRecord { +export interface SubsequentResultRecord { path: Path | undefined; label: string | undefined; id?: string | undefined; @@ -726,25 +730,31 @@ export class DeferredFragmentRecord implements SubsequentResultRecord { } } -export interface StreamRecord extends SubsequentResultRecord { - earlyReturn?: (() => Promise) | undefined; +export interface CancellableStreamRecord extends SubsequentResultRecord { + earlyReturn: () => Promise; +} + +function isCancellableStreamRecord( + subsequentResultRecord: SubsequentResultRecord, +): subsequentResultRecord is CancellableStreamRecord { + return 'earlyReturn' in subsequentResultRecord; } interface NonReconcilableStreamItemsResult { - streamRecord: StreamRecord; + streamRecord: SubsequentResultRecord; errors: ReadonlyArray; result?: never; } interface NonTerminatingStreamItemsResult { - streamRecord: StreamRecord; + streamRecord: SubsequentResultRecord; result: BareStreamItemsResult; incrementalDataRecords: ReadonlyArray; errors?: never; } interface TerminatingStreamItemsResult { - streamRecord: StreamRecord; + streamRecord: SubsequentResultRecord; result?: never; incrementalDataRecords?: never; errors?: never; @@ -762,7 +772,7 @@ export function isNonTerminatingStreamItemsResult( } export interface StreamItemsRecord { - streamRecord: StreamRecord; + streamRecord: SubsequentResultRecord; result: PromiseOrValue; } diff --git a/src/execution/execute.ts b/src/execution/execute.ts index eb0a56a5ec..373a5a15cd 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -56,6 +56,7 @@ import { buildFieldPlan } from './buildFieldPlan.js'; import type { DeferUsage, FieldDetails } from './collectFields.js'; import { collectFields, collectSubfields } from './collectFields.js'; import type { + CancellableStreamRecord, DeferredGroupedFieldSetRecord, DeferredGroupedFieldSetResult, ExecutionResult, @@ -63,7 +64,7 @@ import type { IncrementalDataRecord, StreamItemsRecord, StreamItemsResult, - StreamRecord, + SubsequentResultRecord, } from './IncrementalPublisher.js'; import { buildIncrementalResponse, @@ -143,7 +144,7 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; - cancellableStreams: Set; + cancellableStreams: Set; } export interface ExecutionArgs { @@ -1031,13 +1032,21 @@ async function completeAsyncIteratorValue( // eslint-disable-next-line no-constant-condition while (true) { if (streamUsage && index >= streamUsage.initialCount) { - const streamRecord: StreamRecord = { - label: streamUsage.label, - path, - earlyReturn: asyncIterator.return?.bind(asyncIterator), - }; - - exeContext.cancellableStreams.add(streamRecord); + const returnFn = asyncIterator.return; + let streamRecord; + if (returnFn === undefined) { + streamRecord = { + label: streamUsage.label, + path, + } as SubsequentResultRecord; + } else { + streamRecord = { + label: streamUsage.label, + path, + earlyReturn: returnFn.bind(asyncIterator), + } as CancellableStreamRecord; + exeContext.cancellableStreams.add(streamRecord); + } const firstStreamItems = firstAsyncStreamItems( streamRecord, @@ -1178,7 +1187,7 @@ function completeListValue( const item = iteration.value; if (streamUsage && index >= streamUsage.initialCount) { - const streamRecord: StreamRecord = { + const streamRecord: SubsequentResultRecord = { label: streamUsage.label, path, }; @@ -2031,7 +2040,7 @@ function getDeferredFragmentRecords( } function firstSyncStreamItems( - streamRecord: StreamRecord, + streamRecord: SubsequentResultRecord, initialItem: PromiseOrValue, initialIndex: number, iterator: Iterator, @@ -2107,7 +2116,7 @@ function prependNextStreamItems( } function firstAsyncStreamItems( - streamRecord: StreamRecord, + streamRecord: SubsequentResultRecord, path: Path, initialIndex: number, nodes: ReadonlyArray, @@ -2134,7 +2143,7 @@ function firstAsyncStreamItems( } async function getNextAsyncStreamItemsResult( - streamRecord: StreamRecord, + streamRecord: SubsequentResultRecord, path: Path, index: number, nodes: ReadonlyArray, @@ -2175,7 +2184,7 @@ async function getNextAsyncStreamItemsResult( } function nextAsyncStreamItems( - streamRecord: StreamRecord, + streamRecord: SubsequentResultRecord, path: Path, initialIndex: number, nodes: ReadonlyArray, @@ -2202,7 +2211,7 @@ function nextAsyncStreamItems( } function completeStreamItems( - streamRecord: StreamRecord, + streamRecord: SubsequentResultRecord, itemPath: Path, item: unknown, exeContext: ExecutionContext, @@ -2276,7 +2285,7 @@ function completeStreamItems( function buildStreamItemsResult( errors: ReadonlyArray, - streamRecord: StreamRecord, + streamRecord: SubsequentResultRecord, result: GraphQLResult, ): StreamItemsResult { return { From 7dcb8b3ee73aa9cc770787f62058f5269f90e08b Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:49:53 +0300 Subject: [PATCH 17/27] introduce small helper --- src/execution/execute.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 373a5a15cd..9df49672b3 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2093,17 +2093,16 @@ function prependNextStreamItems( ): PromiseOrValue { if (isPromise(result)) { return result.then((resolved) => - isNonTerminatingStreamItemsResult(resolved) - ? { - ...resolved, - incrementalDataRecords: [ - nextStreamItems, - ...resolved.incrementalDataRecords, - ], - } - : resolved, + prependNextResolvedStreamItems(resolved, nextStreamItems), ); } + return prependNextResolvedStreamItems(result, nextStreamItems); +} + +function prependNextResolvedStreamItems( + result: StreamItemsResult, + nextStreamItems: StreamItemsRecord, +): StreamItemsResult { return isNonTerminatingStreamItemsResult(result) ? { ...result, From 0b9842c54de42e6e33fa4627add06ff07d7a1ab5 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 15 Apr 2024 23:59:49 +0300 Subject: [PATCH 18/27] fix strict types --- src/execution/execute.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 9df49672b3..9b82ec8be1 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1033,7 +1033,7 @@ async function completeAsyncIteratorValue( while (true) { if (streamUsage && index >= streamUsage.initialCount) { const returnFn = asyncIterator.return; - let streamRecord; + let streamRecord: SubsequentResultRecord | CancellableStreamRecord; if (returnFn === undefined) { streamRecord = { label: streamUsage.label, @@ -1044,7 +1044,7 @@ async function completeAsyncIteratorValue( label: streamUsage.label, path, earlyReturn: returnFn.bind(asyncIterator), - } as CancellableStreamRecord; + }; exeContext.cancellableStreams.add(streamRecord); } From 7511630a298e3b3f96dab5b91fa4f2d09e48b077 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 16 Apr 2024 16:27:51 +0300 Subject: [PATCH 19/27] rename NonTerminatingStreamItemsResult to ReconcilableStreamItemsResult --- src/execution/IncrementalPublisher.ts | 32 +++++++++++++-------------- src/execution/execute.ts | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 9d32a349b8..b5f66b6322 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -740,19 +740,19 @@ function isCancellableStreamRecord( return 'earlyReturn' in subsequentResultRecord; } -interface NonReconcilableStreamItemsResult { - streamRecord: SubsequentResultRecord; - errors: ReadonlyArray; - result?: never; -} - -interface NonTerminatingStreamItemsResult { +interface ReconcilableStreamItemsResult { streamRecord: SubsequentResultRecord; result: BareStreamItemsResult; incrementalDataRecords: ReadonlyArray; errors?: never; } +export function isReconcilableStreamItemsResult( + streamItemsResult: StreamItemsResult, +): streamItemsResult is ReconcilableStreamItemsResult { + return streamItemsResult.result !== undefined; +} + interface TerminatingStreamItemsResult { streamRecord: SubsequentResultRecord; result?: never; @@ -760,17 +760,17 @@ interface TerminatingStreamItemsResult { errors?: never; } -export type StreamItemsResult = - | NonReconcilableStreamItemsResult - | NonTerminatingStreamItemsResult - | TerminatingStreamItemsResult; - -export function isNonTerminatingStreamItemsResult( - streamItemsResult: StreamItemsResult, -): streamItemsResult is NonTerminatingStreamItemsResult { - return streamItemsResult.result !== undefined; +interface NonReconcilableStreamItemsResult { + streamRecord: SubsequentResultRecord; + errors: ReadonlyArray; + result?: never; } +export type StreamItemsResult = + | ReconcilableStreamItemsResult + | TerminatingStreamItemsResult + | NonReconcilableStreamItemsResult; + export interface StreamItemsRecord { streamRecord: SubsequentResultRecord; result: PromiseOrValue; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 9b82ec8be1..ee79e8a45a 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -69,7 +69,7 @@ import type { import { buildIncrementalResponse, DeferredFragmentRecord, - isNonTerminatingStreamItemsResult, + isReconcilableStreamItemsResult, } from './IncrementalPublisher.js'; import { mapAsyncIterable } from './mapAsyncIterable.js'; import { @@ -2103,7 +2103,7 @@ function prependNextResolvedStreamItems( result: StreamItemsResult, nextStreamItems: StreamItemsRecord, ): StreamItemsResult { - return isNonTerminatingStreamItemsResult(result) + return isReconcilableStreamItemsResult(result) ? { ...result, incrementalDataRecords: [ From 765860e7efc3ee4f1aa0666ef4122eb06b9a5d11 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 16 Apr 2024 16:25:40 +0300 Subject: [PATCH 20/27] break synchronous loop early on non-reconcilable result --- src/execution/__tests__/stream-test.ts | 2 +- src/execution/execute.ts | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index bf0383ce48..522b82f3d4 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -796,7 +796,7 @@ describe('Execute: stream directive', () => { } `); const result = await complete(document, { - nonNullFriendList: () => [friends[0], null], + nonNullFriendList: () => [friends[0], null, friends[1]], }); expectJSON(result).toDeepEqual([ diff --git a/src/execution/execute.ts b/src/execution/execute.ts index ee79e8a45a..a4b83592ef 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2055,24 +2055,31 @@ function firstSyncStreamItems( const firstStreamItems: StreamItemsRecord = { streamRecord, result: Promise.resolve().then(() => { - const results: Array> = [ - executor(initialPath, initialItem), - ]; + let result = executor(initialPath, initialItem); + const results = [result]; let currentIndex = initialIndex; let iteration = iterator.next(); + let erroredSynchronously = false; while (!iteration.done) { + if (!isPromise(result) && !isReconcilableStreamItemsResult(result)) { + erroredSynchronously = true; + break; + } const item = iteration.value; currentIndex++; const currentPath = addPath(path, currentIndex, undefined); - results.push(executor(currentPath, item)); + result = executor(currentPath, item); + results.push(result); iteration = iterator.next(); } currentIndex = results.length - 1; - let currentResult = prependNextStreamItems(results[currentIndex], { - streamRecord, - result: { streamRecord }, - }); + let currentResult = erroredSynchronously + ? results[currentIndex] + : prependNextStreamItems(results[currentIndex], { + streamRecord, + result: { streamRecord }, + }); while (currentIndex-- > 0) { currentResult = prependNextStreamItems(results[currentIndex], { From 74d092ef127f021a915a4d1cb59f35917e93c9c0 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 16 Apr 2024 21:52:40 +0300 Subject: [PATCH 21/27] just push! --- src/execution/execute.ts | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index a4b83592ef..a894acd320 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -165,7 +165,7 @@ export interface StreamUsage { fieldGroup: FieldGroup; } -type GraphQLResult = [T, ReadonlyArray]; +type GraphQLResult = [T, Array]; const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = 'The provided schema unexpectedly contains experimental directives (@defer or @stream). These directives may only be utilized if experimental execution features are explicitly enabled.'; @@ -328,27 +328,15 @@ function withNewDeferredGroupedFieldSets( ): PromiseOrValue>> { if (isPromise(result)) { return result.then((resolved) => { - appendNewIncrementalDataRecords( - resolved, - newDeferredGroupedFieldSetRecords, - ); + resolved[1].push(...newDeferredGroupedFieldSetRecords); return resolved; }); } - appendNewIncrementalDataRecords(result, newDeferredGroupedFieldSetRecords); + result[1].push(...newDeferredGroupedFieldSetRecords); return result; } -function appendNewIncrementalDataRecords( - acc: GraphQLResult, - newRecords: ReadonlyArray, -): void { - if (newRecords.length > 0) { - acc[1] = acc[1].length === 0 ? newRecords : [...acc[1], ...newRecords]; - } -} - function withError( errors: Array, error: GraphQLError, @@ -566,12 +554,12 @@ function executeFieldsSerially( if (isPromise(result)) { return result.then((resolved) => { acc[0][responseName] = resolved[0]; - appendNewIncrementalDataRecords(acc, resolved[1]); + acc[1].push(...resolved[1]); return acc; }); } acc[0][responseName] = result[0]; - appendNewIncrementalDataRecords(acc, result[1]); + acc[1].push(...result[1]); return acc; }, [Object.create(null), []] as GraphQLResult>, @@ -611,13 +599,13 @@ function executeFields( if (result !== undefined) { if (isPromise(result)) { results[responseName] = result.then((resolved) => { - appendNewIncrementalDataRecords(acc, resolved[1]); + acc[1].push(...resolved[1]); return resolved[0]; }); containsPromise = true; } else { results[responseName] = result[0]; - appendNewIncrementalDataRecords(acc, result[1]); + acc[1].push(...result[1]); } } } @@ -1067,7 +1055,7 @@ async function completeAsyncIteratorValue( ), ); - appendNewIncrementalDataRecords(acc, [firstStreamItems]); + acc[1].push(firstStreamItems); break; } @@ -1101,7 +1089,7 @@ async function completeAsyncIteratorValue( errors, deferMap, ).then((resolved) => { - appendNewIncrementalDataRecords(acc, resolved[1]); + acc[1].push(...resolved[1]); return resolved[0]; }), ); @@ -1210,7 +1198,7 @@ function completeListValue( ), ); - appendNewIncrementalDataRecords(acc, [firstStreamItems]); + acc[1].push(firstStreamItems); break; } @@ -1230,7 +1218,7 @@ function completeListValue( errors, deferMap, ).then((resolved) => { - appendNewIncrementalDataRecords(acc, resolved[1]); + acc[1].push(...resolved[1]); return resolved[0]; }), ); @@ -1296,7 +1284,7 @@ function completeListItemValue( completedResults.push( completedItem.then( (resolved) => { - appendNewIncrementalDataRecords(parent, resolved[1]); + parent[1].push(...resolved[1]); return resolved[0]; }, (rawError) => { @@ -1309,7 +1297,7 @@ function completeListItemValue( } completedResults.push(completedItem[0]); - appendNewIncrementalDataRecords(parent, completedItem[1]); + parent[1].push(...completedItem[1]); } catch (rawError) { handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); completedResults.push(null); From 24136afece0f54058b8a9b615be7081c604e1868 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 16 Apr 2024 21:56:03 +0300 Subject: [PATCH 22/27] rename GraphQLResult to GraphQLWrappedResult --- src/execution/execute.ts | 124 +++++++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 52 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index a894acd320..859d879321 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -165,7 +165,7 @@ export interface StreamUsage { fieldGroup: FieldGroup; } -type GraphQLResult = [T, Array]; +type GraphQLWrappedResult = [T, Array]; const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = 'The provided schema unexpectedly contains experimental directives (@defer or @stream). These directives may only be utilized if experimental execution features are explicitly enabled.'; @@ -282,7 +282,7 @@ function executeOperation( const newDeferMap = addNewDeferredFragments(newDeferUsages, new Map()); - let acc = executeRootGroupedFieldSet( + let graphqlWrappedResult = executeRootGroupedFieldSet( exeContext, operation.operation, rootType, @@ -302,12 +302,12 @@ function executeOperation( newDeferMap, ); - acc = withNewDeferredGroupedFieldSets( - acc, + graphqlWrappedResult = withNewDeferredGroupedFieldSets( + graphqlWrappedResult, newDeferredGroupedFieldSetRecords, ); - if (isPromise(acc)) { - return acc.then( + if (isPromise(graphqlWrappedResult)) { + return graphqlWrappedResult.then( (resolved) => buildDataResponse(exeContext, resolved[0], errors, resolved[1]), (error) => ({ @@ -316,16 +316,21 @@ function executeOperation( }), ); } - return buildDataResponse(exeContext, acc[0], errors, acc[1]); + return buildDataResponse( + exeContext, + graphqlWrappedResult[0], + errors, + graphqlWrappedResult[1], + ); } catch (error) { return { data: null, errors: withError(errors, error) }; } } function withNewDeferredGroupedFieldSets( - result: PromiseOrValue>>, + result: PromiseOrValue>>, newDeferredGroupedFieldSetRecords: ReadonlyArray, -): PromiseOrValue>> { +): PromiseOrValue>> { if (isPromise(result)) { return result.then((resolved) => { resolved[1].push(...newDeferredGroupedFieldSetRecords); @@ -485,7 +490,7 @@ function executeRootGroupedFieldSet( groupedFieldSet: GroupedFieldSet, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { switch (operation) { case OperationTypeNode.QUERY: return executeFields( @@ -534,10 +539,10 @@ function executeFieldsSerially( groupedFieldSet: GroupedFieldSet, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { return promiseReduce( groupedFieldSet, - (acc, [responseName, fieldGroup]) => { + (graphqlWrappedResult, [responseName, fieldGroup]) => { const fieldPath = addPath(path, responseName, parentType.name); const result = executeField( exeContext, @@ -549,20 +554,20 @@ function executeFieldsSerially( deferMap, ); if (result === undefined) { - return acc; + return graphqlWrappedResult; } if (isPromise(result)) { return result.then((resolved) => { - acc[0][responseName] = resolved[0]; - acc[1].push(...resolved[1]); - return acc; + graphqlWrappedResult[0][responseName] = resolved[0]; + graphqlWrappedResult[1].push(...resolved[1]); + return graphqlWrappedResult; }); } - acc[0][responseName] = result[0]; - acc[1].push(...result[1]); - return acc; + graphqlWrappedResult[0][responseName] = result[0]; + graphqlWrappedResult[1].push(...result[1]); + return graphqlWrappedResult; }, - [Object.create(null), []] as GraphQLResult>, + [Object.create(null), []] as GraphQLWrappedResult>, ); } @@ -578,9 +583,12 @@ function executeFields( groupedFieldSet: GroupedFieldSet, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { const results = Object.create(null); - const acc: GraphQLResult> = [results, []]; + const graphqlWrappedResult: GraphQLWrappedResult> = [ + results, + [], + ]; let containsPromise = false; try { @@ -599,13 +607,13 @@ function executeFields( if (result !== undefined) { if (isPromise(result)) { results[responseName] = result.then((resolved) => { - acc[1].push(...resolved[1]); + graphqlWrappedResult[1].push(...resolved[1]); return resolved[0]; }); containsPromise = true; } else { results[responseName] = result[0]; - acc[1].push(...result[1]); + graphqlWrappedResult[1].push(...result[1]); } } } @@ -623,13 +631,16 @@ function executeFields( // If there are no promises, we can just return the object and any incrementalDataRecords if (!containsPromise) { - return acc; + return graphqlWrappedResult; } // Otherwise, results is a map from field name to the result of resolving that // field, which is possibly a promise. Return a promise that will return this // same map, but with any promises replaced with the values they resolved to. - return promiseForObject(results, (resolved) => [resolved, acc[1]]); + return promiseForObject(results, (resolved) => [ + resolved, + graphqlWrappedResult[1], + ]); } function toNodes(fieldGroup: FieldGroup): ReadonlyArray { @@ -650,7 +661,7 @@ function executeField( path: Path, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> | undefined { +): PromiseOrValue> | undefined { const fieldName = fieldGroup.fields[0].node.name.value; const fieldDef = exeContext.schema.getField(parentType, fieldName); if (!fieldDef) { @@ -802,7 +813,7 @@ function completeValue( result: unknown, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { +): PromiseOrValue> { // If result is an Error, throw a located error. if (result instanceof Error) { throw result; @@ -821,7 +832,7 @@ function completeValue( errors, deferMap, ); - if ((completed as GraphQLResult)[0] === null) { + if ((completed as GraphQLWrappedResult)[0] === null) { throw new Error( `Cannot return null for non-nullable field ${info.parentType.name}.${info.fieldName}.`, ); @@ -899,7 +910,7 @@ async function completePromisedValue( result: Promise, errors: Array, deferMap: ReadonlyMap | undefined, -): Promise> { +): Promise> { try { const resolved = await result; let completed = completeValue( @@ -1011,10 +1022,13 @@ async function completeAsyncIteratorValue( asyncIterator: AsyncIterator, errors: Array, deferMap: ReadonlyMap | undefined, -): Promise>> { +): Promise>> { let containsPromise = false; const completedResults: Array = []; - const acc: GraphQLResult> = [completedResults, []]; + const graphqlWrappedResult: GraphQLWrappedResult> = [ + completedResults, + [], + ]; let index = 0; const streamUsage = getStreamUsage(exeContext, fieldGroup, path); // eslint-disable-next-line no-constant-condition @@ -1055,7 +1069,7 @@ async function completeAsyncIteratorValue( ), ); - acc[1].push(firstStreamItems); + graphqlWrappedResult[1].push(firstStreamItems); break; } @@ -1089,7 +1103,7 @@ async function completeAsyncIteratorValue( errors, deferMap, ).then((resolved) => { - acc[1].push(...resolved[1]); + graphqlWrappedResult[1].push(...resolved[1]); return resolved[0]; }), ); @@ -1099,7 +1113,7 @@ async function completeAsyncIteratorValue( completeListItemValue( item, completedResults, - acc, + graphqlWrappedResult, exeContext, itemType, fieldGroup, @@ -1120,9 +1134,9 @@ async function completeAsyncIteratorValue( return containsPromise ? /* c8 ignore start */ Promise.all(completedResults).then((resolved) => [ resolved, - acc[1], + graphqlWrappedResult[1], ]) - : /* c8 ignore stop */ acc; + : /* c8 ignore stop */ graphqlWrappedResult; } /** @@ -1138,7 +1152,7 @@ function completeListValue( result: unknown, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { const itemType = returnType.ofType; if (isAsyncIterable(result)) { @@ -1166,7 +1180,10 @@ function completeListValue( // where the list contains no Promises by avoiding creating another Promise. let containsPromise = false; const completedResults: Array = []; - const acc: GraphQLResult> = [completedResults, []]; + const graphqlWrappedResult: GraphQLWrappedResult> = [ + completedResults, + [], + ]; let index = 0; const streamUsage = getStreamUsage(exeContext, fieldGroup, path); const iterator = result[Symbol.iterator](); @@ -1198,7 +1215,7 @@ function completeListValue( ), ); - acc[1].push(firstStreamItems); + graphqlWrappedResult[1].push(firstStreamItems); break; } @@ -1218,7 +1235,7 @@ function completeListValue( errors, deferMap, ).then((resolved) => { - acc[1].push(...resolved[1]); + graphqlWrappedResult[1].push(...resolved[1]); return resolved[0]; }), ); @@ -1227,7 +1244,7 @@ function completeListValue( completeListItemValue( item, completedResults, - acc, + graphqlWrappedResult, exeContext, itemType, fieldGroup, @@ -1245,8 +1262,11 @@ function completeListValue( } return containsPromise - ? Promise.all(completedResults).then((resolved) => [resolved, acc[1]]) - : acc; + ? Promise.all(completedResults).then((resolved) => [ + resolved, + graphqlWrappedResult[1], + ]) + : graphqlWrappedResult; } /** @@ -1257,7 +1277,7 @@ function completeListValue( function completeListItemValue( item: unknown, completedResults: Array, - parent: GraphQLResult>, + parent: GraphQLWrappedResult>, exeContext: ExecutionContext, itemType: GraphQLOutputType, fieldGroup: FieldGroup, @@ -1336,7 +1356,7 @@ function completeAbstractValue( result: unknown, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { const resolveTypeFn = returnType.resolveType ?? exeContext.typeResolver; const contextValue = exeContext.contextValue; const runtimeType = resolveTypeFn(result, contextValue, info, returnType); @@ -1449,7 +1469,7 @@ function completeObjectValue( result: unknown, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { // If there is an isTypeOf predicate function, call it with the // current result. If isTypeOf returns false, then raise an error rather // than continuing execution. @@ -1561,7 +1581,7 @@ function collectAndExecuteSubfields( result: unknown, errors: Array, deferMap: ReadonlyMap | undefined, -): PromiseOrValue>> { +): PromiseOrValue>> { // Collect sub-fields to execute to complete this value. const { groupedFieldSet, newGroupedFieldSets, newDeferUsages } = buildSubFieldPlan(exeContext, returnType, fieldGroup); @@ -2007,7 +2027,7 @@ function buildDeferredGroupedFieldSetResult( errors: ReadonlyArray, deferredFragmentRecords: ReadonlyArray, path: Path | undefined, - result: GraphQLResult>, + result: GraphQLWrappedResult>, ): DeferredGroupedFieldSetResult { return { deferredFragmentRecords, @@ -2234,7 +2254,7 @@ function completeStreamItems( ); } - let result: PromiseOrValue>; + let result: PromiseOrValue>; try { try { result = completeValue( @@ -2262,7 +2282,7 @@ function completeStreamItems( return result .then(undefined, (rawError) => { handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); - return [null, []] as GraphQLResult; + return [null, []] as GraphQLWrappedResult; }) .then( (resolvedItem) => @@ -2280,7 +2300,7 @@ function completeStreamItems( function buildStreamItemsResult( errors: ReadonlyArray, streamRecord: SubsequentResultRecord, - result: GraphQLResult, + result: GraphQLWrappedResult, ): StreamItemsResult { return { streamRecord, From eb435a08b9c7e93a894b7f43ec54a1d17afe7101 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 17 Apr 2024 22:19:19 +0300 Subject: [PATCH 23/27] pass arguments instead of executor function --- src/execution/execute.ts | 119 ++++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 52 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 859d879321..00cb17ad5c 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1054,19 +1054,11 @@ async function completeAsyncIteratorValue( streamRecord, path, index, - toNodes(fieldGroup), asyncIterator, - (currentItemPath, currentItem) => - completeStreamItems( - streamRecord, - currentItemPath, - currentItem, - exeContext, - [], - streamUsage.fieldGroup, - info, - itemType, - ), + exeContext, + streamUsage.fieldGroup, + info, + itemType, ); graphqlWrappedResult[1].push(firstStreamItems); @@ -1202,17 +1194,10 @@ function completeListValue( item, index, iterator, - (currentItemPath, currentItem) => - completeStreamItems( - streamRecord, - currentItemPath, - currentItem, - exeContext, - [], - streamUsage.fieldGroup, - info, - itemType, - ), + exeContext, + streamUsage.fieldGroup, + info, + itemType, ); graphqlWrappedResult[1].push(firstStreamItems); @@ -2052,10 +2037,10 @@ function firstSyncStreamItems( initialItem: PromiseOrValue, initialIndex: number, iterator: Iterator, - executor: ( - itemPath: Path, - item: PromiseOrValue, - ) => PromiseOrValue, + exeContext: ExecutionContext, + fieldGroup: FieldGroup, + info: GraphQLResolveInfo, + itemType: GraphQLOutputType, ): StreamItemsRecord { const path = streamRecord.path; const initialPath = addPath(path, initialIndex, undefined); @@ -2063,7 +2048,16 @@ function firstSyncStreamItems( const firstStreamItems: StreamItemsRecord = { streamRecord, result: Promise.resolve().then(() => { - let result = executor(initialPath, initialItem); + let result = completeStreamItems( + streamRecord, + initialPath, + initialItem, + exeContext, + [], + fieldGroup, + info, + itemType, + ); const results = [result]; let currentIndex = initialIndex; let iteration = iterator.next(); @@ -2076,7 +2070,16 @@ function firstSyncStreamItems( const item = iteration.value; currentIndex++; const currentPath = addPath(path, currentIndex, undefined); - result = executor(currentPath, item); + result = completeStreamItems( + streamRecord, + currentPath, + item, + exeContext, + [], + fieldGroup, + info, + itemType, + ); results.push(result); iteration = iterator.next(); } @@ -2133,12 +2136,11 @@ function firstAsyncStreamItems( streamRecord: SubsequentResultRecord, path: Path, initialIndex: number, - nodes: ReadonlyArray, asyncIterator: AsyncIterator, - executor: ( - itemPath: Path, - item: PromiseOrValue, - ) => PromiseOrValue, + exeContext: ExecutionContext, + fieldGroup: FieldGroup, + info: GraphQLResolveInfo, + itemType: GraphQLOutputType, ): StreamItemsRecord { const firstStreamItems: StreamItemsRecord = { streamRecord, @@ -2147,9 +2149,11 @@ function firstAsyncStreamItems( streamRecord, path, initialIndex, - nodes, asyncIterator, - executor, + exeContext, + fieldGroup, + info, + itemType, ), ), }; @@ -2160,12 +2164,11 @@ async function getNextAsyncStreamItemsResult( streamRecord: SubsequentResultRecord, path: Path, index: number, - nodes: ReadonlyArray, asyncIterator: AsyncIterator, - executor: ( - itemPath: Path, - item: PromiseOrValue, - ) => PromiseOrValue, + exeContext: ExecutionContext, + fieldGroup: FieldGroup, + info: GraphQLResolveInfo, + itemType: GraphQLOutputType, ): Promise { let iteration; try { @@ -2173,7 +2176,7 @@ async function getNextAsyncStreamItemsResult( } catch (error) { return { streamRecord, - errors: [locatedError(error, nodes, pathToArray(path))], + errors: [locatedError(error, toNodes(fieldGroup), pathToArray(path))], }; } @@ -2183,15 +2186,26 @@ async function getNextAsyncStreamItemsResult( const itemPath = addPath(path, index, undefined); - const result = executor(itemPath, iteration.value); + const result = completeStreamItems( + streamRecord, + itemPath, + iteration.value, + exeContext, + [], + fieldGroup, + info, + itemType, + ); const nextStreamItems: StreamItemsRecord = nextAsyncStreamItems( streamRecord, path, index, - nodes, asyncIterator, - executor, + exeContext, + fieldGroup, + info, + itemType, ); return prependNextStreamItems(result, nextStreamItems); @@ -2201,12 +2215,11 @@ function nextAsyncStreamItems( streamRecord: SubsequentResultRecord, path: Path, initialIndex: number, - nodes: ReadonlyArray, asyncIterator: AsyncIterator, - executor: ( - itemPath: Path, - item: PromiseOrValue, - ) => PromiseOrValue, + exeContext: ExecutionContext, + fieldGroup: FieldGroup, + info: GraphQLResolveInfo, + itemType: GraphQLOutputType, ): StreamItemsRecord { const nextStreamItems: StreamItemsRecord = { streamRecord, @@ -2215,9 +2228,11 @@ function nextAsyncStreamItems( streamRecord, path, initialIndex + 1, - nodes, asyncIterator, - executor, + exeContext, + fieldGroup, + info, + itemType, ), ), }; From f9172556732e90e6cd3f8d9cff847fb1a71fb784 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 17 Apr 2024 22:21:34 +0300 Subject: [PATCH 24/27] remove extra tick --- src/execution/execute.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 00cb17ad5c..8a377d5dbe 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2223,17 +2223,15 @@ function nextAsyncStreamItems( ): StreamItemsRecord { const nextStreamItems: StreamItemsRecord = { streamRecord, - result: Promise.resolve().then(() => - getNextAsyncStreamItemsResult( - streamRecord, - path, - initialIndex + 1, - asyncIterator, - exeContext, - fieldGroup, - info, - itemType, - ), + result: getNextAsyncStreamItemsResult( + streamRecord, + path, + initialIndex + 1, + asyncIterator, + exeContext, + fieldGroup, + info, + itemType, ), }; return nextStreamItems; From 8ffb37f8c2324fb5ddc5e0d2e15c32b21f98b2ff Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 17 Apr 2024 22:27:36 +0300 Subject: [PATCH 25/27] remove another extra tick --- src/execution/execute.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 8a377d5dbe..737f45cd4d 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2144,17 +2144,15 @@ function firstAsyncStreamItems( ): StreamItemsRecord { const firstStreamItems: StreamItemsRecord = { streamRecord, - result: Promise.resolve().then(() => - getNextAsyncStreamItemsResult( - streamRecord, - path, - initialIndex, - asyncIterator, - exeContext, - fieldGroup, - info, - itemType, - ), + result: getNextAsyncStreamItemsResult( + streamRecord, + path, + initialIndex, + asyncIterator, + exeContext, + fieldGroup, + info, + itemType, ), }; return firstStreamItems; From c22cd01eb11c8cad23a6feea614f99d0af00f7ed Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 17 Apr 2024 22:29:49 +0300 Subject: [PATCH 26/27] remove extra function --- src/execution/execute.ts | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 737f45cd4d..c2c6c424b7 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2195,36 +2195,12 @@ async function getNextAsyncStreamItemsResult( itemType, ); - const nextStreamItems: StreamItemsRecord = nextAsyncStreamItems( - streamRecord, - path, - index, - asyncIterator, - exeContext, - fieldGroup, - info, - itemType, - ); - - return prependNextStreamItems(result, nextStreamItems); -} - -function nextAsyncStreamItems( - streamRecord: SubsequentResultRecord, - path: Path, - initialIndex: number, - asyncIterator: AsyncIterator, - exeContext: ExecutionContext, - fieldGroup: FieldGroup, - info: GraphQLResolveInfo, - itemType: GraphQLOutputType, -): StreamItemsRecord { const nextStreamItems: StreamItemsRecord = { streamRecord, result: getNextAsyncStreamItemsResult( streamRecord, path, - initialIndex + 1, + index, asyncIterator, exeContext, fieldGroup, @@ -2232,7 +2208,8 @@ function nextAsyncStreamItems( itemType, ), }; - return nextStreamItems; + + return prependNextStreamItems(result, nextStreamItems); } function completeStreamItems( From 6e44e3a1b85aed92dad078ebbe2cc951255a479d Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 17 Apr 2024 23:09:26 +0300 Subject: [PATCH 27/27] add comment regarding stream terminators --- src/execution/execute.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index c2c6c424b7..68037516e1 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2085,6 +2085,8 @@ function firstSyncStreamItems( } currentIndex = results.length - 1; + // If a non-reconcilable stream items result was encountered, then the stream terminates in error. + // Otherwise, add a stream terminator. let currentResult = erroredSynchronously ? results[currentIndex] : prependNextStreamItems(results[currentIndex], {