Skip to content

Commit

Permalink
use object for GraphQLWrappedResult instead of tuple (#4265)
Browse files Browse the repository at this point in the history 
for readability. benchmarks suggest similar or even better performance


![image](https://github.com/user-attachments/assets/c4f900a4-f3b3-4d68-9d6a-c8505c9f56b7)
  • Loading branch information
@yaacovCR
yaacovCR authored Oct 29, 2024
1 parent 12a5ec9 commit d59c725
Showing 1 changed file with 83 additions and 65 deletions.
148 changes: 83 additions & 65 deletions src/execution/execute.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ export interface StreamUsage {
fieldDetailsList: FieldDetailsList;
}

type GraphQLWrappedResult<T> = [T, Array<IncrementalDataRecord> | undefined];
interface GraphQLWrappedResult<T> {
rawResult: T;
incrementalDataRecords: Array<IncrementalDataRecord> | undefined;
}

const UNEXPECTED_EXPERIMENTAL_DIRECTIVES =
'The provided schema unexpectedly contains experimental directives (@defer or @stream). These directives may only be utilized if experimental execution features are explicitly enabled.';
Expand Down Expand Up @@ -360,18 +363,14 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(

if (isPromise(graphqlWrappedResult)) {
return graphqlWrappedResult.then(
(resolved) => buildDataResponse(exeContext, resolved[0], resolved[1]),
(resolved) => buildDataResponse(exeContext, resolved),
(error: unknown) => ({
data: null,
errors: withError(exeContext.errors, error as GraphQLError),
}),
);
}
return buildDataResponse(
exeContext,
graphqlWrappedResult[0],
graphqlWrappedResult[1],
);
return buildDataResponse(exeContext, graphqlWrappedResult);
} catch (error) {
return { data: null, errors: withError(exeContext.errors, error) };
}
Expand Down Expand Up @@ -442,10 +441,10 @@ function addIncrementalDataRecords(
if (incrementalDataRecords === undefined) {
return;
}
if (graphqlWrappedResult[1] === undefined) {
graphqlWrappedResult[1] = [...incrementalDataRecords];
if (graphqlWrappedResult.incrementalDataRecords === undefined) {
graphqlWrappedResult.incrementalDataRecords = [...incrementalDataRecords];
} else {
graphqlWrappedResult[1].push(...incrementalDataRecords);
graphqlWrappedResult.incrementalDataRecords.push(...incrementalDataRecords);
}
}

Expand All @@ -458,9 +457,9 @@ function withError(

function buildDataResponse(
exeContext: ExecutionContext,
data: ObjMap<unknown>,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord> | undefined,
graphqlWrappedResult: GraphQLWrappedResult<ObjMap<unknown>>,
): ExecutionResult | ExperimentalIncrementalExecutionResults {
const { rawResult: data, incrementalDataRecords } = graphqlWrappedResult;
const errors = exeContext.errors;
if (incrementalDataRecords === undefined) {
return errors !== undefined ? { errors, data } : { data };
Expand Down Expand Up @@ -675,7 +674,7 @@ function executeFieldsSerially(
fieldPath,
incrementalContext,
);
graphqlWrappedResult[0][responseName] = null;
graphqlWrappedResult.rawResult[responseName] = null;
return graphqlWrappedResult;
}

Expand All @@ -693,16 +692,25 @@ function executeFieldsSerially(
}
if (isPromise(result)) {
return result.then((resolved) => {
graphqlWrappedResult[0][responseName] = resolved[0];
addIncrementalDataRecords(graphqlWrappedResult, resolved[1]);
graphqlWrappedResult.rawResult[responseName] = resolved.rawResult;
addIncrementalDataRecords(
graphqlWrappedResult,
resolved.incrementalDataRecords,
);
return graphqlWrappedResult;
});
}
graphqlWrappedResult[0][responseName] = result[0];
addIncrementalDataRecords(graphqlWrappedResult, result[1]);
graphqlWrappedResult.rawResult[responseName] = result.rawResult;
addIncrementalDataRecords(
graphqlWrappedResult,
result.incrementalDataRecords,
);
return graphqlWrappedResult;
},
[Object.create(null), undefined] as GraphQLWrappedResult<ObjMap<unknown>>,
{
rawResult: Object.create(null),
incrementalDataRecords: undefined,
},
);
}

Expand All @@ -720,10 +728,10 @@ function executeFields(
deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord> | undefined,
): PromiseOrValue<GraphQLWrappedResult<ObjMap<unknown>>> {
const results = Object.create(null);
const graphqlWrappedResult: GraphQLWrappedResult<ObjMap<unknown>> = [
results,
undefined,
];
const graphqlWrappedResult: GraphQLWrappedResult<ObjMap<unknown>> = {
rawResult: results,
incrementalDataRecords: undefined,
};
let containsPromise = false;

try {
Expand All @@ -742,13 +750,19 @@ function executeFields(
if (result !== undefined) {
if (isPromise(result)) {
results[responseName] = result.then((resolved) => {
addIncrementalDataRecords(graphqlWrappedResult, resolved[1]);
return resolved[0];
addIncrementalDataRecords(
graphqlWrappedResult,
resolved.incrementalDataRecords,
);
return resolved.rawResult;
});
containsPromise = true;
} else {
results[responseName] = result[0];
addIncrementalDataRecords(graphqlWrappedResult, result[1]);
results[responseName] = result.rawResult;
addIncrementalDataRecords(
graphqlWrappedResult,
result.incrementalDataRecords,
);
}
}
}
Expand All @@ -772,10 +786,10 @@ function executeFields(
// Otherwise, results is a map from field name to the result of resolving that
// field, which is possibly a promise. Return a promise that will return this
// same map, but with any promises replaced with the values they resolved to.
return promiseForObject(results, (resolved) => [
resolved,
graphqlWrappedResult[1],
]);
return promiseForObject(results, (resolved) => ({
rawResult: resolved,
incrementalDataRecords: graphqlWrappedResult.incrementalDataRecords,
}));
}

function toNodes(fieldDetailsList: FieldDetailsList): ReadonlyArray<FieldNode> {
Expand Down Expand Up @@ -871,7 +885,7 @@ function executeField(
path,
incrementalContext,
);
return [null, undefined];
return { rawResult: null, incrementalDataRecords: undefined };
});
}
return completed;
Expand All @@ -884,7 +898,7 @@ function executeField(
path,
incrementalContext,
);
return [null, undefined];
return { rawResult: null, incrementalDataRecords: undefined };
}
}

