Skip to content

Commit 4649b23

Browse files
committed
enhancement: remove extra ticks (#3754)
Promise resolution order changes in some instances, resulting in different orders for some errors within the errors array, as well as in different values of hasNext within incremental delivery payloads. This PR introduces an async `completePromisedValue` helper function rather than using a promise chain (see below links). https://github.com/tc39/proposal-faster-promise-adoption tc39/ecma262#2770 tc39/ecma262#2772 tc39/ecma262#1250 https://v8.dev/blog/fast-async Depends on #3793
1 parent 56c8574 commit 4649b23

File tree

2 files changed

+60
-33
lines changed

2 files changed

+60
-33
lines changed

src/execution/__tests__/nonnull-test.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,16 @@ describe('Execute: handles non-nullable types', () => {
259259
path: ['syncNest', 'syncNest', 'sync'],
260260
locations: [{ line: 6, column: 22 }],
261261
},
262+
{
263+
message: promiseError.message,
264+
path: ['syncNest', 'promise'],
265+
locations: [{ line: 5, column: 11 }],
266+
},
267+
{
268+
message: promiseError.message,
269+
path: ['syncNest', 'syncNest', 'promise'],
270+
locations: [{ line: 6, column: 27 }],
271+
},
262272
{
263273
message: syncError.message,
264274
path: ['syncNest', 'promiseNest', 'sync'],
@@ -274,21 +284,6 @@ describe('Execute: handles non-nullable types', () => {
274284
path: ['promiseNest', 'syncNest', 'sync'],
275285
locations: [{ line: 12, column: 22 }],
276286
},
277-
{
278-
message: promiseError.message,
279-
path: ['syncNest', 'promise'],
280-
locations: [{ line: 5, column: 11 }],
281-
},
282-
{
283-
message: promiseError.message,
284-
path: ['syncNest', 'syncNest', 'promise'],
285-
locations: [{ line: 6, column: 27 }],
286-
},
287-
{
288-
message: syncError.message,
289-
path: ['promiseNest', 'promiseNest', 'sync'],
290-
locations: [{ line: 13, column: 25 }],
291-
},
292287
{
293288
message: promiseError.message,
294289
path: ['syncNest', 'promiseNest', 'promise'],
@@ -304,6 +299,11 @@ describe('Execute: handles non-nullable types', () => {
304299
path: ['promiseNest', 'syncNest', 'promise'],
305300
locations: [{ line: 12, column: 27 }],
306301
},
302+
{
303+
message: syncError.message,
304+
path: ['promiseNest', 'promiseNest', 'sync'],
305+
locations: [{ line: 13, column: 25 }],
306+
},
307307
{
308308
message: promiseError.message,
309309
path: ['promiseNest', 'promiseNest', 'promise'],

src/execution/execute.ts

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -526,15 +526,14 @@ function executeField(
526526
const result = resolveFn(source, args, contextValue, info);
527527

528528
if (isPromise(result)) {
529-
const completed = result.then((resolved) =>
530-
completeValue(exeContext, returnType, fieldNodes, info, path, resolved),
529+
return completePromisedValue(
530+
exeContext,
531+
returnType,
532+
fieldNodes,
533+
info,
534+
path,
535+
result,
531536
);
532-
// Note: we don't rely on a `catch` method, but we do expect "thenable"
533-
// to take a second callback for the error case.
534-
return completed.then(undefined, (rawError) => {
535-
const error = locatedError(rawError, fieldNodes, pathToArray(path));
536-
return handleFieldError(error, returnType, errors);
537-
});
538537
}
539538

540539
const completed = completeValue(
@@ -712,6 +711,36 @@ function completeValue(
712711
);
713712
}
714713

714+
async function completePromisedValue(
715+
exeContext: ExecutionContext,
716+
returnType: GraphQLOutputType,
717+
fieldNodes: ReadonlyArray<FieldNode>,
718+
info: GraphQLResolveInfo,
719+
path: Path,
720+
result: Promise<unknown>,
721+
): Promise<unknown> {
722+
try {
723+
const resolved = await result;
724+
let completed = completeValue(
725+
exeContext,
726+
returnType,
727+
fieldNodes,
728+
info,
729+
path,
730+
resolved,
731+
);
732+
if (isPromise(completed)) {
733+
// see: https://github.com/tc39/proposal-faster-promise-adoption
734+
// it is faster to await a promise prior to returning it from an async function
735+
completed = await completed;
736+
}
737+
return completed;
738+
} catch (rawError) {
739+
const error = locatedError(rawError, fieldNodes, pathToArray(path));
740+
return handleFieldError(error, returnType, exeContext.errors);
741+
}
742+
}
743+
715744
/**
716745
* Complete a async iterator value by completing the result and calling
717746
* recursively until all the results are completed.
@@ -845,17 +874,15 @@ function completeListItemValue(
845874
itemPath: Path,
846875
): boolean {
847876
if (isPromise(item)) {
848-
const completedItem = item.then((resolved) =>
849-
completeValue(exeContext, itemType, fieldNodes, info, itemPath, resolved),
850-
);
851-
852-
// Note: we don't rely on a `catch` method, but we do expect "thenable"
853-
// to take a second callback for the error case.
854877
completedResults.push(
855-
completedItem.then(undefined, (rawError) => {
856-
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
857-
return handleFieldError(error, itemType, errors);
858-
}),
878+
completePromisedValue(
879+
exeContext,
880+
itemType,
881+
fieldNodes,
882+
info,
883+
itemPath,
884+
item,
885+
),
859886
);
860887

861888
return true;

0 commit comments

Comments
 (0)