From f2ec6a0a320208e95b95d67602f75c16d1edb393 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 21 Mar 2024 12:57:43 +0200 Subject: [PATCH] remove filtering again --- src/execution/IncrementalPublisher.ts | 17 +- src/execution/__tests__/stream-test.ts | 13 +- src/execution/execute.ts | 565 ++++++++++--------------- src/jsutils/promiseForObject.ts | 22 - 4 files changed, 234 insertions(+), 383 deletions(-) delete mode 100644 src/jsutils/promiseForObject.ts diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index c9d4d67242..1c310cd4f3 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -520,7 +520,7 @@ class IncrementalPublisher { ); } - if (deferredGroupedFieldSetResult.incrementalDataRecords) { + if (deferredGroupedFieldSetResult.incrementalDataRecords.length > 0) { this._addIncrementalDataRecords( deferredGroupedFieldSetResult.incrementalDataRecords, ); @@ -616,7 +616,7 @@ class IncrementalPublisher { this._incremental.push(incrementalEntry); - if (streamItemsResult.incrementalDataRecords) { + if (streamItemsResult.incrementalDataRecords.length > 0) { this._addIncrementalDataRecords( streamItemsResult.incrementalDataRecords, ); @@ -658,13 +658,13 @@ class IncrementalPublisher { } } -export function isDeferredFragmentRecord( +function isDeferredFragmentRecord( subsequentResultRecord: SubsequentResultRecord, ): subsequentResultRecord is DeferredFragmentRecord { return subsequentResultRecord instanceof DeferredFragmentRecord; } -export function isDeferredGroupedFieldSetRecord( +function isDeferredGroupedFieldSetRecord( incrementalDataRecord: IncrementalDataRecord, ): incrementalDataRecord is DeferredGroupedFieldSetRecord { return incrementalDataRecord instanceof DeferredGroupedFieldSetRecord; @@ -673,8 +673,7 @@ export function isDeferredGroupedFieldSetRecord( export interface IncrementalContext { deferUsageSet: DeferUsageSet | undefined; path: Path | undefined; - errors?: Map | undefined; - incrementalDataRecords?: Array | undefined; + errors?: Array | undefined; } export type DeferredGroupedFieldSetResult = @@ -691,7 +690,7 @@ interface ReconcilableDeferredGroupedFieldSetResult { deferredFragmentRecords: ReadonlyArray; path: Array; result: BareDeferredGroupedFieldSetResult; - incrementalDataRecords?: ReadonlyArray | undefined; + incrementalDataRecords: ReadonlyArray; sent?: true | undefined; } @@ -796,7 +795,7 @@ interface NonReconcilableStreamItemsResult { interface NonTerminatingStreamItemsResult { streamRecord: StreamRecord; result: BareStreamItemsResult; - incrementalDataRecords?: ReadonlyArray | undefined; + incrementalDataRecords: ReadonlyArray; } interface TerminatingStreamItemsResult { @@ -860,7 +859,7 @@ export class StreamItemsRecord { ...result, incrementalDataRecords: [ this.nextStreamItems, - ...(result.incrementalDataRecords ?? []), + ...result.incrementalDataRecords, ], } : result; diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index 22be4e4d35..a44c31fa5c 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -2096,23 +2096,14 @@ describe('Execute: stream directive', () => { id: '2', }, ], - completed: [{ id: '2' }], - hasNext: true, + completed: [{ id: '1' }, { id: '2' }], + hasNext: false, }, done: false, }); const result5 = await iterator.next(); expectJSON(result5).toDeepEqual({ - value: { - completed: [{ id: '1' }], - hasNext: false, - }, - done: false, - }); - - const result6 = await iterator.next(); - expectJSON(result6).toDeepEqual({ value: undefined, done: true, }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 8526bc824b..f8f961ca34 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -9,7 +9,6 @@ import { memoize3 } from '../jsutils/memoize3.js'; import type { ObjMap } from '../jsutils/ObjMap.js'; import type { Path } from '../jsutils/Path.js'; import { addPath, pathToArray } from '../jsutils/Path.js'; -import { promiseForObject } from '../jsutils/promiseForObject.js'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; import { promiseReduce } from '../jsutils/promiseReduce.js'; @@ -70,7 +69,6 @@ import { buildIncrementalResponse, DeferredFragmentRecord, DeferredGroupedFieldSetRecord, - isDeferredGroupedFieldSetRecord, StreamItemsRecord, StreamRecord, } from './IncrementalPublisher.js'; @@ -142,9 +140,8 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; - errors?: Map | undefined; + errors?: Array | undefined; cancellableStreams?: Set | undefined; - incrementalDataRecords?: Array; } export interface ExecutionArgs { @@ -165,6 +162,8 @@ export interface StreamUsage { fieldGroup: FieldGroup; } +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.'; @@ -254,7 +253,7 @@ function executeImpl( // at which point we still log the error and null the parent field, which // in this case is the entire response. try { - let data: PromiseOrValue>; + let acc: PromiseOrValue>>; const { operation, schema, fragments, variableValues, rootValue } = exeContext; const rootType = schema.getRootType(operation.operation); @@ -268,7 +267,7 @@ function executeImpl( const { groupedFieldSet: nonPartitionedGroupedFieldSet, newDeferUsages } = collectFields(schema, fragments, variableValues, rootType, operation); if (newDeferUsages.length === 0) { - data = executeRootGroupedFieldSet( + acc = executeRootGroupedFieldSet( exeContext, operation.operation, rootType, @@ -283,7 +282,7 @@ function executeImpl( const newDeferMap = addNewDeferredFragments(newDeferUsages, new Map()); - data = executeRootGroupedFieldSet( + acc = executeRootGroupedFieldSet( exeContext, operation.operation, rootType, @@ -303,133 +302,54 @@ function executeImpl( newDeferMap, ); - addIncrementalDataRecords( - exeContext, + acc = withNewDeferredGroupedFieldSets( + acc, newDeferredGroupedFieldSetRecords, ); } } - - if (isPromise(data)) { - return data.then( - (resolved) => buildDataResponse(exeContext, resolved), + if (isPromise(acc)) { + return acc.then( + (resolved) => buildDataResponse(exeContext, resolved[0], resolved[1]), (error) => ({ data: null, errors: withError(exeContext.errors, error), }), ); } - return buildDataResponse(exeContext, data); + return buildDataResponse(exeContext, acc[0], acc[1]); } catch (error) { return { data: null, errors: withError(exeContext.errors, error) }; } } -function addIncrementalDataRecords( - context: ExecutionContext | IncrementalContext, - newIncrementalDataRecords: ReadonlyArray, -): void { - const incrementalDataRecords = context.incrementalDataRecords; - if (incrementalDataRecords === undefined) { - context.incrementalDataRecords = [...newIncrementalDataRecords]; - return; - } - incrementalDataRecords.push(...newIncrementalDataRecords); -} - function withError( - errors: ReadonlyMap | undefined, + errors: Array | undefined, error: GraphQLError, ): ReadonlyArray { - return errors === undefined ? [error] : [...errors.values(), error]; + return errors === undefined ? [error] : [...errors, error]; } function buildDataResponse( exeContext: ExecutionContext, data: ObjMap, + incrementalDataRecords: ReadonlyArray, ): ExecutionResult | ExperimentalIncrementalExecutionResults { - const { errors, incrementalDataRecords } = exeContext; - if (incrementalDataRecords === undefined) { - return buildSingleResult(data, errors); - } - - if (errors === undefined) { - return buildIncrementalResponse( - exeContext, - data, - undefined, - incrementalDataRecords, - ); - } - - const filteredIncrementalDataRecords = filterIncrementalDataRecords( - undefined, - errors, - incrementalDataRecords, - ); - - if (filteredIncrementalDataRecords.length === 0) { - return buildSingleResult(data, errors); + const { errors } = exeContext; + if (incrementalDataRecords.length === 0) { + return errors !== undefined + ? { errors: Array.from(errors.values()), data } + : { data }; } return buildIncrementalResponse( exeContext, data, - Array.from(errors.values()), - filteredIncrementalDataRecords, + errors, + incrementalDataRecords, ); } -function buildSingleResult( - data: ObjMap, - errors: ReadonlyMap | undefined, -): ExecutionResult { - return errors !== undefined - ? { errors: Array.from(errors.values()), data } - : { data }; -} - -function filterIncrementalDataRecords( - initialPath: Path | undefined, - errors: ReadonlyMap, - incrementalDataRecords: ReadonlyArray, -): ReadonlyArray { - const filteredIncrementalDataRecords: Array = []; - for (const incrementalDataRecord of incrementalDataRecords) { - let currentPath: Path | undefined = isDeferredGroupedFieldSetRecord( - incrementalDataRecord, - ) - ? incrementalDataRecord.path - : incrementalDataRecord.streamRecord.path; - - if (errors.has(currentPath)) { - continue; - } - - const paths: Array = [currentPath]; - let filtered = false; - while (currentPath !== initialPath) { - // Because currentPath leads to initialPath or is undefined, and the - // loop will exit if initialPath is undefined, currentPath must be - // defined. - // TODO: Consider, however, adding an invariant. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - currentPath = currentPath!.prev; - if (errors.has(currentPath)) { - filtered = true; - break; - } - paths.push(currentPath); - } - - if (!filtered) { - filteredIncrementalDataRecords.push(incrementalDataRecord); - } - } - - return filteredIncrementalDataRecords; -} - /** * Also implements the "Executing requests" section of the GraphQL specification. * However, it guarantees to complete synchronously (or throw an error) assuming @@ -552,7 +472,7 @@ function executeRootGroupedFieldSet( rootValue: unknown, groupedFieldSet: GroupedFieldSet, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { +): PromiseOrValue>> { switch (operation) { case OperationTypeNode.QUERY: return executeFields( @@ -599,10 +519,10 @@ function executeFieldsSerially( path: Path | undefined, groupedFieldSet: GroupedFieldSet, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { +): PromiseOrValue>> { return promiseReduce( groupedFieldSet, - (results, [responseName, fieldGroup]) => { + (acc, [responseName, fieldGroup]) => { const fieldPath = addPath(path, responseName, parentType.name); const result = executeField( exeContext, @@ -614,18 +534,20 @@ function executeFieldsSerially( deferMap, ); if (result === undefined) { - return results; + return acc; } if (isPromise(result)) { return result.then((resolved) => { - results[responseName] = resolved; - return results; + acc[0][responseName] = resolved[0]; + acc[1].push(...resolved[1]); + return acc; }); } - results[responseName] = result; - return results; + acc[0][responseName] = result[0]; + acc[1].push(...result[1]); + return acc; }, - Object.create(null), + [Object.create(null), []] as GraphQLResult>, ); } @@ -641,9 +563,9 @@ function executeFields( groupedFieldSet: GroupedFieldSet, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { - const results = Object.create(null); - let containsPromise = false; +): PromiseOrValue>> { + const acc: GraphQLResult> = [Object.create(null), []]; + const promises: Array> = []; try { for (const [responseName, fieldGroup] of groupedFieldSet) { @@ -659,16 +581,24 @@ function executeFields( ); if (result !== undefined) { - results[responseName] = result; if (isPromise(result)) { - containsPromise = true; + acc[0][responseName] = undefined; + promises.push( + result.then((resolved) => { + acc[0][responseName] = resolved[0]; + acc[1].push(...resolved[1]); + }), + ); + } else { + acc[0][responseName] = result[0]; + acc[1].push(...result[1]); } } } } catch (error) { - if (containsPromise) { + if (promises.length > 0) { // Ensure that any promises returned by other fields are handled, as they may also reject. - return promiseForObject(results).finally(() => { + return Promise.all(promises).finally(() => { throw error; }) as never; } @@ -676,14 +606,14 @@ function executeFields( } // If there are no promises, we can just return the object and any incrementalDataRecords - if (!containsPromise) { - return results; + if (promises.length === 0) { + 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 Promise.all(promises).then(() => acc); } function toNodes(fieldGroup: FieldGroup): ReadonlyArray { @@ -704,7 +634,7 @@ function executeField( path: Path, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue { +): PromiseOrValue> | undefined { const fieldName = fieldGroup[0].node.name.value; const fieldDef = exeContext.schema.getField(parentType, fieldName); if (!fieldDef) { @@ -763,7 +693,7 @@ function executeField( path, incrementalContext, ); - return null; + return [null, []]; }); } @@ -790,7 +720,7 @@ function executeField( path, incrementalContext, ); - return null; + return [null, []]; }); } return completed; @@ -803,7 +733,7 @@ function executeField( path, incrementalContext, ); - return null; + return [null, []]; } } @@ -855,10 +785,10 @@ function handleFieldError( const context = incrementalContext ?? exeContext; let errors = context.errors; if (errors === undefined) { - errors = new Map(); + errors = []; context.errors = errors; } - errors.set(path, error); + errors.push(error); } /** @@ -891,7 +821,7 @@ function completeValue( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue { +): PromiseOrValue> { // If result is an Error, throw a located error. if (result instanceof Error) { throw result; @@ -910,7 +840,7 @@ function completeValue( 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}.`, ); @@ -920,7 +850,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 @@ -940,7 +870,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 @@ -1062,9 +992,9 @@ async function completeAsyncIteratorValue( asyncIterator: AsyncIterator, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): Promise> { - let containsPromise = false; - const completedResults: Array = []; +): Promise>> { + const acc: GraphQLResult> = [[], []]; + const promises: Array> = []; let index = 0; // eslint-disable-next-line no-constant-condition while (true) { @@ -1081,26 +1011,24 @@ async function completeAsyncIteratorValue( break; } - if ( - completeListItemValue( - iteration.value, - completedResults, - exeContext, - itemType, - fieldGroup, - info, - itemPath, - incrementalContext, - deferMap, - ) - ) { - containsPromise = true; - } + completeListItemValue( + iteration.value, + acc, + index, + promises, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ); index++; } - return containsPromise ? Promise.all(completedResults) : completedResults; + return promises.length > 0 ? Promise.all(promises).then(() => acc) : acc; } /** @@ -1117,9 +1045,9 @@ async function completeAsyncIteratorValueWithPossibleStream( streamUsage: StreamUsage, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): Promise> { - let containsPromise = false; - const completedResults: Array = []; +): Promise>> { + const acc: GraphQLResult> = [[], []]; + const promises: Array> = []; let index = 0; const initialCount = streamUsage.initialCount; // eslint-disable-next-line no-constant-condition @@ -1155,8 +1083,7 @@ async function completeAsyncIteratorValueWithPossibleStream( ), ); - const context = incrementalContext ?? exeContext; - addIncrementalDataRecord(context, firstStreamItems); + acc[1].push(firstStreamItems); break; } @@ -1175,40 +1102,25 @@ async function completeAsyncIteratorValueWithPossibleStream( break; } - if ( - completeListItemValue( - iteration.value, - completedResults, - exeContext, - itemType, - fieldGroup, - info, - itemPath, - incrementalContext, - deferMap, - ) /* c8 ignore start */ - ) { - // TODO: add test case for asyncIterator that yields promises - containsPromise = true; - } /* c8 ignore stop */ + completeListItemValue( + iteration.value, + acc, + index, + promises, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ); index++; } - return containsPromise - ? /* c8 ignore start */ Promise.all(completedResults) - : /* c8 ignore stop */ completedResults; -} - -function addIncrementalDataRecord( - context: ExecutionContext | IncrementalContext, - newIncrementalDataRecord: IncrementalDataRecord, -): void { - const incrementalDataRecords = context.incrementalDataRecords; - if (incrementalDataRecords === undefined) { - context.incrementalDataRecords = [newIncrementalDataRecord]; - return; - } - incrementalDataRecords.push(newIncrementalDataRecord); + return promises.length > 0 + ? /* c8 ignore start */ Promise.all(promises).then(() => acc) + : /* c8 ignore stop */ acc; } /** @@ -1224,7 +1136,7 @@ function completeListValue( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { +): PromiseOrValue>> { const itemType = returnType.ofType; const streamUsage = getStreamUsage(exeContext, fieldGroup, path); @@ -1298,37 +1210,35 @@ function completeIterableValue( items: Iterable, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { - let index = 0; +): PromiseOrValue>> { // 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; - const completedResults: Array = []; + const acc: GraphQLResult> = [[], []]; + const promises: Array> = []; + let index = 0; for (const item of items) { // 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 ( - completeListItemValue( - item, - completedResults, - exeContext, - itemType, - fieldGroup, - info, - itemPath, - incrementalContext, - deferMap, - ) - ) { - containsPromise = true; - } + completeListItemValue( + item, + acc, + index, + promises, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ); index++; } - return containsPromise ? Promise.all(completedResults) : completedResults; + return promises.length > 0 ? Promise.all(promises).then(() => acc) : acc; } function completeIterableValueWithPossibleStream( @@ -1341,12 +1251,12 @@ function completeIterableValueWithPossibleStream( streamUsage: StreamUsage, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { - let index = 0; +): PromiseOrValue>> { // 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; - const completedResults: Array = []; + const acc: GraphQLResult> = [[], []]; + const promises: Array> = []; + let index = 0; const initialCount = streamUsage.initialCount; const iterator = items[Symbol.iterator](); let iteration = iterator.next(); @@ -1377,8 +1287,7 @@ function completeIterableValueWithPossibleStream( ), ); - const context = incrementalContext ?? exeContext; - addIncrementalDataRecord(context, firstStreamItems); + acc[1].push(firstStreamItems); break; } @@ -1386,28 +1295,26 @@ function completeIterableValueWithPossibleStream( // since from here on it is not ever accessed by resolver functions. const itemPath = addPath(path, index, undefined); - if ( - completeListItemValue( - item, - completedResults, - exeContext, - itemType, - fieldGroup, - info, - itemPath, - incrementalContext, - deferMap, - ) - ) { - containsPromise = true; - } + completeListItemValue( + item, + acc, + index, + promises, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ); index++; iteration = iterator.next(); } - return containsPromise ? Promise.all(completedResults) : completedResults; + return promises.length > 0 ? Promise.all(promises).then(() => acc) : acc; } /** @@ -1417,7 +1324,9 @@ function completeIterableValueWithPossibleStream( */ function completeListItemValue( item: unknown, - completedResults: Array, + acc: GraphQLResult>, + index: number, + promises: Array>, exeContext: ExecutionContext, itemType: GraphQLOutputType, fieldGroup: FieldGroup, @@ -1425,9 +1334,10 @@ function completeListItemValue( itemPath: Path, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): boolean { +): void { if (isPromise(item)) { - completedResults.push( + acc[0].push(undefined); + promises.push( item .then((resolved) => completeValue( @@ -1450,11 +1360,14 @@ function completeListItemValue( itemPath, incrementalContext, ); - return null; + return [null, []] as GraphQLResult; + }) + .then((resolved) => { + acc[0][index] = resolved[0]; + acc[1].push(...resolved[1]); }), ); - - return true; + return; } try { @@ -1470,26 +1383,31 @@ function completeListItemValue( ); if (isPromise(completedItem)) { + acc[0].push(undefined); // 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, - incrementalContext, - ); - return null; - }), + promises.push( + completedItem + .then(undefined, (rawError) => { + handleFieldError( + rawError, + exeContext, + itemType, + fieldGroup, + itemPath, + incrementalContext, + ); + return [null, []] as GraphQLResult; + }) + .then((resolved) => { + acc[0][index] = resolved[0]; + acc[1].push(...resolved[1]); + }), ); - - return true; + } else { + acc[0][index] = completedItem[0]; + acc[1].push(...completedItem[1]); } - - completedResults.push(completedItem); } catch (rawError) { handleFieldError( rawError, @@ -1499,10 +1417,8 @@ function completeListItemValue( itemPath, incrementalContext, ); - completedResults.push(null); + acc[0].push(null); } - - return false; } /** @@ -1536,7 +1452,7 @@ function completeAbstractValue( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { +): PromiseOrValue>> { const resolveTypeFn = returnType.resolveType ?? exeContext.typeResolver; const contextValue = exeContext.contextValue; const runtimeType = resolveTypeFn(result, contextValue, info, returnType); @@ -1649,7 +1565,7 @@ function completeObjectValue( result: unknown, incrementalContext: IncrementalContext | undefined, 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. @@ -1761,7 +1677,7 @@ function collectAndExecuteSubfields( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, -): PromiseOrValue> { +): PromiseOrValue>> { // Collect sub-fields to execute to complete this value. const { groupedFieldSet: nonPartitionedGroupedFieldSet, newDeferUsages } = collectSubfields(exeContext, returnType, fieldGroup); @@ -1804,9 +1720,12 @@ function collectAndExecuteSubfields( deferMap, ); - const context = incrementalContext ?? exeContext; - addIncrementalDataRecords(context, newDeferredGroupedFieldSetRecords); + return withNewDeferredGroupedFieldSets( + subFields, + newDeferredGroupedFieldSetRecords, + ); } + return subFields; } @@ -1841,9 +1760,12 @@ function collectAndExecuteSubfields( newDeferMap, ); - const context = incrementalContext ?? exeContext; - addIncrementalDataRecords(context, newDeferredGroupedFieldSetRecords); + return withNewDeferredGroupedFieldSets( + subFields, + newDeferredGroupedFieldSetRecords, + ); } + return subFields; } @@ -1863,6 +1785,19 @@ function buildSubFieldPlan( return fieldPlan; } +function withNewDeferredGroupedFieldSets( + result: PromiseOrValue>>, + newDeferredGroupedFieldSetRecords: ReadonlyArray, +): PromiseOrValue>> { + if (isPromise(result)) { + return result.then((resolved) => [ + resolved[0], + [...resolved[1], ...newDeferredGroupedFieldSetRecords], + ]); + } + return [result[0], [...result[1], ...newDeferredGroupedFieldSetRecords]]; +} + /** * If a resolveType function is not given, then a default resolve behavior is * used which attempts two strategies: @@ -2205,9 +2140,9 @@ function executeDeferredGroupedFieldSet( incrementalContext: IncrementalContext, deferMap: ReadonlyMap, ): PromiseOrValue { - let data; + let result; try { - data = executeFields( + result = executeFields( exeContext, parentType, sourceValue, @@ -2225,8 +2160,8 @@ function executeDeferredGroupedFieldSet( }; } - if (isPromise(data)) { - return data.then( + if (isPromise(result)) { + return result.then( (resolved) => buildDeferredGroupedFieldSetResult( incrementalContext, @@ -2247,7 +2182,7 @@ function executeDeferredGroupedFieldSet( incrementalContext, deferredFragmentRecords, path, - data, + result, ); } @@ -2255,40 +2190,15 @@ function buildDeferredGroupedFieldSetResult( incrementalContext: IncrementalContext, deferredFragmentRecords: ReadonlyArray, path: Path | undefined, - data: ObjMap, + result: GraphQLResult>, ): DeferredGroupedFieldSetResult { - const { errors, incrementalDataRecords } = incrementalContext; - if (incrementalDataRecords === undefined) { - return { - deferredFragmentRecords, - path: pathToArray(path), - result: - errors === undefined - ? { - data, - } - : { data, errors: [...errors.values()] }, - }; - } - - if (errors === undefined) { - return { - deferredFragmentRecords, - path: pathToArray(path), - result: { data }, - incrementalDataRecords, - }; - } - + const { errors } = incrementalContext; return { deferredFragmentRecords, path: pathToArray(path), - result: { data, errors: [...errors.values()] }, - incrementalDataRecords: filterIncrementalDataRecords( - path, - errors, - incrementalDataRecords, - ), + result: + errors === undefined ? { data: result[0] } : { data: result[0], errors }, + incrementalDataRecords: result[1], }; } @@ -2503,7 +2413,7 @@ function completeStreamItems( itemPath, incrementalContext, ); - return null; + return [null, []] as GraphQLResult; }) .then( (resolvedItem) => @@ -2520,10 +2430,10 @@ function completeStreamItems( ); } - let completedItem; + let result: PromiseOrValue>; try { try { - completedItem = completeValue( + result = completeValue( exeContext, itemType, fieldGroup, @@ -2542,7 +2452,7 @@ function completeStreamItems( itemPath, incrementalContext, ); - completedItem = null; + result = [null, []]; } } catch (error) { return { @@ -2552,8 +2462,8 @@ function completeStreamItems( }; } - if (isPromise(completedItem)) { - return completedItem + if (isPromise(result)) { + return result .then(undefined, (rawError) => { handleFieldError( rawError, @@ -2563,7 +2473,7 @@ function completeStreamItems( itemPath, incrementalContext, ); - return null; + return [null, []] as GraphQLResult; }) .then( (resolvedItem) => @@ -2580,51 +2490,24 @@ function completeStreamItems( ); } - return buildStreamItemsResult( - incrementalContext, - streamRecord, - completedItem, - ); + return buildStreamItemsResult(incrementalContext, streamRecord, result); } function buildStreamItemsResult( incrementalContext: IncrementalContext, streamRecord: StreamRecord, - completedItem: unknown, + result: GraphQLResult, ): StreamItemsResult { - const { errors, incrementalDataRecords } = incrementalContext; - if (incrementalDataRecords === undefined) { - return { - streamRecord, - result: - errors === undefined - ? { items: [completedItem] } - : { - items: [completedItem], - errors: [...errors.values()], - }, - }; - } - - if (errors === undefined) { - return { - streamRecord, - result: { items: [completedItem] }, - incrementalDataRecords, - }; - } - - const path = incrementalContext.path; + const { errors } = incrementalContext; return { streamRecord, - result: { - items: [completedItem], - errors: [...errors.values()], - }, - incrementalDataRecords: filterIncrementalDataRecords( - path, - errors, - incrementalDataRecords, - ), + result: + errors === undefined + ? { items: [result[0]] } + : { + items: [result[0]], + errors: [...errors], + }, + incrementalDataRecords: result[1], }; } diff --git a/src/jsutils/promiseForObject.ts b/src/jsutils/promiseForObject.ts deleted file mode 100644 index ff48d9f218..0000000000 --- a/src/jsutils/promiseForObject.ts +++ /dev/null @@ -1,22 +0,0 @@ -import type { ObjMap } from './ObjMap.js'; - -/** - * This function transforms a JS object `ObjMap>` into - * a `Promise>` - * - * 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( - object: ObjMap>, -): Promise> { - const keys = Object.keys(object); - const values = Object.values(object); - - const resolvedValues = await Promise.all(values); - const resolvedObject = Object.create(null); - for (let i = 0; i < keys.length; ++i) { - resolvedObject[keys[i]] = resolvedValues[i]; - } - return resolvedObject; -}