From e89cbcfe4e843a5599b5520b0d5960cd30dcf8ec Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 22 Jul 2024 17:50:13 +0300 Subject: [PATCH 1/9] experiment: lazily create DeferredFragments goal: avoid creating or passing around the deferMap methodology: - each DeferredFragmentRecord will be unique for a given deferUsage and creationPath - we annotate the path and deferUsage with a "fieldDepth" property representing the number of nested fields in the operation corresponding to the current execution path and the defer directive. - from a path for a deferredGroupedFieldSet, we can derive the path for the deferredFragment from a given deferUsage by "rewinding" the deferredGroupedFieldSet path to the depth of the given deferUsage - we also add a "fieldDepth" property to the FieldDetails structure as well so as to not require an additional depth argument to collectFields which would complicate memoization --- src/execution/IncrementalGraph.ts | 86 ++++++++- src/execution/IncrementalPublisher.ts | 21 ++- src/execution/__tests__/executor-test.ts | 5 +- src/execution/collectFields.ts | 61 +++--- src/execution/execute.ts | 175 +++--------------- src/execution/types.ts | 7 +- src/jsutils/Path.ts | 30 ++- src/jsutils/__tests__/Path-test.ts | 2 + .../rules/SingleFieldSubscriptionsRule.ts | 2 +- 9 files changed, 186 insertions(+), 203 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index cc31c21207..ac21be7d19 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -1,12 +1,14 @@ import { BoxedPromiseOrValue } from '../jsutils/BoxedPromiseOrValue.js'; import { invariant } from '../jsutils/invariant.js'; import { isPromise } from '../jsutils/isPromise.js'; +import type { Path } from '../jsutils/Path.js'; +import { pathAtFieldDepth } from '../jsutils/Path.js'; import { promiseWithResolvers } from '../jsutils/promiseWithResolvers.js'; import type { GraphQLError } from '../error/GraphQLError.js'; +import type { DeferUsage } from './collectFields.js'; import type { - DeferredFragmentRecord, DeferredGroupedFieldSetRecord, IncrementalDataRecord, IncrementalDataRecordResult, @@ -16,6 +18,7 @@ import type { SubsequentResultRecord, } from './types.js'; import { + DeferredFragmentRecord, isDeferredFragmentRecord, isDeferredGroupedFieldSetRecord, } from './types.js'; @@ -26,6 +29,16 @@ import { export class IncrementalGraph { private _rootNodes: Set; + private _rootDeferredFragments = new WeakMap< + DeferUsage, + DeferredFragmentRecord + >(); + + private _nonRootDeferredFragments = new WeakMap< + Path, + WeakMap + >(); + private _completedQueue: Array; private _nextQueue: Array< (iterable: Iterable | undefined) => void @@ -52,8 +65,10 @@ export class IncrementalGraph { addCompletedReconcilableDeferredGroupedFieldSet( reconcilableResult: ReconcilableDeferredGroupedFieldSetResult, ): void { - for (const deferredFragmentRecord of reconcilableResult - .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + const deferredFragmentRecords = this.getDeferredFragmentRecords( + reconcilableResult.deferredGroupedFieldSetRecord, + ); + for (const deferredFragmentRecord of deferredFragmentRecords) { deferredFragmentRecord.deferredGroupedFieldSetRecords.delete( reconcilableResult.deferredGroupedFieldSetRecord, ); @@ -64,12 +79,21 @@ export class IncrementalGraph { if (incrementalDataRecords !== undefined) { this._addIncrementalDataRecords( incrementalDataRecords, - reconcilableResult.deferredGroupedFieldSetRecord - .deferredFragmentRecords, + deferredFragmentRecords, ); } } + getDeferredFragmentRecords( + deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, + ): Array { + const { deferUsages, path } = deferredGroupedFieldSetRecord; + return Array.from(deferUsages).map((deferUsage) => { + const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); + return this._getDeferredFragmentRecord(deferUsage, deferUsagePath); + }); + } + *currentCompletedBatch(): Generator { let completed; while ((completed = this._completedQueue.shift()) !== undefined) { @@ -119,8 +143,10 @@ export class IncrementalGraph { ); this._removeRootNode(deferredFragmentRecord); for (const reconcilableResult of reconcilableResults) { - for (const otherDeferredFragmentRecord of reconcilableResult - .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + const deferredFragmentRecords = this.getDeferredFragmentRecords( + reconcilableResult.deferredGroupedFieldSetRecord, + ); + for (const otherDeferredFragmentRecord of deferredFragmentRecords) { otherDeferredFragmentRecord.reconcilableResults.delete( reconcilableResult, ); @@ -146,6 +172,40 @@ export class IncrementalGraph { this._removeRootNode(streamRecord); } + private _getDeferredFragmentRecord( + deferUsage: DeferUsage, + path: Path | undefined, + ): DeferredFragmentRecord { + let deferredFragmentRecords: + | WeakMap + | undefined; + if (path === undefined) { + deferredFragmentRecords = this._rootDeferredFragments; + } else { + deferredFragmentRecords = this._nonRootDeferredFragments.get(path); + if (deferredFragmentRecords === undefined) { + deferredFragmentRecords = new WeakMap(); + this._nonRootDeferredFragments.set(path, deferredFragmentRecords); + } + } + let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); + if (deferredFragmentRecord === undefined) { + let parent: DeferredFragmentRecord | undefined; + const parentDeferUsage = deferUsage.parentDeferUsage; + if (parentDeferUsage !== undefined) { + const parentPath = pathAtFieldDepth(path, parentDeferUsage.fieldDepth); + parent = this._getDeferredFragmentRecord(parentDeferUsage, parentPath); + } + deferredFragmentRecord = new DeferredFragmentRecord( + path, + deferUsage.label, + parent, + ); + deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); + } + return deferredFragmentRecord; + } + private _removeRootNode( subsequentResultRecord: SubsequentResultRecord, ): void { @@ -159,7 +219,10 @@ export class IncrementalGraph { ): void { for (const incrementalDataRecord of incrementalDataRecords) { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { - for (const deferredFragmentRecord of incrementalDataRecord.deferredFragmentRecords) { + const deferredFragmentRecords = this.getDeferredFragmentRecords( + incrementalDataRecord, + ); + for (const deferredFragmentRecord of deferredFragmentRecords) { this._addDeferredFragment( deferredFragmentRecord, initialResultChildren, @@ -216,8 +279,11 @@ export class IncrementalGraph { private _completesRootNode( deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, ): boolean { - return deferredGroupedFieldSetRecord.deferredFragmentRecords.some( - (deferredFragmentRecord) => this._rootNodes.has(deferredFragmentRecord), + const deferredFragmentRecords = this.getDeferredFragmentRecords( + deferredGroupedFieldSetRecord, + ); + return deferredFragmentRecords.some((deferredFragmentRecord) => + this._rootNodes.has(deferredFragmentRecord), ); } diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index dd27033ed8..3e70b278fd 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -226,8 +226,11 @@ class IncrementalPublisher { deferredGroupedFieldSetResult, ) ) { - for (const deferredFragmentRecord of deferredGroupedFieldSetResult - .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + const deferredFragmentRecords = + this._incrementalGraph.getDeferredFragmentRecords( + deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord, + ); + for (const deferredFragmentRecord of deferredFragmentRecords) { const id = deferredFragmentRecord.id; if ( !this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord) @@ -248,8 +251,11 @@ class IncrementalPublisher { deferredGroupedFieldSetResult, ); - for (const deferredFragmentRecord of deferredGroupedFieldSetResult - .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + const deferredFragmentRecords = + this._incrementalGraph.getDeferredFragmentRecords( + deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord, + ); + for (const deferredFragmentRecord of deferredFragmentRecords) { const completion = this._incrementalGraph.completeDeferredFragment( deferredFragmentRecord, ); @@ -334,8 +340,11 @@ class IncrementalPublisher { let maxLength = pathToArray(initialDeferredFragmentRecord.path).length; let bestId = initialId; - for (const deferredFragmentRecord of deferredGroupedFieldSetResult - .deferredGroupedFieldSetRecord.deferredFragmentRecords) { + const deferredFragmentRecords = + this._incrementalGraph.getDeferredFragmentRecords( + deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord, + ); + for (const deferredFragmentRecord of deferredFragmentRecords) { if (deferredFragmentRecord === initialDeferredFragmentRecord) { continue; } diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index de33f8c91b..97ae573241 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -239,7 +239,7 @@ describe('Execute: Handles basic execution tasks', () => { const field = operation.selectionSet.selections[0]; expect(resolvedInfo).to.deep.include({ fieldNodes: [field], - path: { prev: undefined, key: 'result', typename: 'Test' }, + path: { prev: undefined, key: 'result', typename: 'Test', fieldDepth: 1 }, variableValues: { var: 'abc' }, }); }); @@ -291,12 +291,15 @@ describe('Execute: Handles basic execution tasks', () => { expect(path).to.deep.equal({ key: 'l2', typename: 'SomeObject', + fieldDepth: 2, prev: { key: 0, typename: undefined, + fieldDepth: 1, prev: { key: 'l1', typename: 'SomeQuery', + fieldDepth: 1, prev: undefined, }, }, diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index d411ff3f77..4171cfbfac 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -28,17 +28,21 @@ import { getDirectiveValues } from './values.js'; export interface DeferUsage { label: string | undefined; + fieldDepth: number; parentDeferUsage: DeferUsage | undefined; } export interface FieldDetails { node: FieldNode; + fieldDepth: number; deferUsage: DeferUsage | undefined; } export type FieldGroup = ReadonlyArray; -export type GroupedFieldSet = ReadonlyMap; +export type GroupedFieldSet = ReadonlyMap & { + encounteredDefer?: boolean; +}; interface CollectFieldsContext { schema: GraphQLSchema; @@ -64,12 +68,8 @@ export function collectFields( variableValues: { [variable: string]: unknown }, runtimeType: GraphQLObjectType, operation: OperationDefinitionNode, -): { - groupedFieldSet: GroupedFieldSet; - newDeferUsages: ReadonlyArray; -} { +): GroupedFieldSet { const groupedFieldSet = new AccumulatorMap(); - const newDeferUsages: Array = []; const context: CollectFieldsContext = { schema, fragments, @@ -79,13 +79,8 @@ export function collectFields( visitedFragmentNames: new Set(), }; - collectFieldsImpl( - context, - operation.selectionSet, - groupedFieldSet, - newDeferUsages, - ); - return { groupedFieldSet, newDeferUsages }; + collectFieldsImpl(context, operation.selectionSet, groupedFieldSet); + return groupedFieldSet; } /** @@ -106,10 +101,7 @@ export function collectSubfields( operation: OperationDefinitionNode, returnType: GraphQLObjectType, fieldGroup: FieldGroup, -): { - groupedFieldSet: GroupedFieldSet; - newDeferUsages: ReadonlyArray; -} { +): GroupedFieldSet { const context: CollectFieldsContext = { schema, fragments, @@ -119,32 +111,30 @@ export function collectSubfields( visitedFragmentNames: new Set(), }; const subGroupedFieldSet = new AccumulatorMap(); - const newDeferUsages: Array = []; for (const fieldDetail of fieldGroup) { - const node = fieldDetail.node; + const { node, fieldDepth, deferUsage } = fieldDetail; if (node.selectionSet) { collectFieldsImpl( context, node.selectionSet, subGroupedFieldSet, - newDeferUsages, - fieldDetail.deferUsage, + fieldDepth + 1, + deferUsage, ); } } - return { - groupedFieldSet: subGroupedFieldSet, - newDeferUsages, - }; + return subGroupedFieldSet; } function collectFieldsImpl( context: CollectFieldsContext, selectionSet: SelectionSetNode, - groupedFieldSet: AccumulatorMap, - newDeferUsages: Array, + groupedFieldSet: AccumulatorMap & { + encounteredDefer?: boolean; + }, + fieldDepth = 0, deferUsage?: DeferUsage, ): void { const { @@ -164,6 +154,7 @@ function collectFieldsImpl( } groupedFieldSet.add(getFieldEntryKey(selection), { node: selection, + fieldDepth, deferUsage, }); break; @@ -180,6 +171,7 @@ function collectFieldsImpl( operation, variableValues, selection, + fieldDepth, deferUsage, ); @@ -188,16 +180,16 @@ function collectFieldsImpl( context, selection.selectionSet, groupedFieldSet, - newDeferUsages, + fieldDepth, deferUsage, ); } else { - newDeferUsages.push(newDeferUsage); + groupedFieldSet.encounteredDefer = true; collectFieldsImpl( context, selection.selectionSet, groupedFieldSet, - newDeferUsages, + fieldDepth, newDeferUsage, ); } @@ -211,6 +203,7 @@ function collectFieldsImpl( operation, variableValues, selection, + fieldDepth, deferUsage, ); @@ -235,16 +228,16 @@ function collectFieldsImpl( context, fragment.selectionSet, groupedFieldSet, - newDeferUsages, + fieldDepth, deferUsage, ); } else { - newDeferUsages.push(newDeferUsage); + groupedFieldSet.encounteredDefer = true; collectFieldsImpl( context, fragment.selectionSet, groupedFieldSet, - newDeferUsages, + fieldDepth, newDeferUsage, ); } @@ -263,6 +256,7 @@ function getDeferUsage( operation: OperationDefinitionNode, variableValues: { [variable: string]: unknown }, node: FragmentSpreadNode | InlineFragmentNode, + fieldDepth: number, parentDeferUsage: DeferUsage | undefined, ): DeferUsage | undefined { const defer = getDirectiveValues(GraphQLDeferDirective, node, variableValues); @@ -283,6 +277,7 @@ function getDeferUsage( return { label: typeof defer.label === 'string' ? defer.label : undefined, parentDeferUsage, + fieldDepth, }; } diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 68a901f4b7..adc12e936f 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -50,11 +50,7 @@ import { assertValidSchema } from '../type/validate.js'; import type { DeferUsageSet, FieldPlan } from './buildFieldPlan.js'; import { buildFieldPlan } from './buildFieldPlan.js'; -import type { - DeferUsage, - FieldGroup, - GroupedFieldSet, -} from './collectFields.js'; +import type { FieldGroup, GroupedFieldSet } from './collectFields.js'; import { collectFields, collectSubfields as _collectSubfields, @@ -72,7 +68,6 @@ import type { StreamItemResult, StreamRecord, } from './types.js'; -import { DeferredFragmentRecord } from './types.js'; import { getArgumentValues, getDirectiveValues, @@ -142,6 +137,7 @@ export interface ExecutionContext { subscribeFieldResolver: GraphQLFieldResolver; enableEarlyExecution: boolean; errors: Array | undefined; + encounteredDefer: boolean; cancellableStreams: Set | undefined; } @@ -274,32 +270,32 @@ function executeOperation( ); } - const collectedFields = collectFields( + const originalGroupedFieldSet = collectFields( schema, fragments, variableValues, rootType, operation, ); - let groupedFieldSet = collectedFields.groupedFieldSet; - const newDeferUsages = collectedFields.newDeferUsages; let graphqlWrappedResult: PromiseOrValue< GraphQLWrappedResult> >; - if (newDeferUsages.length === 0) { + if ( + !exeContext.encounteredDefer && + originalGroupedFieldSet.encounteredDefer !== true + ) { graphqlWrappedResult = executeRootGroupedFieldSet( exeContext, operation.operation, rootType, rootValue, - groupedFieldSet, - undefined, + originalGroupedFieldSet, ); } else { - const fieldPLan = buildFieldPlan(groupedFieldSet); - groupedFieldSet = fieldPLan.groupedFieldSet; - const newGroupedFieldSets = fieldPLan.newGroupedFieldSets; - const newDeferMap = addNewDeferredFragments(newDeferUsages, new Map()); + exeContext.encounteredDefer = true; + const { groupedFieldSet, newGroupedFieldSets } = buildFieldPlan( + originalGroupedFieldSet, + ); graphqlWrappedResult = executeRootGroupedFieldSet( exeContext, @@ -307,7 +303,6 @@ function executeOperation( rootType, rootValue, groupedFieldSet, - newDeferMap, ); if (newGroupedFieldSets.size > 0) { @@ -319,7 +314,6 @@ function executeOperation( undefined, undefined, newGroupedFieldSets, - newDeferMap, ); graphqlWrappedResult = withNewDeferredGroupedFieldSets( @@ -505,6 +499,7 @@ export function buildExecutionContext( subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, enableEarlyExecution: enableEarlyExecution === true, errors: undefined, + encounteredDefer: false, cancellableStreams: undefined, }; } @@ -526,7 +521,6 @@ function executeRootGroupedFieldSet( rootType: GraphQLObjectType, rootValue: unknown, groupedFieldSet: GroupedFieldSet, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { switch (operation) { case OperationTypeNode.QUERY: @@ -537,7 +531,6 @@ function executeRootGroupedFieldSet( undefined, groupedFieldSet, undefined, - deferMap, ); case OperationTypeNode.MUTATION: return executeFieldsSerially( @@ -547,7 +540,6 @@ function executeRootGroupedFieldSet( undefined, groupedFieldSet, undefined, - deferMap, ); case OperationTypeNode.SUBSCRIPTION: // TODO: deprecate `subscribe` and move all logic here @@ -559,7 +551,6 @@ function executeRootGroupedFieldSet( undefined, groupedFieldSet, undefined, - deferMap, ); } } @@ -575,7 +566,6 @@ function executeFieldsSerially( path: Path | undefined, groupedFieldSet: GroupedFieldSet, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { return promiseReduce( groupedFieldSet, @@ -588,7 +578,6 @@ function executeFieldsSerially( fieldGroup, fieldPath, incrementalContext, - deferMap, ); if (result === undefined) { return graphqlWrappedResult; @@ -619,7 +608,6 @@ function executeFields( path: Path | undefined, groupedFieldSet: GroupedFieldSet, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const results = Object.create(null); const graphqlWrappedResult: GraphQLWrappedResult> = [ @@ -638,7 +626,6 @@ function executeFields( fieldGroup, fieldPath, incrementalContext, - deferMap, ); if (result !== undefined) { @@ -697,7 +684,6 @@ function executeField( fieldGroup: FieldGroup, path: Path, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue> | undefined { const fieldName = fieldGroup[0].node.name.value; const fieldDef = exeContext.schema.getField(parentType, fieldName); @@ -743,7 +729,6 @@ function executeField( path, result, incrementalContext, - deferMap, ); } @@ -755,7 +740,6 @@ function executeField( path, result, incrementalContext, - deferMap, ); if (isPromise(completed)) { @@ -870,7 +854,6 @@ function completeValue( path: Path, result: unknown, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue> { // If result is an Error, throw a located error. if (result instanceof Error) { @@ -888,7 +871,6 @@ function completeValue( path, result, incrementalContext, - deferMap, ); if ((completed as GraphQLWrappedResult)[0] === null) { throw new Error( @@ -913,7 +895,6 @@ function completeValue( path, result, incrementalContext, - deferMap, ); } @@ -934,7 +915,6 @@ function completeValue( path, result, incrementalContext, - deferMap, ); } @@ -948,7 +928,6 @@ function completeValue( path, result, incrementalContext, - deferMap, ); } /* c8 ignore next 6 */ @@ -967,7 +946,6 @@ async function completePromisedValue( path: Path, result: Promise, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): Promise> { try { const resolved = await result; @@ -979,7 +957,6 @@ async function completePromisedValue( path, resolved, incrementalContext, - deferMap, ); if (isPromise(completed)) { @@ -1057,6 +1034,7 @@ function getStreamUsage( const streamedFieldGroup: FieldGroup = fieldGroup.map((fieldDetails) => ({ node: fieldDetails.node, + fieldDepth: 0, deferUsage: undefined, })); @@ -1084,7 +1062,6 @@ async function completeAsyncIteratorValue( path: Path, asyncIterator: AsyncIterator, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): Promise>> { let containsPromise = false; const completedResults: Array = []; @@ -1165,7 +1142,6 @@ async function completeAsyncIteratorValue( info, itemPath, incrementalContext, - deferMap, ), ); containsPromise = true; @@ -1181,7 +1157,6 @@ async function completeAsyncIteratorValue( info, itemPath, incrementalContext, - deferMap, ) // TODO: add tests for stream backed by asyncIterator that completes to a promise /* c8 ignore start */ @@ -1221,7 +1196,6 @@ function completeListValue( path: Path, result: unknown, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const itemType = returnType.ofType; @@ -1236,7 +1210,6 @@ function completeListValue( path, asyncIterator, incrementalContext, - deferMap, ); } @@ -1254,7 +1227,6 @@ function completeListValue( path, result, incrementalContext, - deferMap, ); } @@ -1266,7 +1238,6 @@ function completeIterableValue( path: Path, items: Iterable, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): 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. @@ -1318,7 +1289,6 @@ function completeIterableValue( info, itemPath, incrementalContext, - deferMap, ), ); containsPromise = true; @@ -1333,7 +1303,6 @@ function completeIterableValue( info, itemPath, incrementalContext, - deferMap, ) ) { containsPromise = true; @@ -1366,7 +1335,6 @@ function completeListItemValue( info: GraphQLResolveInfo, itemPath: Path, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): boolean { try { const completedItem = completeValue( @@ -1377,7 +1345,6 @@ function completeListItemValue( itemPath, item, incrementalContext, - deferMap, ); if (isPromise(completedItem)) { @@ -1430,7 +1397,6 @@ async function completePromisedListItemValue( info: GraphQLResolveInfo, itemPath: Path, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): Promise { try { const resolved = await item; @@ -1442,7 +1408,6 @@ async function completePromisedListItemValue( itemPath, resolved, incrementalContext, - deferMap, ); if (isPromise(completed)) { completed = await completed; @@ -1492,7 +1457,6 @@ function completeAbstractValue( path: Path, result: unknown, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const resolveTypeFn = returnType.resolveType ?? exeContext.typeResolver; const contextValue = exeContext.contextValue; @@ -1515,7 +1479,6 @@ function completeAbstractValue( path, result, incrementalContext, - deferMap, ), ); } @@ -1535,7 +1498,6 @@ function completeAbstractValue( path, result, incrementalContext, - deferMap, ); } @@ -1605,7 +1567,6 @@ function completeObjectValue( path: Path, result: unknown, 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 @@ -1625,7 +1586,6 @@ function completeObjectValue( path, result, incrementalContext, - deferMap, ); }); } @@ -1642,7 +1602,6 @@ function completeObjectValue( path, result, incrementalContext, - deferMap, ); } @@ -1657,59 +1616,6 @@ function invalidReturnTypeError( ); } -/** - * Instantiates new DeferredFragmentRecords for the given path within an - * incremental data record, returning an updated map of DeferUsage - * objects to DeferredFragmentRecords. - * - * Note: As defer directives may be used with operations returning lists, - * a DeferUsage object may correspond to many DeferredFragmentRecords. - * - * DeferredFragmentRecord creation includes the following steps: - * 1. The new DeferredFragmentRecord is instantiated at the given path. - * 2. The parent result record is calculated from the given incremental data - * record. - * 3. The IncrementalPublisher is notified that a new DeferredFragmentRecord - * with the calculated parent has been added; the record will be released only - * after the parent has completed. - * - */ -function addNewDeferredFragments( - newDeferUsages: ReadonlyArray, - newDeferMap: Map, - path?: Path | undefined, -): ReadonlyMap { - // For each new deferUsage object: - for (const newDeferUsage of newDeferUsages) { - const parentDeferUsage = newDeferUsage.parentDeferUsage; - - const parent = - parentDeferUsage === undefined - ? undefined - : deferredFragmentRecordFromDeferUsage(parentDeferUsage, newDeferMap); - - // Instantiate the new record. - const deferredFragmentRecord = new DeferredFragmentRecord( - path, - newDeferUsage.label, - parent, - ); - - // Update the map. - newDeferMap.set(newDeferUsage, deferredFragmentRecord); - } - - return newDeferMap; -} - -function deferredFragmentRecordFromDeferUsage( - deferUsage: DeferUsage, - deferMap: ReadonlyMap, -): DeferredFragmentRecord { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return deferMap.get(deferUsage)!; -} - function collectAndExecuteSubfields( exeContext: ExecutionContext, returnType: GraphQLObjectType, @@ -1717,40 +1623,32 @@ function collectAndExecuteSubfields( path: Path, result: unknown, incrementalContext: IncrementalContext | undefined, - deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { // Collect sub-fields to execute to complete this value. - const collectedSubfields = collectSubfields( + const originalGroupedFieldSet = collectSubfields( exeContext, returnType, fieldGroup, ); - let groupedFieldSet = collectedSubfields.groupedFieldSet; - const newDeferUsages = collectedSubfields.newDeferUsages; - if (deferMap === undefined && newDeferUsages.length === 0) { + if ( + !exeContext.encounteredDefer && + originalGroupedFieldSet.encounteredDefer !== true + ) { return executeFields( exeContext, returnType, result, path, - groupedFieldSet, + originalGroupedFieldSet, incrementalContext, - undefined, ); } - const subFieldPlan = buildSubFieldPlan( - groupedFieldSet, + exeContext.encounteredDefer = true; + const { groupedFieldSet, newGroupedFieldSets } = buildSubFieldPlan( + originalGroupedFieldSet, incrementalContext?.deferUsageSet, ); - groupedFieldSet = subFieldPlan.groupedFieldSet; - const newGroupedFieldSets = subFieldPlan.newGroupedFieldSets; - const newDeferMap = addNewDeferredFragments( - newDeferUsages, - new Map(deferMap), - path, - ); - const subFields = executeFields( exeContext, returnType, @@ -1758,7 +1656,6 @@ function collectAndExecuteSubfields( path, groupedFieldSet, incrementalContext, - newDeferMap, ); if (newGroupedFieldSets.size > 0) { @@ -1769,7 +1666,6 @@ function collectAndExecuteSubfields( path, incrementalContext?.deferUsageSet, newGroupedFieldSets, - newDeferMap, ); return withNewDeferredGroupedFieldSets( @@ -2009,7 +1905,7 @@ function executeSubscription( ); } - const { groupedFieldSet } = collectFields( + const groupedFieldSet = collectFields( schema, fragments, variableValues, @@ -2095,19 +1991,14 @@ function executeDeferredGroupedFieldSets( path: Path | undefined, parentDeferUsages: DeferUsageSet | undefined, newGroupedFieldSets: Map, - deferMap: ReadonlyMap, ): ReadonlyArray { const newDeferredGroupedFieldSetRecords: Array = []; for (const [deferUsageSet, groupedFieldSet] of newGroupedFieldSets) { - const deferredFragmentRecords = getDeferredFragmentRecords( - deferUsageSet, - deferMap, - ); - const deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord = { - deferredFragmentRecords, + deferUsages: deferUsageSet, + path, result: undefined as unknown as BoxedPromiseOrValue, }; @@ -2124,7 +2015,6 @@ function executeDeferredGroupedFieldSets( errors: undefined, deferUsageSet, }, - deferMap, ); if (exeContext.enableEarlyExecution) { @@ -2168,7 +2058,6 @@ function executeDeferredGroupedFieldSet( path: Path | undefined, groupedFieldSet: GroupedFieldSet, incrementalContext: IncrementalContext, - deferMap: ReadonlyMap, ): PromiseOrValue { let result; try { @@ -2179,7 +2068,6 @@ function executeDeferredGroupedFieldSet( path, groupedFieldSet, incrementalContext, - deferMap, ); } catch (error) { return { @@ -2229,15 +2117,6 @@ function buildDeferredGroupedFieldSetResult( }; } -function getDeferredFragmentRecords( - deferUsages: DeferUsageSet, - deferMap: ReadonlyMap, -): ReadonlyArray { - return Array.from(deferUsages).map((deferUsage) => - deferredFragmentRecordFromDeferUsage(deferUsage, deferMap), - ); -} - function buildSyncStreamItemQueue( initialItem: PromiseOrValue, initialIndex: number, @@ -2427,7 +2306,6 @@ function completeStreamItem( itemPath, item, incrementalContext, - new Map(), ).then( (resolvedItem) => buildStreamItemResult(incrementalContext.errors, resolvedItem), @@ -2448,7 +2326,6 @@ function completeStreamItem( itemPath, item, incrementalContext, - new Map(), ); } catch (rawError) { handleFieldError( diff --git a/src/execution/types.ts b/src/execution/types.ts index c88ae9986e..467555ff82 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -7,6 +7,8 @@ import type { GraphQLFormattedError, } from '../error/GraphQLError.js'; +import type { DeferUsage } from './collectFields.js'; + /** * The result of GraphQL execution. * @@ -169,7 +171,7 @@ export interface FormattedCompletedResult { export function isDeferredGroupedFieldSetRecord( incrementalDataRecord: IncrementalDataRecord, ): incrementalDataRecord is DeferredGroupedFieldSetRecord { - return 'deferredFragmentRecords' in incrementalDataRecord; + return 'deferUsages' in incrementalDataRecord; } export type DeferredGroupedFieldSetResult = @@ -208,7 +210,8 @@ type ThunkIncrementalResult = | (() => BoxedPromiseOrValue); export interface DeferredGroupedFieldSetRecord { - deferredFragmentRecords: ReadonlyArray; + deferUsages: ReadonlySet; + path: Path | undefined; result: ThunkIncrementalResult; } diff --git a/src/jsutils/Path.ts b/src/jsutils/Path.ts index d223b6e752..f8200c6359 100644 --- a/src/jsutils/Path.ts +++ b/src/jsutils/Path.ts @@ -4,6 +4,7 @@ export interface Path { readonly prev: Path | undefined; readonly key: string | number; readonly typename: string | undefined; + readonly fieldDepth: number; } /** @@ -14,7 +15,12 @@ export function addPath( key: string | number, typename: string | undefined, ): Path { - return { prev, key, typename }; + const fieldDepth = prev + ? typeof key === 'number' + ? prev.fieldDepth + : prev.fieldDepth + 1 + : 1; + return { prev, key, typename, fieldDepth }; } /** @@ -31,3 +37,25 @@ export function pathToArray( } return flattened.reverse(); } + +export function pathAtFieldDepth( + path: Path | undefined, + fieldDepth: number, +): Path | undefined { + if (fieldDepth === 0) { + return undefined; + } + let currentPath = path; + while (currentPath !== undefined) { + if (currentPath.fieldDepth === fieldDepth) { + return currentPath; + } + currentPath = currentPath.prev; + } + /* c8 ignore next 5 */ + throw new Error( + `Path is of fieldDepth ${ + path === undefined ? 0 : path.fieldDepth + }, but fieldDepth ${fieldDepth} requested.`, + ); +} diff --git a/src/jsutils/__tests__/Path-test.ts b/src/jsutils/__tests__/Path-test.ts index 0484377db9..906e1b4f67 100644 --- a/src/jsutils/__tests__/Path-test.ts +++ b/src/jsutils/__tests__/Path-test.ts @@ -11,6 +11,7 @@ describe('Path', () => { prev: undefined, key: 1, typename: 'First', + fieldDepth: 1, }); }); @@ -22,6 +23,7 @@ describe('Path', () => { prev: first, key: 'two', typename: 'Second', + fieldDepth: 2, }); }); diff --git a/src/validation/rules/SingleFieldSubscriptionsRule.ts b/src/validation/rules/SingleFieldSubscriptionsRule.ts index 700bc0bda7..9707bfe8be 100644 --- a/src/validation/rules/SingleFieldSubscriptionsRule.ts +++ b/src/validation/rules/SingleFieldSubscriptionsRule.ts @@ -47,7 +47,7 @@ export function SingleFieldSubscriptionsRule( fragments[definition.name.value] = definition; } } - const { groupedFieldSet } = collectFields( + const groupedFieldSet = collectFields( schema, fragments, variableValues, From 1a95631db4e75287d8b70274bd2322a56899d687 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 22 Jul 2024 23:25:35 +0300 Subject: [PATCH 2/9] expose getDeferredFragmentRecord --- src/execution/IncrementalGraph.ts | 99 ++++++++++++++------------- src/execution/IncrementalPublisher.ts | 39 +++++------ 2 files changed, 72 insertions(+), 66 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index ac21be7d19..b1b706d370 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -65,7 +65,7 @@ export class IncrementalGraph { addCompletedReconcilableDeferredGroupedFieldSet( reconcilableResult: ReconcilableDeferredGroupedFieldSetResult, ): void { - const deferredFragmentRecords = this.getDeferredFragmentRecords( + const deferredFragmentRecords = this._getDeferredFragmentRecords( reconcilableResult.deferredGroupedFieldSetRecord, ); for (const deferredFragmentRecord of deferredFragmentRecords) { @@ -84,14 +84,46 @@ export class IncrementalGraph { } } - getDeferredFragmentRecords( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, - ): Array { - const { deferUsages, path } = deferredGroupedFieldSetRecord; - return Array.from(deferUsages).map((deferUsage) => { - const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); - return this._getDeferredFragmentRecord(deferUsage, deferUsagePath); - }); + getDeferredFragmentRecord( + deferUsage: DeferUsage, + path: Path | undefined, + ): DeferredFragmentRecord { + const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); + let deferredFragmentRecords: + | WeakMap + | undefined; + if (deferUsagePath === undefined) { + deferredFragmentRecords = this._rootDeferredFragments; + } else { + deferredFragmentRecords = + this._nonRootDeferredFragments.get(deferUsagePath); + if (deferredFragmentRecords === undefined) { + deferredFragmentRecords = new WeakMap(); + this._nonRootDeferredFragments.set( + deferUsagePath, + deferredFragmentRecords, + ); + } + } + let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); + if (deferredFragmentRecord === undefined) { + let parent: DeferredFragmentRecord | undefined; + const parentDeferUsage = deferUsage.parentDeferUsage; + if (parentDeferUsage !== undefined) { + const parentPath = pathAtFieldDepth( + deferUsagePath, + parentDeferUsage.fieldDepth, + ); + parent = this.getDeferredFragmentRecord(parentDeferUsage, parentPath); + } + deferredFragmentRecord = new DeferredFragmentRecord( + deferUsagePath, + deferUsage.label, + parent, + ); + deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); + } + return deferredFragmentRecord; } *currentCompletedBatch(): Generator { @@ -143,7 +175,7 @@ export class IncrementalGraph { ); this._removeRootNode(deferredFragmentRecord); for (const reconcilableResult of reconcilableResults) { - const deferredFragmentRecords = this.getDeferredFragmentRecords( + const deferredFragmentRecords = this._getDeferredFragmentRecords( reconcilableResult.deferredGroupedFieldSetRecord, ); for (const otherDeferredFragmentRecord of deferredFragmentRecords) { @@ -172,40 +204,6 @@ export class IncrementalGraph { this._removeRootNode(streamRecord); } - private _getDeferredFragmentRecord( - deferUsage: DeferUsage, - path: Path | undefined, - ): DeferredFragmentRecord { - let deferredFragmentRecords: - | WeakMap - | undefined; - if (path === undefined) { - deferredFragmentRecords = this._rootDeferredFragments; - } else { - deferredFragmentRecords = this._nonRootDeferredFragments.get(path); - if (deferredFragmentRecords === undefined) { - deferredFragmentRecords = new WeakMap(); - this._nonRootDeferredFragments.set(path, deferredFragmentRecords); - } - } - let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); - if (deferredFragmentRecord === undefined) { - let parent: DeferredFragmentRecord | undefined; - const parentDeferUsage = deferUsage.parentDeferUsage; - if (parentDeferUsage !== undefined) { - const parentPath = pathAtFieldDepth(path, parentDeferUsage.fieldDepth); - parent = this._getDeferredFragmentRecord(parentDeferUsage, parentPath); - } - deferredFragmentRecord = new DeferredFragmentRecord( - path, - deferUsage.label, - parent, - ); - deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); - } - return deferredFragmentRecord; - } - private _removeRootNode( subsequentResultRecord: SubsequentResultRecord, ): void { @@ -219,7 +217,7 @@ export class IncrementalGraph { ): void { for (const incrementalDataRecord of incrementalDataRecords) { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { - const deferredFragmentRecords = this.getDeferredFragmentRecords( + const deferredFragmentRecords = this._getDeferredFragmentRecords( incrementalDataRecord, ); for (const deferredFragmentRecord of deferredFragmentRecords) { @@ -246,6 +244,15 @@ export class IncrementalGraph { } } + private _getDeferredFragmentRecords( + deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, + ): Array { + const { deferUsages, path } = deferredGroupedFieldSetRecord; + return Array.from(deferUsages).map((deferUsage) => + this.getDeferredFragmentRecord(deferUsage, path), + ); + } + private _promoteNonEmptyToRoot( maybeEmptyNewRootNodes: Set, ): ReadonlyArray { @@ -279,7 +286,7 @@ export class IncrementalGraph { private _completesRootNode( deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, ): boolean { - const deferredFragmentRecords = this.getDeferredFragmentRecords( + const deferredFragmentRecords = this._getDeferredFragmentRecords( deferredGroupedFieldSetRecord, ); return deferredFragmentRecords.some((deferredFragmentRecord) => diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 3e70b278fd..753cf87900 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -4,11 +4,11 @@ import { pathToArray } from '../jsutils/Path.js'; import type { GraphQLError } from '../error/GraphQLError.js'; +import type { DeferUsage } from './collectFields.js'; import { IncrementalGraph } from './IncrementalGraph.js'; import type { CancellableStreamRecord, CompletedResult, - DeferredFragmentRecord, DeferredGroupedFieldSetResult, ExperimentalIncrementalExecutionResults, IncrementalDataRecord, @@ -221,16 +221,16 @@ class IncrementalPublisher { deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, context: SubsequentIncrementalExecutionResultContext, ): void { + const { deferUsages, path } = + deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord; if ( isNonReconcilableDeferredGroupedFieldSetResult( deferredGroupedFieldSetResult, ) ) { - const deferredFragmentRecords = - this._incrementalGraph.getDeferredFragmentRecords( - deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord, - ); - for (const deferredFragmentRecord of deferredFragmentRecords) { + for (const deferUsage of deferUsages) { + const deferredFragmentRecord = + this._incrementalGraph.getDeferredFragmentRecord(deferUsage, path); const id = deferredFragmentRecord.id; if ( !this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord) @@ -251,11 +251,9 @@ class IncrementalPublisher { deferredGroupedFieldSetResult, ); - const deferredFragmentRecords = - this._incrementalGraph.getDeferredFragmentRecords( - deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord, - ); - for (const deferredFragmentRecord of deferredFragmentRecords) { + for (const deferUsage of deferUsages) { + const deferredFragmentRecord = + this._incrementalGraph.getDeferredFragmentRecord(deferUsage, path); const completion = this._incrementalGraph.completeDeferredFragment( deferredFragmentRecord, ); @@ -270,7 +268,7 @@ class IncrementalPublisher { for (const reconcilableResult of reconcilableResults) { const { bestId, subPath } = this._getBestIdAndSubPath( id, - deferredFragmentRecord, + deferUsage, reconcilableResult, ); const incrementalEntry: IncrementalDeferResult = { @@ -334,17 +332,18 @@ class IncrementalPublisher { private _getBestIdAndSubPath( initialId: string, - initialDeferredFragmentRecord: DeferredFragmentRecord, + initialDeferUsage: DeferUsage, deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, ): { bestId: string; subPath: ReadonlyArray | undefined } { + const { deferUsages, path } = + deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord; + const initialDeferredFragmentRecord = + this._incrementalGraph.getDeferredFragmentRecord(initialDeferUsage, path); let maxLength = pathToArray(initialDeferredFragmentRecord.path).length; let bestId = initialId; - - const deferredFragmentRecords = - this._incrementalGraph.getDeferredFragmentRecords( - deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord, - ); - for (const deferredFragmentRecord of deferredFragmentRecords) { + for (const deferUsage of deferUsages) { + const deferredFragmentRecord = + this._incrementalGraph.getDeferredFragmentRecord(deferUsage, path); if (deferredFragmentRecord === initialDeferredFragmentRecord) { continue; } @@ -361,7 +360,7 @@ class IncrementalPublisher { bestId = id; } } - const subPath = deferredGroupedFieldSetResult.path.slice(maxLength); + const subPath = pathToArray(path).slice(maxLength); return { bestId, subPath: subPath.length > 0 ? subPath : undefined, From da66403ec8412b2af14a654e7ae85dbf9cd37c0c Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 22 Jul 2024 23:31:45 +0300 Subject: [PATCH 3/9] remove helper it causes extra looping --- src/execution/IncrementalGraph.ts | 51 +++++++++++++++---------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index b1b706d370..91914497ff 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -65,10 +65,15 @@ export class IncrementalGraph { addCompletedReconcilableDeferredGroupedFieldSet( reconcilableResult: ReconcilableDeferredGroupedFieldSetResult, ): void { - const deferredFragmentRecords = this._getDeferredFragmentRecords( - reconcilableResult.deferredGroupedFieldSetRecord, - ); - for (const deferredFragmentRecord of deferredFragmentRecords) { + const { deferUsages, path } = + reconcilableResult.deferredGroupedFieldSetRecord; + const deferredFragmentRecords: Array = []; + for (const deferUsage of deferUsages) { + const deferredFragmentRecord = this.getDeferredFragmentRecord( + deferUsage, + path, + ); + deferredFragmentRecords.push(deferredFragmentRecord); deferredFragmentRecord.deferredGroupedFieldSetRecords.delete( reconcilableResult.deferredGroupedFieldSetRecord, ); @@ -175,10 +180,13 @@ export class IncrementalGraph { ); this._removeRootNode(deferredFragmentRecord); for (const reconcilableResult of reconcilableResults) { - const deferredFragmentRecords = this._getDeferredFragmentRecords( - reconcilableResult.deferredGroupedFieldSetRecord, - ); - for (const otherDeferredFragmentRecord of deferredFragmentRecords) { + const { deferUsages, path } = + reconcilableResult.deferredGroupedFieldSetRecord; + for (const otherDeferUsage of deferUsages) { + const otherDeferredFragmentRecord = this.getDeferredFragmentRecord( + otherDeferUsage, + path, + ); otherDeferredFragmentRecord.reconcilableResults.delete( reconcilableResult, ); @@ -217,10 +225,12 @@ export class IncrementalGraph { ): void { for (const incrementalDataRecord of incrementalDataRecords) { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { - const deferredFragmentRecords = this._getDeferredFragmentRecords( - incrementalDataRecord, - ); - for (const deferredFragmentRecord of deferredFragmentRecords) { + const { deferUsages, path } = incrementalDataRecord; + for (const deferUsage of deferUsages) { + const deferredFragmentRecord = this.getDeferredFragmentRecord( + deferUsage, + path, + ); this._addDeferredFragment( deferredFragmentRecord, initialResultChildren, @@ -244,15 +254,6 @@ export class IncrementalGraph { } } - private _getDeferredFragmentRecords( - deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, - ): Array { - const { deferUsages, path } = deferredGroupedFieldSetRecord; - return Array.from(deferUsages).map((deferUsage) => - this.getDeferredFragmentRecord(deferUsage, path), - ); - } - private _promoteNonEmptyToRoot( maybeEmptyNewRootNodes: Set, ): ReadonlyArray { @@ -286,11 +287,9 @@ export class IncrementalGraph { private _completesRootNode( deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, ): boolean { - const deferredFragmentRecords = this._getDeferredFragmentRecords( - deferredGroupedFieldSetRecord, - ); - return deferredFragmentRecords.some((deferredFragmentRecord) => - this._rootNodes.has(deferredFragmentRecord), + const { deferUsages, path } = deferredGroupedFieldSetRecord; + return Array.from(deferUsages).some((deferUsage) => + this._rootNodes.has(this.getDeferredFragmentRecord(deferUsage, path)), ); } From 7a87b49244714029d7665783466e0202f5b047a3 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 23 Jul 2024 10:47:05 +0300 Subject: [PATCH 4/9] make getDeferredFragmentRecord private --- src/execution/IncrementalGraph.ts | 149 +++++++++++++++++--------- src/execution/IncrementalPublisher.ts | 62 ++--------- 2 files changed, 109 insertions(+), 102 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 91914497ff..9f96c3ecd0 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -2,7 +2,7 @@ import { BoxedPromiseOrValue } from '../jsutils/BoxedPromiseOrValue.js'; import { invariant } from '../jsutils/invariant.js'; import { isPromise } from '../jsutils/isPromise.js'; import type { Path } from '../jsutils/Path.js'; -import { pathAtFieldDepth } from '../jsutils/Path.js'; +import { pathAtFieldDepth, pathToArray } from '../jsutils/Path.js'; import { promiseWithResolvers } from '../jsutils/promiseWithResolvers.js'; import type { GraphQLError } from '../error/GraphQLError.js'; @@ -10,6 +10,7 @@ import type { GraphQLError } from '../error/GraphQLError.js'; import type { DeferUsage } from './collectFields.js'; import type { DeferredGroupedFieldSetRecord, + DeferredGroupedFieldSetResult, IncrementalDataRecord, IncrementalDataRecordResult, ReconcilableDeferredGroupedFieldSetResult, @@ -69,7 +70,7 @@ export class IncrementalGraph { reconcilableResult.deferredGroupedFieldSetRecord; const deferredFragmentRecords: Array = []; for (const deferUsage of deferUsages) { - const deferredFragmentRecord = this.getDeferredFragmentRecord( + const deferredFragmentRecord = this._getDeferredFragmentRecord( deferUsage, path, ); @@ -89,46 +90,37 @@ export class IncrementalGraph { } } - getDeferredFragmentRecord( - deferUsage: DeferUsage, - path: Path | undefined, - ): DeferredFragmentRecord { - const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); - let deferredFragmentRecords: - | WeakMap - | undefined; - if (deferUsagePath === undefined) { - deferredFragmentRecords = this._rootDeferredFragments; - } else { - deferredFragmentRecords = - this._nonRootDeferredFragments.get(deferUsagePath); - if (deferredFragmentRecords === undefined) { - deferredFragmentRecords = new WeakMap(); - this._nonRootDeferredFragments.set( - deferUsagePath, - deferredFragmentRecords, - ); + getBestIdAndSubPath( + initialDeferUsage: DeferUsage, + deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, + ): { bestId: string; subPath: ReadonlyArray | undefined } { + const { deferUsages, path } = + deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord; + let bestDeferUsage = initialDeferUsage; + let maxFieldDepth = initialDeferUsage.fieldDepth; + for (const deferUsage of deferUsages) { + if (deferUsage === initialDeferUsage) { + continue; } - } - let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); - if (deferredFragmentRecord === undefined) { - let parent: DeferredFragmentRecord | undefined; - const parentDeferUsage = deferUsage.parentDeferUsage; - if (parentDeferUsage !== undefined) { - const parentPath = pathAtFieldDepth( - deferUsagePath, - parentDeferUsage.fieldDepth, - ); - parent = this.getDeferredFragmentRecord(parentDeferUsage, parentPath); + const fieldDepth = deferUsage.fieldDepth; + if (fieldDepth > maxFieldDepth) { + maxFieldDepth = fieldDepth; + bestDeferUsage = deferUsage; } - deferredFragmentRecord = new DeferredFragmentRecord( - deferUsagePath, - deferUsage.label, - parent, - ); - deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); } - return deferredFragmentRecord; + const deferredFragmentRecord = this._getDeferredFragmentRecord( + bestDeferUsage, + path, + ); + const bestId = deferredFragmentRecord.id; + invariant(bestId !== undefined); + const subPath = pathToArray(path).slice( + pathToArray(deferredFragmentRecord.path).length, + ); + return { + bestId, + subPath: subPath.length > 0 ? subPath : undefined, + }; } *currentCompletedBatch(): Generator { @@ -163,12 +155,20 @@ export class IncrementalGraph { return this._rootNodes.size > 0; } - completeDeferredFragment(deferredFragmentRecord: DeferredFragmentRecord): + completeDeferredFragment( + deferUsage: DeferUsage, + path: Path | undefined, + ): | { + id: string; newRootNodes: ReadonlyArray; reconcilableResults: ReadonlyArray; } | undefined { + const deferredFragmentRecord = this._getDeferredFragmentRecord( + deferUsage, + path, + ); if ( !this._rootNodes.has(deferredFragmentRecord) || deferredFragmentRecord.deferredGroupedFieldSetRecords.size > 0 @@ -180,12 +180,12 @@ export class IncrementalGraph { ); this._removeRootNode(deferredFragmentRecord); for (const reconcilableResult of reconcilableResults) { - const { deferUsages, path } = + const { deferUsages, path: resultPath } = reconcilableResult.deferredGroupedFieldSetRecord; for (const otherDeferUsage of deferUsages) { - const otherDeferredFragmentRecord = this.getDeferredFragmentRecord( + const otherDeferredFragmentRecord = this._getDeferredFragmentRecord( otherDeferUsage, - path, + resultPath, ); otherDeferredFragmentRecord.reconcilableResults.delete( reconcilableResult, @@ -195,17 +195,24 @@ export class IncrementalGraph { const newRootNodes = this._promoteNonEmptyToRoot( deferredFragmentRecord.children, ); - return { newRootNodes, reconcilableResults }; + const id = deferredFragmentRecord.id; + invariant(id !== undefined); + return { id, newRootNodes, reconcilableResults }; } removeDeferredFragment( - deferredFragmentRecord: DeferredFragmentRecord, - ): boolean { + deferUsage: DeferUsage, + path: Path | undefined, + ): string | undefined { + const deferredFragmentRecord = this._getDeferredFragmentRecord( + deferUsage, + path, + ); if (!this._rootNodes.has(deferredFragmentRecord)) { - return false; + return; } this._removeRootNode(deferredFragmentRecord); - return true; + return deferredFragmentRecord.id; } removeStream(streamRecord: StreamRecord): void { @@ -227,7 +234,7 @@ export class IncrementalGraph { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { const { deferUsages, path } = incrementalDataRecord; for (const deferUsage of deferUsages) { - const deferredFragmentRecord = this.getDeferredFragmentRecord( + const deferredFragmentRecord = this._getDeferredFragmentRecord( deferUsage, path, ); @@ -254,6 +261,48 @@ export class IncrementalGraph { } } + private _getDeferredFragmentRecord( + deferUsage: DeferUsage, + path: Path | undefined, + ): DeferredFragmentRecord { + const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); + let deferredFragmentRecords: + | WeakMap + | undefined; + if (deferUsagePath === undefined) { + deferredFragmentRecords = this._rootDeferredFragments; + } else { + deferredFragmentRecords = + this._nonRootDeferredFragments.get(deferUsagePath); + if (deferredFragmentRecords === undefined) { + deferredFragmentRecords = new WeakMap(); + this._nonRootDeferredFragments.set( + deferUsagePath, + deferredFragmentRecords, + ); + } + } + let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); + if (deferredFragmentRecord === undefined) { + let parent: DeferredFragmentRecord | undefined; + const parentDeferUsage = deferUsage.parentDeferUsage; + if (parentDeferUsage !== undefined) { + const parentPath = pathAtFieldDepth( + deferUsagePath, + parentDeferUsage.fieldDepth, + ); + parent = this._getDeferredFragmentRecord(parentDeferUsage, parentPath); + } + deferredFragmentRecord = new DeferredFragmentRecord( + deferUsagePath, + deferUsage.label, + parent, + ); + deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); + } + return deferredFragmentRecord; + } + private _promoteNonEmptyToRoot( maybeEmptyNewRootNodes: Set, ): ReadonlyArray { @@ -289,7 +338,7 @@ export class IncrementalGraph { ): boolean { const { deferUsages, path } = deferredGroupedFieldSetRecord; return Array.from(deferUsages).some((deferUsage) => - this._rootNodes.has(this.getDeferredFragmentRecord(deferUsage, path)), + this._rootNodes.has(this._getDeferredFragmentRecord(deferUsage, path)), ); } diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 753cf87900..ebd6839586 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -4,7 +4,6 @@ import { pathToArray } from '../jsutils/Path.js'; import type { GraphQLError } from '../error/GraphQLError.js'; -import type { DeferUsage } from './collectFields.js'; import { IncrementalGraph } from './IncrementalGraph.js'; import type { CancellableStreamRecord, @@ -229,16 +228,14 @@ class IncrementalPublisher { ) ) { for (const deferUsage of deferUsages) { - const deferredFragmentRecord = - this._incrementalGraph.getDeferredFragmentRecord(deferUsage, path); - const id = deferredFragmentRecord.id; - if ( - !this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord) - ) { + const id = this._incrementalGraph.removeDeferredFragment( + deferUsage, + path, + ); + if (id === undefined) { // This can occur if multiple deferred grouped field sets error for a fragment. continue; } - invariant(id !== undefined); context.completed.push({ id, errors: deferredGroupedFieldSetResult.errors, @@ -252,22 +249,18 @@ class IncrementalPublisher { ); for (const deferUsage of deferUsages) { - const deferredFragmentRecord = - this._incrementalGraph.getDeferredFragmentRecord(deferUsage, path); const completion = this._incrementalGraph.completeDeferredFragment( - deferredFragmentRecord, + deferUsage, + path, ); if (completion === undefined) { continue; } - const id = deferredFragmentRecord.id; - invariant(id !== undefined); const incremental = context.incremental; const { newRootNodes, reconcilableResults } = completion; context.pending.push(...this._toPendingResults(newRootNodes)); for (const reconcilableResult of reconcilableResults) { - const { bestId, subPath } = this._getBestIdAndSubPath( - id, + const { bestId, subPath } = this._incrementalGraph.getBestIdAndSubPath( deferUsage, reconcilableResult, ); @@ -280,6 +273,8 @@ class IncrementalPublisher { } incremental.push(incrementalEntry); } + const id = completion.id; + invariant(id !== undefined); context.completed.push({ id }); } } @@ -330,43 +325,6 @@ class IncrementalPublisher { } } - private _getBestIdAndSubPath( - initialId: string, - initialDeferUsage: DeferUsage, - deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, - ): { bestId: string; subPath: ReadonlyArray | undefined } { - const { deferUsages, path } = - deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord; - const initialDeferredFragmentRecord = - this._incrementalGraph.getDeferredFragmentRecord(initialDeferUsage, path); - let maxLength = pathToArray(initialDeferredFragmentRecord.path).length; - let bestId = initialId; - for (const deferUsage of deferUsages) { - const deferredFragmentRecord = - this._incrementalGraph.getDeferredFragmentRecord(deferUsage, path); - if (deferredFragmentRecord === initialDeferredFragmentRecord) { - continue; - } - 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; - } - const fragmentPath = pathToArray(deferredFragmentRecord.path); - const length = fragmentPath.length; - if (length > maxLength) { - maxLength = length; - bestId = id; - } - } - const subPath = pathToArray(path).slice(maxLength); - return { - bestId, - subPath: subPath.length > 0 ? subPath : undefined, - }; - } - private async _returnAsyncIterators(): Promise { const cancellableStreams = this._context.cancellableStreams; if (cancellableStreams === undefined) { From c370186153482c7a57ed20cc099d55f1960dfbfd Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 23 Jul 2024 10:50:05 +0300 Subject: [PATCH 5/9] remove need for Array.from which causes double-looping --- src/execution/IncrementalGraph.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 9f96c3ecd0..c79de35d45 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -337,9 +337,16 @@ export class IncrementalGraph { deferredGroupedFieldSetRecord: DeferredGroupedFieldSetRecord, ): boolean { const { deferUsages, path } = deferredGroupedFieldSetRecord; - return Array.from(deferUsages).some((deferUsage) => - this._rootNodes.has(this._getDeferredFragmentRecord(deferUsage, path)), - ); + for (const deferUsage of deferUsages) { + const deferredFragmentRecord = this._getDeferredFragmentRecord( + deferUsage, + path, + ); + if (this._rootNodes.has(deferredFragmentRecord)) { + return true; + } + } + return false; } private _addDeferredFragment( From cc5acf9c21b615542b34882706db8c09df586d86 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 23 Jul 2024 11:26:37 +0300 Subject: [PATCH 6/9] introduce factory get rid of WeakMap --- src/execution/IncrementalGraph.ts | 115 +++++++++++++------------- src/execution/IncrementalPublisher.ts | 4 +- src/execution/types.ts | 6 +- 3 files changed, 60 insertions(+), 65 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index c79de35d45..09d5dc863a 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -27,25 +27,60 @@ import { /** * @internal */ -export class IncrementalGraph { - private _rootNodes: Set; - - private _rootDeferredFragments = new WeakMap< +class DeferredFragmentFactory { + private _rootDeferredFragments = new Map< DeferUsage, DeferredFragmentRecord >(); - private _nonRootDeferredFragments = new WeakMap< - Path, - WeakMap - >(); + get(deferUsage: DeferUsage, path: Path | undefined): DeferredFragmentRecord { + const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); + let deferredFragmentRecords: + | Map + | undefined; + if (deferUsagePath === undefined) { + deferredFragmentRecords = this._rootDeferredFragments; + } else { + deferredFragmentRecords = ( + deferUsagePath as unknown as { + deferredFragmentRecords: Map; + } + ).deferredFragmentRecords; + if (deferredFragmentRecords === undefined) { + deferredFragmentRecords = new Map(); + ( + deferUsagePath as unknown as { + deferredFragmentRecords: Map; + } + ).deferredFragmentRecords = deferredFragmentRecords; + } + } + let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); + if (deferredFragmentRecord === undefined) { + deferredFragmentRecord = new DeferredFragmentRecord( + deferUsage, + deferUsagePath, + deferUsage.label, + ); + deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); + } + return deferredFragmentRecord; + } +} +/** + * @internal + */ +export class IncrementalGraph { + private _rootNodes: Set; + private _deferredFragmentFactory: DeferredFragmentFactory; private _completedQueue: Array; private _nextQueue: Array< (iterable: Iterable | undefined) => void >; constructor() { + this._deferredFragmentFactory = new DeferredFragmentFactory(); this._rootNodes = new Set(); this._completedQueue = []; this._nextQueue = []; @@ -70,7 +105,7 @@ export class IncrementalGraph { reconcilableResult.deferredGroupedFieldSetRecord; const deferredFragmentRecords: Array = []; for (const deferUsage of deferUsages) { - const deferredFragmentRecord = this._getDeferredFragmentRecord( + const deferredFragmentRecord = this._deferredFragmentFactory.get( deferUsage, path, ); @@ -108,7 +143,7 @@ export class IncrementalGraph { bestDeferUsage = deferUsage; } } - const deferredFragmentRecord = this._getDeferredFragmentRecord( + const deferredFragmentRecord = this._deferredFragmentFactory.get( bestDeferUsage, path, ); @@ -165,7 +200,7 @@ export class IncrementalGraph { reconcilableResults: ReadonlyArray; } | undefined { - const deferredFragmentRecord = this._getDeferredFragmentRecord( + const deferredFragmentRecord = this._deferredFragmentFactory.get( deferUsage, path, ); @@ -183,7 +218,7 @@ export class IncrementalGraph { const { deferUsages, path: resultPath } = reconcilableResult.deferredGroupedFieldSetRecord; for (const otherDeferUsage of deferUsages) { - const otherDeferredFragmentRecord = this._getDeferredFragmentRecord( + const otherDeferredFragmentRecord = this._deferredFragmentFactory.get( otherDeferUsage, resultPath, ); @@ -204,7 +239,7 @@ export class IncrementalGraph { deferUsage: DeferUsage, path: Path | undefined, ): string | undefined { - const deferredFragmentRecord = this._getDeferredFragmentRecord( + const deferredFragmentRecord = this._deferredFragmentFactory.get( deferUsage, path, ); @@ -234,7 +269,7 @@ export class IncrementalGraph { if (isDeferredGroupedFieldSetRecord(incrementalDataRecord)) { const { deferUsages, path } = incrementalDataRecord; for (const deferUsage of deferUsages) { - const deferredFragmentRecord = this._getDeferredFragmentRecord( + const deferredFragmentRecord = this._deferredFragmentFactory.get( deferUsage, path, ); @@ -261,48 +296,6 @@ export class IncrementalGraph { } } - private _getDeferredFragmentRecord( - deferUsage: DeferUsage, - path: Path | undefined, - ): DeferredFragmentRecord { - const deferUsagePath = pathAtFieldDepth(path, deferUsage.fieldDepth); - let deferredFragmentRecords: - | WeakMap - | undefined; - if (deferUsagePath === undefined) { - deferredFragmentRecords = this._rootDeferredFragments; - } else { - deferredFragmentRecords = - this._nonRootDeferredFragments.get(deferUsagePath); - if (deferredFragmentRecords === undefined) { - deferredFragmentRecords = new WeakMap(); - this._nonRootDeferredFragments.set( - deferUsagePath, - deferredFragmentRecords, - ); - } - } - let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); - if (deferredFragmentRecord === undefined) { - let parent: DeferredFragmentRecord | undefined; - const parentDeferUsage = deferUsage.parentDeferUsage; - if (parentDeferUsage !== undefined) { - const parentPath = pathAtFieldDepth( - deferUsagePath, - parentDeferUsage.fieldDepth, - ); - parent = this._getDeferredFragmentRecord(parentDeferUsage, parentPath); - } - deferredFragmentRecord = new DeferredFragmentRecord( - deferUsagePath, - deferUsage.label, - parent, - ); - deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); - } - return deferredFragmentRecord; - } - private _promoteNonEmptyToRoot( maybeEmptyNewRootNodes: Set, ): ReadonlyArray { @@ -338,7 +331,7 @@ export class IncrementalGraph { ): boolean { const { deferUsages, path } = deferredGroupedFieldSetRecord; for (const deferUsage of deferUsages) { - const deferredFragmentRecord = this._getDeferredFragmentRecord( + const deferredFragmentRecord = this._deferredFragmentFactory.get( deferUsage, path, ); @@ -356,12 +349,16 @@ export class IncrementalGraph { if (this._rootNodes.has(deferredFragmentRecord)) { return; } - const parent = deferredFragmentRecord.parent; - if (parent === undefined) { + const parentDeferUsage = deferredFragmentRecord.deferUsage.parentDeferUsage; + if (parentDeferUsage === undefined) { invariant(initialResultChildren !== undefined); initialResultChildren.add(deferredFragmentRecord); return; } + const parent = this._deferredFragmentFactory.get( + parentDeferUsage, + deferredFragmentRecord.path, + ); parent.children.add(deferredFragmentRecord); this._addDeferredFragment(parent, initialResultChildren); } diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index ebd6839586..def58981c8 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -257,7 +257,7 @@ class IncrementalPublisher { continue; } const incremental = context.incremental; - const { newRootNodes, reconcilableResults } = completion; + const { id, newRootNodes, reconcilableResults } = completion; context.pending.push(...this._toPendingResults(newRootNodes)); for (const reconcilableResult of reconcilableResults) { const { bestId, subPath } = this._incrementalGraph.getBestIdAndSubPath( @@ -273,8 +273,6 @@ class IncrementalPublisher { } incremental.push(incrementalEntry); } - const id = completion.id; - invariant(id !== undefined); context.completed.push({ id }); } } diff --git a/src/execution/types.ts b/src/execution/types.ts index 467555ff82..54378fc4c9 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -219,22 +219,22 @@ export type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord; /** @internal */ export class DeferredFragmentRecord { + deferUsage: DeferUsage; path: Path | undefined; label: string | undefined; id?: string | undefined; - parent: DeferredFragmentRecord | undefined; deferredGroupedFieldSetRecords: Set; reconcilableResults: Set; children: Set; constructor( + deferUsage: DeferUsage, path: Path | undefined, label: string | undefined, - parent: DeferredFragmentRecord | undefined, ) { + this.deferUsage = deferUsage; this.path = path; this.label = label; - this.parent = parent; this.deferredGroupedFieldSetRecords = new Set(); this.reconcilableResults = new Set(); this.children = new Set(); From 87074463818129d24f403c15d9f8cf9b01f7d9a9 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 23 Jul 2024 17:50:25 +0300 Subject: [PATCH 7/9] IncrementalGraph knows not of id --- src/execution/IncrementalGraph.ts | 36 ++++++++------------------- src/execution/IncrementalPublisher.ts | 34 +++++++++++++++++-------- 2 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 09d5dc863a..779e38efa4 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -2,7 +2,7 @@ import { BoxedPromiseOrValue } from '../jsutils/BoxedPromiseOrValue.js'; import { invariant } from '../jsutils/invariant.js'; import { isPromise } from '../jsutils/isPromise.js'; import type { Path } from '../jsutils/Path.js'; -import { pathAtFieldDepth, pathToArray } from '../jsutils/Path.js'; +import { pathAtFieldDepth } from '../jsutils/Path.js'; import { promiseWithResolvers } from '../jsutils/promiseWithResolvers.js'; import type { GraphQLError } from '../error/GraphQLError.js'; @@ -10,7 +10,6 @@ import type { GraphQLError } from '../error/GraphQLError.js'; import type { DeferUsage } from './collectFields.js'; import type { DeferredGroupedFieldSetRecord, - DeferredGroupedFieldSetResult, IncrementalDataRecord, IncrementalDataRecordResult, ReconcilableDeferredGroupedFieldSetResult, @@ -125,12 +124,11 @@ export class IncrementalGraph { } } - getBestIdAndSubPath( + getDeepestDeferredFragmentRecord( initialDeferUsage: DeferUsage, - deferredGroupedFieldSetResult: DeferredGroupedFieldSetResult, - ): { bestId: string; subPath: ReadonlyArray | undefined } { - const { deferUsages, path } = - deferredGroupedFieldSetResult.deferredGroupedFieldSetRecord; + deferUsages: ReadonlySet, + path: Path | undefined, + ): DeferredFragmentRecord { let bestDeferUsage = initialDeferUsage; let maxFieldDepth = initialDeferUsage.fieldDepth; for (const deferUsage of deferUsages) { @@ -143,19 +141,7 @@ export class IncrementalGraph { bestDeferUsage = deferUsage; } } - const deferredFragmentRecord = this._deferredFragmentFactory.get( - bestDeferUsage, - path, - ); - const bestId = deferredFragmentRecord.id; - invariant(bestId !== undefined); - const subPath = pathToArray(path).slice( - pathToArray(deferredFragmentRecord.path).length, - ); - return { - bestId, - subPath: subPath.length > 0 ? subPath : undefined, - }; + return this._deferredFragmentFactory.get(bestDeferUsage, path); } *currentCompletedBatch(): Generator { @@ -195,7 +181,7 @@ export class IncrementalGraph { path: Path | undefined, ): | { - id: string; + deferredFragmentRecord: DeferredFragmentRecord; newRootNodes: ReadonlyArray; reconcilableResults: ReadonlyArray; } @@ -230,15 +216,13 @@ export class IncrementalGraph { const newRootNodes = this._promoteNonEmptyToRoot( deferredFragmentRecord.children, ); - const id = deferredFragmentRecord.id; - invariant(id !== undefined); - return { id, newRootNodes, reconcilableResults }; + return { deferredFragmentRecord, newRootNodes, reconcilableResults }; } removeDeferredFragment( deferUsage: DeferUsage, path: Path | undefined, - ): string | undefined { + ): DeferredFragmentRecord | undefined { const deferredFragmentRecord = this._deferredFragmentFactory.get( deferUsage, path, @@ -247,7 +231,7 @@ export class IncrementalGraph { return; } this._removeRootNode(deferredFragmentRecord); - return deferredFragmentRecord.id; + return deferredFragmentRecord; } removeStream(streamRecord: StreamRecord): void { diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index def58981c8..ec7cde4789 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -228,14 +228,14 @@ class IncrementalPublisher { ) ) { for (const deferUsage of deferUsages) { - const id = this._incrementalGraph.removeDeferredFragment( - deferUsage, - path, - ); - if (id === undefined) { + const deferredFragmentRecord = + this._incrementalGraph.removeDeferredFragment(deferUsage, path); + if (deferredFragmentRecord === undefined) { // This can occur if multiple deferred grouped field sets error for a fragment. continue; } + const id = deferredFragmentRecord.id; + invariant(id !== undefined); context.completed.push({ id, errors: deferredGroupedFieldSetResult.errors, @@ -257,18 +257,30 @@ class IncrementalPublisher { continue; } const incremental = context.incremental; - const { id, newRootNodes, reconcilableResults } = completion; + const { deferredFragmentRecord, newRootNodes, reconcilableResults } = + completion; + const id = deferredFragmentRecord.id; + invariant(id !== undefined); context.pending.push(...this._toPendingResults(newRootNodes)); for (const reconcilableResult of reconcilableResults) { - const { bestId, subPath } = this._incrementalGraph.getBestIdAndSubPath( - deferUsage, - reconcilableResult, - ); + const { deferUsages: resultDeferUsages, path: resultPath } = + reconcilableResult.deferredGroupedFieldSetRecord; + const bestDeferredFragmentRecord = + this._incrementalGraph.getDeepestDeferredFragmentRecord( + deferUsage, + resultDeferUsages, + resultPath, + ); + const bestId = bestDeferredFragmentRecord.id; + invariant(bestId !== undefined); const incrementalEntry: IncrementalDeferResult = { ...reconcilableResult.result, id: bestId, }; - if (subPath !== undefined) { + const subPath = pathToArray(resultPath).slice( + pathToArray(bestDeferredFragmentRecord.path).length, + ); + if (subPath.length > 0) { incrementalEntry.subPath = subPath; } incremental.push(incrementalEntry); From 7bab5d8735ec343b1bd72e52eb3e9f20c46d0121 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 23 Jul 2024 17:55:09 +0300 Subject: [PATCH 8/9] save parentDeferUsage instead of deferUsage --- src/execution/IncrementalGraph.ts | 7 ++++--- src/execution/types.ts | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 779e38efa4..36f1898931 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -56,10 +56,11 @@ class DeferredFragmentFactory { } let deferredFragmentRecord = deferredFragmentRecords.get(deferUsage); if (deferredFragmentRecord === undefined) { + const { label, parentDeferUsage } = deferUsage; deferredFragmentRecord = new DeferredFragmentRecord( - deferUsage, deferUsagePath, - deferUsage.label, + label, + parentDeferUsage, ); deferredFragmentRecords.set(deferUsage, deferredFragmentRecord); } @@ -333,7 +334,7 @@ export class IncrementalGraph { if (this._rootNodes.has(deferredFragmentRecord)) { return; } - const parentDeferUsage = deferredFragmentRecord.deferUsage.parentDeferUsage; + const parentDeferUsage = deferredFragmentRecord.parentDeferUsage; if (parentDeferUsage === undefined) { invariant(initialResultChildren !== undefined); initialResultChildren.add(deferredFragmentRecord); diff --git a/src/execution/types.ts b/src/execution/types.ts index 54378fc4c9..18ef93c747 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -219,22 +219,22 @@ export type SubsequentResultRecord = DeferredFragmentRecord | StreamRecord; /** @internal */ export class DeferredFragmentRecord { - deferUsage: DeferUsage; path: Path | undefined; label: string | undefined; + parentDeferUsage: DeferUsage | undefined; id?: string | undefined; deferredGroupedFieldSetRecords: Set; reconcilableResults: Set; children: Set; constructor( - deferUsage: DeferUsage, path: Path | undefined, label: string | undefined, + parentDeferUsage: DeferUsage | undefined, ) { - this.deferUsage = deferUsage; this.path = path; this.label = label; + this.parentDeferUsage = parentDeferUsage; this.deferredGroupedFieldSetRecords = new Set(); this.reconcilableResults = new Set(); this.children = new Set(); From b4bc441aae5a4662fe3f5a0c4520547466809b8e Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 23 Jul 2024 18:01:04 +0300 Subject: [PATCH 9/9] small tweak --- src/execution/IncrementalPublisher.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index ec7cde4789..d49c07426a 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -256,12 +256,10 @@ class IncrementalPublisher { if (completion === undefined) { continue; } - const incremental = context.incremental; const { deferredFragmentRecord, newRootNodes, reconcilableResults } = completion; - const id = deferredFragmentRecord.id; - invariant(id !== undefined); context.pending.push(...this._toPendingResults(newRootNodes)); + const incremental = context.incremental; for (const reconcilableResult of reconcilableResults) { const { deferUsages: resultDeferUsages, path: resultPath } = reconcilableResult.deferredGroupedFieldSetRecord; @@ -285,6 +283,8 @@ class IncrementalPublisher { } incremental.push(incrementalEntry); } + const id = deferredFragmentRecord.id; + invariant(id !== undefined); context.completed.push({ id }); } }