Expand Down Expand Up @@ -997,7 +1011,7 @@ function completeValue(
incrementalContext,
deferMap,
);
if ((completed as GraphQLWrappedResult<unknown>)[0] === null) {
if ((completed as GraphQLWrappedResult<unknown>).rawResult === null) {
throw new Error(
`Cannot return null for non-nullable field ${info.parentType}.${info.fieldName}.`,
);
Expand All @@ -1007,7 +1021,7 @@ function completeValue(

// If result value is null or undefined then return null.
if (result == null) {
return [null, undefined];
return { rawResult: null, incrementalDataRecords: undefined };
}

// If field type is List, complete each item in the list with the inner type
Expand All @@ -1027,7 +1041,10 @@ function completeValue(
// If field type is a leaf type, Scalar or Enum, coerce to a valid value,
// returning null if coercion is not possible.
if (isLeafType(returnType)) {
return [completeLeafValue(returnType, result), undefined];
return {
rawResult: completeLeafValue(returnType, result),
incrementalDataRecords: undefined,
};
}

// If field type is an abstract type, Interface or Union, determine the
Expand Down Expand Up @@ -1103,7 +1120,7 @@ async function completePromisedValue(
path,
incrementalContext,
);
return [null, undefined];
return { rawResult: null, incrementalDataRecords: undefined };
}
}

Expand Down Expand Up @@ -1201,10 +1218,10 @@ async function completeAsyncIteratorValue(
): Promise<GraphQLWrappedResult<ReadonlyArray<unknown>>> {
let containsPromise = false;
const completedResults: Array<unknown> = [];
const graphqlWrappedResult: GraphQLWrappedResult<Array<unknown>> = [
completedResults,
undefined,
];
const graphqlWrappedResult: GraphQLWrappedResult<Array<unknown>> = {
rawResult: completedResults,
incrementalDataRecords: undefined,
};
let index = 0;
const streamUsage = getStreamUsage(
exeContext.validatedExecutionArgs,
Expand Down Expand Up @@ -1323,10 +1340,10 @@ async function completeAsyncIteratorValue(
}

return containsPromise
? /* c8 ignore start */ Promise.all(completedResults).then((resolved) => [
resolved,
graphqlWrappedResult[1],
])
? /* c8 ignore start */ Promise.all(completedResults).then((resolved) => ({
rawResult: resolved,
incrementalDataRecords: graphqlWrappedResult.incrementalDataRecords,
}))
: /* c8 ignore stop */ graphqlWrappedResult;
}

Expand Down Expand Up @@ -1393,10 +1410,10 @@ function completeIterableValue(
// where the list contains no Promises by avoiding creating another Promise.
let containsPromise = false;
const completedResults: Array<unknown> = [];
const graphqlWrappedResult: GraphQLWrappedResult<Array<unknown>> = [
completedResults,
undefined,
];
const graphqlWrappedResult: GraphQLWrappedResult<Array<unknown>> = {
rawResult: completedResults,
incrementalDataRecords: undefined,
};
let index = 0;
const streamUsage = getStreamUsage(
exeContext.validatedExecutionArgs,
Expand Down Expand Up @@ -1469,10 +1486,10 @@ function completeIterableValue(
}

return containsPromise
? Promise.all(completedResults).then((resolved) => [
resolved,
graphqlWrappedResult[1],
])
? Promise.all(completedResults).then((resolved) => ({
rawResult: resolved,
incrementalDataRecords: graphqlWrappedResult.incrementalDataRecords,
}))
: graphqlWrappedResult;
}

Expand Down Expand Up @@ -1511,8 +1528,8 @@ function completeListItemValue(
completedResults.push(
completedItem.then(
(resolved) => {
addIncrementalDataRecords(parent, resolved[1]);
return resolved[0];
addIncrementalDataRecords(parent, resolved.incrementalDataRecords);
return resolved.rawResult;
},
(rawError: unknown) => {
handleFieldError(
Expand All @@ -1530,8 +1547,8 @@ function completeListItemValue(
return true;
}

completedResults.push(completedItem[0]);
addIncrementalDataRecords(parent, completedItem[1]);
completedResults.push(completedItem.rawResult);
addIncrementalDataRecords(parent, completedItem.incrementalDataRecords);
} catch (rawError) {
handleFieldError(
rawError,
Expand Down Expand Up @@ -1572,8 +1589,8 @@ async function completePromisedListItemValue(
if (isPromise(completed)) {
completed = await completed;
}
addIncrementalDataRecords(parent, completed[1]);
return completed[0];
addIncrementalDataRecords(parent, completed.incrementalDataRecords);
return completed.rawResult;
} catch (rawError) {
handleFieldError(
rawError,
Expand Down Expand Up @@ -2343,12 +2360,12 @@ function buildCompletedExecutionGroup(
path: Path | undefined,
result: GraphQLWrappedResult<ObjMap<unknown>>,
): CompletedExecutionGroup {
const { rawResult: data, incrementalDataRecords } = result;
return {
pendingExecutionGroup,
path: pathToArray(path),
result:
errors === undefined ? { data: result[0] } : { data: result[0], errors },
incrementalDataRecords: result[1],
result: errors === undefined ? { data } : { data, errors },
incrementalDataRecords,
};
}

Expand Down Expand Up @@ -2583,7 +2600,7 @@ function completeStreamItem(
itemPath,
incrementalContext,
);
result = [null, undefined];
result = { rawResult: null, incrementalDataRecords: undefined };
}
} catch (error) {
return {
Expand All @@ -2602,7 +2619,7 @@ function completeStreamItem(
itemPath,
incrementalContext,
);
return [null, undefined] as GraphQLWrappedResult<unknown>;
return { rawResult: null, incrementalDataRecords: undefined };
})
.then(
(resolvedItem) =>
Expand All @@ -2620,9 +2637,10 @@ function buildStreamItemResult(
errors: ReadonlyArray<GraphQLError> | undefined,
result: GraphQLWrappedResult<unknown>,
): StreamItemResult {
const { rawResult: item, incrementalDataRecords } = result;
return {
item: result[0],
item,
errors,
incrementalDataRecords: result[1],
incrementalDataRecords,
};
}

0 comments on commit d59c725

Please sign in to comment.