From 195c37236eb741ef8dfa7870fb9cd284a7d7cade Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 4 Apr 2024 17:04:59 +0300 Subject: [PATCH] use undefined for empty --- src/execution/IncrementalPublisher.ts | 31 ++-- src/execution/execute.ts | 200 +++++++++++++++++--------- 2 files changed, 152 insertions(+), 79 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index b5f66b6322e..0722da1ed10 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -1,3 +1,4 @@ +import { invariant } from '../jsutils/invariant.js'; import { isPromise } from '../jsutils/isPromise.js'; import type { ObjMap } from '../jsutils/ObjMap.js'; import type { Path } from '../jsutils/Path.js'; @@ -172,7 +173,7 @@ export interface FormattedCompletedResult { export function buildIncrementalResponse( context: IncrementalPublisherContext, result: ObjMap, - errors: ReadonlyArray, + errors: ReadonlyArray | undefined, incrementalDataRecords: ReadonlyArray, ): ExperimentalIncrementalExecutionResults { const incrementalPublisher = new IncrementalPublisher(context); @@ -184,7 +185,7 @@ export function buildIncrementalResponse( } interface IncrementalPublisherContext { - cancellableStreams: Set; + cancellableStreams: Set | undefined; } /** @@ -218,7 +219,7 @@ class IncrementalPublisher { buildResponse( data: ObjMap, - errors: ReadonlyArray, + errors: ReadonlyArray | undefined, incrementalDataRecords: ReadonlyArray, ): ExperimentalIncrementalExecutionResults { this._addIncrementalDataRecords(incrementalDataRecords); @@ -227,7 +228,7 @@ class IncrementalPublisher { const pending = this._pendingSourcesToResults(); const initialResult: InitialIncrementalExecutionResult = - errors.length === 0 + errors === undefined ? { data, pending, hasNext: true } : { errors, data, pending, hasNext: true }; @@ -444,8 +445,12 @@ class IncrementalPublisher { }; const returnStreamIterators = async (): Promise => { + const cancellableStreams = this._context.cancellableStreams; + if (cancellableStreams === undefined) { + return; + } const promises: Array> = []; - for (const streamRecord of this._context.cancellableStreams) { + for (const streamRecord of cancellableStreams) { if (streamRecord.earlyReturn !== undefined) { promises.push(streamRecord.earlyReturn()); } @@ -519,9 +524,11 @@ class IncrementalPublisher { ); } - this._addIncrementalDataRecords( - deferredGroupedFieldSetResult.incrementalDataRecords, - ); + const incrementalDataRecords = + deferredGroupedFieldSetResult.incrementalDataRecords; + if (incrementalDataRecords !== undefined) { + this._addIncrementalDataRecords(incrementalDataRecords); + } for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) { const id = deferredFragmentRecord.id; @@ -587,6 +594,7 @@ class IncrementalPublisher { }); this._pending.delete(streamRecord); if (isCancellableStreamRecord(streamRecord)) { + invariant(this._context.cancellableStreams !== undefined); this._context.cancellableStreams.delete(streamRecord); streamRecord.earlyReturn().catch(() => { /* c8 ignore next 1 */ @@ -597,6 +605,7 @@ class IncrementalPublisher { this._completed.push({ id }); this._pending.delete(streamRecord); if (isCancellableStreamRecord(streamRecord)) { + invariant(this._context.cancellableStreams !== undefined); this._context.cancellableStreams.delete(streamRecord); } } else { @@ -607,7 +616,7 @@ class IncrementalPublisher { this._incremental.push(incrementalEntry); - if (streamItemsResult.incrementalDataRecords.length > 0) { + if (streamItemsResult.incrementalDataRecords !== undefined) { this._addIncrementalDataRecords( streamItemsResult.incrementalDataRecords, ); @@ -675,7 +684,7 @@ interface ReconcilableDeferredGroupedFieldSetResult { deferredFragmentRecords: ReadonlyArray; path: Array; result: BareDeferredGroupedFieldSetResult; - incrementalDataRecords: ReadonlyArray; + incrementalDataRecords: ReadonlyArray | undefined; sent?: true | undefined; errors?: never; } @@ -743,7 +752,7 @@ function isCancellableStreamRecord( interface ReconcilableStreamItemsResult { streamRecord: SubsequentResultRecord; result: BareStreamItemsResult; - incrementalDataRecords: ReadonlyArray; + incrementalDataRecords: ReadonlyArray | undefined; errors?: never; } diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 2e6dce1c27b..88d5a0749da 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -142,12 +142,12 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; - errors: Array; - cancellableStreams: Set; + errors: Array | undefined; + cancellableStreams: Set | undefined; } interface IncrementalContext { - errors: Array; + errors: Array | undefined; deferUsageSet?: DeferUsageSet | undefined; } @@ -169,7 +169,7 @@ export interface StreamUsage { fieldGroup: FieldGroup; } -type GraphQLWrappedResult = [T, Array]; +type GraphQLWrappedResult = [T, Array | undefined]; 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.'; @@ -353,30 +353,44 @@ function withNewDeferredGroupedFieldSets( ): PromiseOrValue>> { if (isPromise(result)) { return result.then((resolved) => { - resolved[1].push(...newDeferredGroupedFieldSetRecords); + addIncrementalDataRecords(resolved, newDeferredGroupedFieldSetRecords); return resolved; }); } - result[1].push(...newDeferredGroupedFieldSetRecords); + addIncrementalDataRecords(result, newDeferredGroupedFieldSetRecords); return result; } +function addIncrementalDataRecords( + graphqlWrappedResult: GraphQLWrappedResult, + incrementalDataRecords: ReadonlyArray | undefined, +): void { + if (incrementalDataRecords === undefined) { + return; + } + if (graphqlWrappedResult[1] === undefined) { + graphqlWrappedResult[1] = [...incrementalDataRecords]; + } else { + graphqlWrappedResult[1].push(...incrementalDataRecords); + } +} + function withError( - errors: Array, + errors: Array | undefined, error: GraphQLError, ): ReadonlyArray { - return errors.length === 0 ? [error] : [...errors, error]; + return errors === undefined ? [error] : [...errors, error]; } function buildDataResponse( exeContext: ExecutionContext, data: ObjMap, - incrementalDataRecords: ReadonlyArray, + incrementalDataRecords: ReadonlyArray | undefined, ): ExecutionResult | ExperimentalIncrementalExecutionResults { const errors = exeContext.errors; - if (incrementalDataRecords.length === 0) { - return errors.length > 0 ? { errors, data } : { data }; + if (incrementalDataRecords === undefined) { + return errors !== undefined ? { errors, data } : { data }; } return buildIncrementalResponse( @@ -488,8 +502,8 @@ export function buildExecutionContext( fieldResolver: fieldResolver ?? defaultFieldResolver, typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, - errors: [], - cancellableStreams: new Set(), + errors: undefined, + cancellableStreams: undefined, }; } @@ -499,8 +513,8 @@ function buildPerEventExecutionContext( ): ExecutionContext { return { ...exeContext, - errors: [], rootValue: payload, + errors: undefined, }; } @@ -580,15 +594,15 @@ function executeFieldsSerially( if (isPromise(result)) { return result.then((resolved) => { graphqlWrappedResult[0][responseName] = resolved[0]; - graphqlWrappedResult[1].push(...resolved[1]); + addIncrementalDataRecords(graphqlWrappedResult, resolved[1]); return graphqlWrappedResult; }); } graphqlWrappedResult[0][responseName] = result[0]; - graphqlWrappedResult[1].push(...result[1]); + addIncrementalDataRecords(graphqlWrappedResult, result[1]); return graphqlWrappedResult; }, - [Object.create(null), []] as GraphQLWrappedResult>, + [Object.create(null), undefined] as GraphQLWrappedResult>, ); } @@ -608,7 +622,7 @@ function executeFields( const results = Object.create(null); const graphqlWrappedResult: GraphQLWrappedResult> = [ results, - [], + undefined, ]; let containsPromise = false; @@ -628,13 +642,13 @@ function executeFields( if (result !== undefined) { if (isPromise(result)) { results[responseName] = result.then((resolved) => { - graphqlWrappedResult[1].push(...resolved[1]); + addIncrementalDataRecords(graphqlWrappedResult, resolved[1]); return resolved[0]; }); containsPromise = true; } else { results[responseName] = result[0]; - graphqlWrappedResult[1].push(...result[1]); + addIncrementalDataRecords(graphqlWrappedResult, result[1]); } } } @@ -746,16 +760,28 @@ 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) => { - const errors = (incrementalContext ?? exeContext).errors; - handleFieldError(rawError, returnType, fieldGroup, path, errors); - return [null, []]; + handleFieldError( + rawError, + exeContext, + returnType, + fieldGroup, + path, + incrementalContext, + ); + return [null, undefined]; }); } return completed; } catch (rawError) { - const errors = (incrementalContext ?? exeContext).errors; - handleFieldError(rawError, returnType, fieldGroup, path, errors); - return [null, []]; + handleFieldError( + rawError, + exeContext, + returnType, + fieldGroup, + path, + incrementalContext, + ); + return [null, undefined]; } } @@ -788,10 +814,11 @@ export function buildResolveInfo( function handleFieldError( rawError: unknown, + exeContext: ExecutionContext, returnType: GraphQLOutputType, fieldGroup: FieldGroup, path: Path, - errors: Array, + incrementalContext: IncrementalContext | undefined, ): void { const error = locatedError(rawError, toNodes(fieldGroup), pathToArray(path)); @@ -803,6 +830,12 @@ function handleFieldError( // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. + const context = incrementalContext ?? exeContext; + let errors = context.errors; + if (errors === undefined) { + errors = []; + context.errors = errors; + } errors.push(error); } @@ -865,7 +898,7 @@ function completeValue( // If result value is null or undefined then return null. if (result == null) { - return [null, []]; + return [null, undefined]; } // If field type is List, complete each item in the list with the inner type @@ -885,7 +918,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), undefined]; } // If field type is an abstract type, Interface or Union, determine the @@ -952,9 +985,15 @@ async function completePromisedValue( } return completed; } catch (rawError) { - const errors = (incrementalContext ?? exeContext).errors; - handleFieldError(rawError, returnType, fieldGroup, path, errors); - return [null, []]; + handleFieldError( + rawError, + exeContext, + returnType, + fieldGroup, + path, + incrementalContext, + ); + return [null, undefined]; } } @@ -1049,7 +1088,7 @@ async function completeAsyncIteratorValue( const completedResults: Array = []; const graphqlWrappedResult: GraphQLWrappedResult> = [ completedResults, - [], + undefined, ]; let index = 0; // eslint-disable-next-line no-constant-condition @@ -1137,7 +1176,7 @@ async function completeAsyncIteratorValueWithPossibleStream( const completedResults: Array = []; const graphqlWrappedResult: GraphQLWrappedResult> = [ completedResults, - [], + undefined, ]; let index = 0; // eslint-disable-next-line no-constant-condition @@ -1156,6 +1195,9 @@ async function completeAsyncIteratorValueWithPossibleStream( path, earlyReturn: returnFn.bind(asyncIterator), }; + if (exeContext.cancellableStreams === undefined) { + exeContext.cancellableStreams = new Set(); + } exeContext.cancellableStreams.add(streamRecord); } @@ -1170,7 +1212,7 @@ async function completeAsyncIteratorValueWithPossibleStream( itemType, ); - graphqlWrappedResult[1].push(firstStreamItems); + addIncrementalDataRecords(graphqlWrappedResult, [firstStreamItems]); break; } @@ -1332,7 +1374,7 @@ function completeIterableValue( const completedResults: Array = []; const graphqlWrappedResult: GraphQLWrappedResult> = [ completedResults, - [], + undefined, ]; let index = 0; for (const item of items) { @@ -1399,7 +1441,7 @@ function completeIterableValueWithPossibleStream( const completedResults: Array = []; const graphqlWrappedResult: GraphQLWrappedResult> = [ completedResults, - [], + undefined, ]; let index = 0; const iterator = items[Symbol.iterator](); @@ -1424,7 +1466,7 @@ function completeIterableValueWithPossibleStream( itemType, ); - graphqlWrappedResult[1].push(firstStreamItems); + addIncrementalDataRecords(graphqlWrappedResult, [firstStreamItems]); break; } @@ -1511,12 +1553,18 @@ function completeListItemValue( completedResults.push( completedItem.then( (resolved) => { - parent[1].push(...resolved[1]); + addIncrementalDataRecords(parent, resolved[1]); return resolved[0]; }, (rawError) => { - const errors = (incrementalContext ?? exeContext).errors; - handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); + handleFieldError( + rawError, + exeContext, + itemType, + fieldGroup, + itemPath, + incrementalContext, + ); return null; }, ), @@ -1525,10 +1573,16 @@ function completeListItemValue( } completedResults.push(completedItem[0]); - parent[1].push(...completedItem[1]); + addIncrementalDataRecords(parent, completedItem[1]); } catch (rawError) { - const errors = (incrementalContext ?? exeContext).errors; - handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); + handleFieldError( + rawError, + exeContext, + itemType, + fieldGroup, + itemPath, + incrementalContext, + ); completedResults.push(null); } return false; @@ -1560,11 +1614,17 @@ async function completePromisedListItemValue( if (isPromise(completed)) { completed = await completed; } - parent[1].push(...completed[1]); + addIncrementalDataRecords(parent, completed[1]); return completed[0]; } catch (rawError) { - const errors = (incrementalContext ?? exeContext).errors; - handleFieldError(rawError, itemType, fieldGroup, itemPath, errors); + handleFieldError( + rawError, + exeContext, + itemType, + fieldGroup, + itemPath, + incrementalContext, + ); return null; } } @@ -2222,7 +2282,7 @@ function executeDeferredGroupedFieldSets( path, groupedFieldSet, { - errors: [], + errors: undefined, deferUsageSet, }, deferMap, @@ -2312,7 +2372,7 @@ function executeDeferredGroupedFieldSet( } function buildDeferredGroupedFieldSetResult( - errors: ReadonlyArray, + errors: ReadonlyArray | undefined, deferredFragmentRecords: ReadonlyArray, path: Path | undefined, result: GraphQLWrappedResult>, @@ -2321,7 +2381,7 @@ function buildDeferredGroupedFieldSetResult( deferredFragmentRecords, path: pathToArray(path), result: - errors.length === 0 ? { data: result[0] } : { data: result[0], errors }, + errors === undefined ? { data: result[0] } : { data: result[0], errors }, incrementalDataRecords: result[1], }; } @@ -2356,7 +2416,7 @@ function firstSyncStreamItems( initialPath, initialItem, exeContext, - { errors: [] }, + { errors: undefined }, fieldGroup, info, itemType, @@ -2378,7 +2438,7 @@ function firstSyncStreamItems( currentPath, item, exeContext, - { errors: [] }, + { errors: undefined }, fieldGroup, info, itemType, @@ -2426,15 +2486,17 @@ function prependNextResolvedStreamItems( result: StreamItemsResult, nextStreamItems: StreamItemsRecord, ): StreamItemsResult { - return isReconcilableStreamItemsResult(result) - ? { - ...result, - incrementalDataRecords: [ - nextStreamItems, - ...result.incrementalDataRecords, - ], - } - : result; + if (!isReconcilableStreamItemsResult(result)) { + return result; + } + const incrementalDataRecords = result.incrementalDataRecords; + return { + ...result, + incrementalDataRecords: + incrementalDataRecords === undefined + ? [nextStreamItems] + : [nextStreamItems, ...incrementalDataRecords], + }; } function firstAsyncStreamItems( @@ -2494,7 +2556,7 @@ async function getNextAsyncStreamItemsResult( itemPath, iteration.value, exeContext, - { errors: [] }, + { errors: undefined }, fieldGroup, info, itemType, @@ -2567,12 +2629,13 @@ function completeStreamItems( } catch (rawError) { handleFieldError( rawError, + exeContext, itemType, fieldGroup, itemPath, - incrementalContext.errors, + incrementalContext, ); - result = [null, []]; + result = [null, undefined]; } } catch (error) { return { @@ -2586,12 +2649,13 @@ function completeStreamItems( .then(undefined, (rawError) => { handleFieldError( rawError, + exeContext, itemType, fieldGroup, itemPath, - incrementalContext.errors, + incrementalContext, ); - return [null, []] as GraphQLWrappedResult; + return [null, undefined] as GraphQLWrappedResult; }) .then( (resolvedItem) => @@ -2615,14 +2679,14 @@ function completeStreamItems( } function buildStreamItemsResult( - errors: ReadonlyArray, + errors: ReadonlyArray | undefined, streamRecord: SubsequentResultRecord, result: GraphQLWrappedResult, ): StreamItemsResult { return { streamRecord, result: - errors.length === 0 + errors === undefined ? { items: [result[0]] } : { items: [result[0]],