Skip to content

perf: do not use undefined for empty error array #4469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

perf: do not use undefined for empty error array #4469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/execution/IncrementalPublisher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
export function buildIncrementalResponse(
context: IncrementalPublisherContext,
result: ObjMap<unknown>,
errors: ReadonlyArray<GraphQLError> | undefined,
errors: ReadonlyArray<GraphQLError>,
newDeferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord> | undefined,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
): ExperimentalIncrementalExecutionResults {
Expand Down Expand Up @@ -75,7 +75,7 @@ class IncrementalPublisher {

buildResponse(
data: ObjMap<unknown>,
errors: ReadonlyArray<GraphQLError> | undefined,
errors: ReadonlyArray<GraphQLError>,
newDeferredFragmentRecords:
| ReadonlyArray<DeferredFragmentRecord>
| undefined,
Expand All @@ -88,10 +88,9 @@ class IncrementalPublisher {

const pending = this._toPendingResults(newRootNodes);

const initialResult: InitialIncrementalExecutionResult =
errors === undefined
? { data, pending, hasNext: true }
: { errors, data, pending, hasNext: true };
const initialResult: InitialIncrementalExecutionResult = errors.length
? { errors, data, pending, hasNext: true }
: { data, pending, hasNext: true };

return {
initialResult,
Expand Down
52 changes: 20 additions & 32 deletions src/execution/execute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ export interface ValidatedExecutionArgs {

export interface ExecutionContext {
validatedExecutionArgs: ValidatedExecutionArgs;
errors: Array<GraphQLError> | undefined;
errors: Array<GraphQLError>;
abortSignalListener: AbortSignalListener | undefined;
completed: boolean;
cancellableStreams: Set<CancellableStreamRecord> | undefined;
errorPropagation: boolean;
}

interface IncrementalContext {
errors: Array<GraphQLError> | undefined;
errors: Array<GraphQLError>;
completed: boolean;
deferUsageSet?: DeferUsageSet | undefined;
}
Expand Down Expand Up @@ -338,7 +338,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
const abortSignal = validatedExecutionArgs.abortSignal;
const exeContext: ExecutionContext = {
validatedExecutionArgs,
errors: undefined,
errors: [],
abortSignalListener: abortSignal
? new AbortSignalListener(abortSignal)
: undefined,
Expand Down Expand Up @@ -395,7 +395,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
exeContext.abortSignalListener?.disconnect();
return {
data: null,
errors: withError(exeContext.errors, error as GraphQLError),
errors: [...exeContext.errors, error as GraphQLError],
};
},
);
Expand All @@ -407,17 +407,10 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
// TODO: add test case for synchronous null bubbling to root with cancellation
/* c8 ignore next */
exeContext.abortSignalListener?.disconnect();
return { data: null, errors: withError(exeContext.errors, error) };
return { data: null, errors: [...exeContext.errors, error] };
}
}

function withError(
errors: Array<GraphQLError> | undefined,
error: GraphQLError,
): ReadonlyArray<GraphQLError> {
return errors === undefined ? [error] : [...errors, error];
}

function buildDataResponse(
exeContext: ExecutionContext,
graphqlWrappedResult: GraphQLWrappedResult<ObjMap<unknown>>,
Expand All @@ -430,7 +423,7 @@ function buildDataResponse(
const errors = exeContext.errors;
if (incrementalDataRecords === undefined) {
exeContext.abortSignalListener?.disconnect();
return errors !== undefined ? { errors, data } : { data };
return errors.length ? { errors, data } : { data };
}

return buildIncrementalResponse(
Expand Down Expand Up @@ -1076,12 +1069,7 @@ function handleFieldError(
// Otherwise, error protection is applied, logging the error and resolving
// a null value for this field if one is encountered.
const context = incrementalContext ?? exeContext;
let errors = context.errors;
if (errors === undefined) {
errors = [];
context.errors = errors;
}
errors.push(error);
context.errors.push(error);
}

/**
Expand Down Expand Up @@ -2513,7 +2501,7 @@ function collectExecutionGroups(
path,
groupedFieldSet,
{
errors: undefined,
errors: [],
completed: false,
deferUsageSet,
},
Expand Down Expand Up @@ -2578,7 +2566,7 @@ function executeExecutionGroup(
return {
pendingExecutionGroup,
path: pathToArray(path),
errors: withError(incrementalContext.errors, error),
errors: [...incrementalContext.errors, error],
};
}

Expand All @@ -2598,7 +2586,7 @@ function executeExecutionGroup(
return {
pendingExecutionGroup,
path: pathToArray(path),
errors: withError(incrementalContext.errors, error as GraphQLError),
errors: [...incrementalContext.errors, error as GraphQLError],
};
},
);
Expand All @@ -2614,7 +2602,7 @@ function executeExecutionGroup(
}

function buildCompletedExecutionGroup(
errors: ReadonlyArray<GraphQLError> | undefined,
errors: ReadonlyArray<GraphQLError>,
pendingExecutionGroup: PendingExecutionGroup,
path: Path | undefined,
result: GraphQLWrappedResult<ObjMap<unknown>>,
Expand All @@ -2627,7 +2615,7 @@ function buildCompletedExecutionGroup(
return {
pendingExecutionGroup,
path: pathToArray(path),
result: errors === undefined ? { data } : { data, errors },
result: errors.length ? { errors, data } : { data },
newDeferredFragmentRecords,
incrementalDataRecords,
};
Expand Down Expand Up @@ -2664,7 +2652,7 @@ function buildSyncStreamItemQueue(
initialPath,
initialItem,
exeContext,
{ errors: undefined, completed: false },
{ errors: [], completed: false },
fieldDetailsList,
info,
itemType,
Expand All @@ -2681,7 +2669,7 @@ function buildSyncStreamItemQueue(
/* c8 ignore next 6 */
if (currentStreamItem instanceof BoxedPromiseOrValue) {
const result = currentStreamItem.value;
if (!isPromise(result) && result.errors !== undefined) {
if (!isPromise(result) && result.item === undefined) {
break;
}
}
Expand All @@ -2695,7 +2683,7 @@ function buildSyncStreamItemQueue(
itemPath,
value,
exeContext,
{ errors: undefined, completed: false },
{ errors: [], completed: false },
fieldDetailsList,
info,
itemType,
Expand Down Expand Up @@ -2787,7 +2775,7 @@ async function getNextAsyncStreamItemResult(
itemPath,
iteration.value,
exeContext,
{ errors: undefined, completed: false },
{ errors: [], completed: false },
fieldDetailsList,
info,
itemType,
Expand Down Expand Up @@ -2845,7 +2833,7 @@ function completeStreamItem(
(error: unknown) => {
incrementalContext.completed = true;
return {
errors: withError(incrementalContext.errors, error as GraphQLError),
errors: [...incrementalContext.errors, error as GraphQLError],
};
},
);
Expand Down Expand Up @@ -2882,7 +2870,7 @@ function completeStreamItem(
} catch (error) {
incrementalContext.completed = true;
return {
errors: withError(incrementalContext.errors, error),
errors: [...incrementalContext.errors, error],
};
}

Expand Down Expand Up @@ -2911,7 +2899,7 @@ function completeStreamItem(
(error: unknown) => {
incrementalContext.completed = true;
return {
errors: withError(incrementalContext.errors, error as GraphQLError),
errors: [...incrementalContext.errors, error as GraphQLError],
};
},
);
Expand All @@ -2922,7 +2910,7 @@ function completeStreamItem(
}

function buildStreamItemResult(
errors: ReadonlyArray<GraphQLError> | undefined,
errors: ReadonlyArray<GraphQLError>,
result: GraphQLWrappedResult<unknown>,
): StreamItemResult {
const {
Expand Down
2 changes: 1 addition & 1 deletion src/execution/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ export interface StreamItemResult {
| ReadonlyArray<DeferredFragmentRecord>
| undefined;
incrementalDataRecords?: ReadonlyArray<IncrementalDataRecord> | undefined;
errors?: ReadonlyArray<GraphQLError> | undefined;
errors?: ReadonlyArray<GraphQLError>;
}

export type StreamItemRecord = ThunkIncrementalResult<StreamItemResult>;
Expand Down
Loading