From f534464ede921e0bf472b10eda88e2ad94e1470a Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 9 Dec 2022 10:56:49 +0200 Subject: [PATCH 01/16] add eslint-plugin-promise ...and enable basic rules --- .eslintrc.yml | 22 ++++++++++++++++++++++ package-lock.json | 20 ++++++++++++++++++++ package.json | 1 + 3 files changed, 43 insertions(+) diff --git a/.eslintrc.yml b/.eslintrc.yml index 4049462418..d187d42c0e 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -9,6 +9,7 @@ plugins: - node - import - simple-import-sort + - promise settings: node: tryExtensions: ['.js', '.ts', '.jsx', '.json', '.node', '.d.ts'] @@ -177,6 +178,27 @@ rules: - ["^\\./"] simple-import-sort/exports: off # TODO + ############################################################################## + # `eslint-plugin-promise` rule list based on `v6.1.x` + # https://github.com/eslint-community/eslint-plugin-promise/blob/main/README.md + ############################################################################## + + promise/always-return: off + promise/avoid-new: off + promise/catch-or-return: error + promise/no-callback-in-promise: error + promise/no-multiple-resolved: error + promise/no-native: off + promise/no-nesting: error + promise/no-new-statics: error + promise/no-promise-in-callback: off + promise/no-return-in-finally: error + promise/no-return-wrap: error + promise/param-names: off + promise/prefer-await-to-callbacks: off + promise/prefer-await-to-then: off + promise/valid-params: error + ############################################################################## # ESLint builtin rules list based on `v8.27.x` ############################################################################## diff --git a/package-lock.json b/package-lock.json index 1f295999b7..2cc1b9526f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,6 +31,7 @@ "eslint-plugin-import": "2.26.0", "eslint-plugin-internal-rules": "file:./resources/eslint-internal-rules", "eslint-plugin-node": "11.1.0", + "eslint-plugin-promise": "^6.1.1", "eslint-plugin-react": "7.31.10", "eslint-plugin-react-hooks": "4.6.0", "eslint-plugin-simple-import-sort": "8.0.0", @@ -8208,6 +8209,18 @@ "semver": "bin/semver.js" } }, + "node_modules/eslint-plugin-promise": { + "version": "6.1.1", + "resolved": "https://registry.npmjs.org/eslint-plugin-promise/-/eslint-plugin-promise-6.1.1.tgz", + "integrity": "sha512-tjqWDwVZQo7UIPMeDReOpUgHCmCiH+ePnVT+5zVapL0uuHnegBUs2smM13CzOs2Xb5+MHMRFTs9v24yjba4Oig==", + "dev": true, + "engines": { + "node": "^12.22.0 || ^14.17.0 || >=16.0.0" + }, + "peerDependencies": { + "eslint": "^7.0.0 || ^8.0.0" + } + }, "node_modules/eslint-plugin-react": { "version": "7.31.10", "resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-7.31.10.tgz", @@ -24233,6 +24246,13 @@ } } }, + "eslint-plugin-promise": { + "version": "6.1.1", + "resolved": "https://registry.npmjs.org/eslint-plugin-promise/-/eslint-plugin-promise-6.1.1.tgz", + "integrity": "sha512-tjqWDwVZQo7UIPMeDReOpUgHCmCiH+ePnVT+5zVapL0uuHnegBUs2smM13CzOs2Xb5+MHMRFTs9v24yjba4Oig==", + "dev": true, + "requires": {} + }, "eslint-plugin-react": { "version": "7.31.10", "resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-7.31.10.tgz", diff --git a/package.json b/package.json index 88c2668dca..4edfaba020 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "eslint-plugin-import": "2.26.0", "eslint-plugin-internal-rules": "file:./resources/eslint-internal-rules", "eslint-plugin-node": "11.1.0", + "eslint-plugin-promise": "^6.1.1", "eslint-plugin-react": "7.31.10", "eslint-plugin-react-hooks": "4.6.0", "eslint-plugin-simple-import-sort": "8.0.0", From 67e453c29eb26be087d461acc1be59ec4d2ce0ab Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 13 Dec 2022 06:15:19 +0200 Subject: [PATCH 02/16] enable promise/prefer-await-to-then leads to one less tick in `executeStreamField` in the branch where completion of a non-promise yields a promise. --- .eslintrc.yml | 2 +- .../__tests__/resolveOnNextTick-test.ts | 13 +- .../expectEqualPromisesOrValues.ts | 7 +- src/execution/__tests__/simplePubSub-test.ts | 10 +- src/execution/__tests__/stream-test.ts | 5 - src/execution/execute.ts | 301 +++++++++++------- src/jsutils/promiseReduce.ts | 12 +- 7 files changed, 212 insertions(+), 138 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index d187d42c0e..127a3c91b8 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -196,7 +196,7 @@ rules: promise/no-return-wrap: error promise/param-names: off promise/prefer-await-to-callbacks: off - promise/prefer-await-to-then: off + promise/prefer-await-to-then: error promise/valid-params: error ############################################################################## diff --git a/src/__testUtils__/__tests__/resolveOnNextTick-test.ts b/src/__testUtils__/__tests__/resolveOnNextTick-test.ts index 96e618b2db..9ee798e961 100644 --- a/src/__testUtils__/__tests__/resolveOnNextTick-test.ts +++ b/src/__testUtils__/__tests__/resolveOnNextTick-test.ts @@ -7,12 +7,13 @@ describe('resolveOnNextTick', () => { it('resolves promise on the next tick', async () => { const output = []; - const promise1 = resolveOnNextTick().then(() => { - output.push('second'); - }); - const promise2 = resolveOnNextTick().then(() => { - output.push('third'); - }); + async function outputOnNextTick(message: string) { + await resolveOnNextTick(); + output.push(message); + } + + const promise1 = outputOnNextTick('second'); + const promise2 = outputOnNextTick('third'); output.push('first'); await Promise.all([promise1, promise2]); diff --git a/src/__testUtils__/expectEqualPromisesOrValues.ts b/src/__testUtils__/expectEqualPromisesOrValues.ts index 6911fb32c0..468a058e0a 100644 --- a/src/__testUtils__/expectEqualPromisesOrValues.ts +++ b/src/__testUtils__/expectEqualPromisesOrValues.ts @@ -5,13 +5,18 @@ import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; import { expectMatchingValues } from './expectMatchingValues.js'; +async function expectMatchingPromises(items: Promise>) { + const values = await items; + return expectMatchingValues(values); +} + export function expectEqualPromisesOrValues( items: ReadonlyArray>, ): PromiseOrValue { const [firstItem, ...remainingItems] = items; if (isPromise(firstItem)) { if (remainingItems.every(isPromise)) { - return Promise.all(items).then(expectMatchingValues); + return expectMatchingPromises(Promise.all(items)); } } else if (remainingItems.every((item) => !isPromise(item))) { return expectMatchingValues(items); diff --git a/src/execution/__tests__/simplePubSub-test.ts b/src/execution/__tests__/simplePubSub-test.ts index 48d45afbc0..ecb884d3d7 100644 --- a/src/execution/__tests__/simplePubSub-test.ts +++ b/src/execution/__tests__/simplePubSub-test.ts @@ -22,9 +22,13 @@ describe('SimplePubSub', () => { value: 'Banana', }); + async function getNextItem(i: AsyncIterator) { + return i.next(); + } + // Read ahead - const i3 = iterator.next().then((x) => x); - const i4 = iterator.next().then((x) => x); + const i3 = getNextItem(iterator); + const i4 = getNextItem(iterator); // Publish expect(pubsub.emit('Coconut')).to.equal(true); @@ -35,7 +39,7 @@ describe('SimplePubSub', () => { expect(await i3).to.deep.equal({ done: false, value: 'Coconut' }); // Read ahead - const i5 = iterator.next().then((x) => x); + const i5 = getNextItem(iterator); // Terminate queue await iterator.return(); diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index aed5211ae1..a45c0ed286 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -984,11 +984,6 @@ describe('Execute: stream directive', () => { }, ], }, - ], - hasNext: true, - }, - { - incremental: [ { items: [{ nonNullName: 'Han' }], path: ['friendList', 2], diff --git a/src/execution/execute.ts b/src/execution/execute.ts index a7ff5ce607..bd4ff7fcd7 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -291,14 +291,15 @@ export function execute(args: ExecutionArgs): PromiseOrValue { return result; } - return result.then((incrementalResult) => { + return (async () => { + const incrementalResult = await result; if ('initialResult' in incrementalResult) { return { errors: [new GraphQLError(UNEXPECTED_MULTIPLE_PAYLOADS)], }; } return incrementalResult; - }); + })(); } /** @@ -345,8 +346,9 @@ function executeImpl( try { const result = executeOperation(exeContext); if (isPromise(result)) { - return result.then( - (data) => { + return (async () => { + try { + const data = await result; const initialResult = buildResponse(data, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -358,13 +360,13 @@ function executeImpl( }; } return initialResult; - }, - (error) => { + } catch (error) { exeContext.errors.push(error); return buildResponse(null, exeContext.errors); - }, - ); + } + })(); } + const initialResult = buildResponse(result, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -597,10 +599,10 @@ function executeFieldsSerially( return results; } if (isPromise(result)) { - return result.then((resolvedResult) => { - results[responseName] = resolvedResult; + return (async () => { + results[responseName] = await result; return results; - }); + })(); } results[responseName] = result; return results; @@ -646,9 +648,14 @@ function executeFields( } catch (error) { if (containsPromise) { // Ensure that any promises returned by other fields are handled, as they may also reject. - return promiseForObject(results).finally(() => { - throw error; - }); + return (async () => { + try { + await promiseForObject(results); + throw error; + } catch { + throw error; + } + })(); } throw error; } @@ -678,7 +685,6 @@ function executeField( path: Path, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { - const errors = asyncPayloadRecord?.errors ?? exeContext.errors; const fieldName = fieldNodes[0].name.value; const fieldDef = exeContext.schema.getField(parentType, fieldName); if (!fieldDef) { @@ -737,17 +743,37 @@ function executeField( ); if (isPromise(completed)) { - // Note: we don't rely on a `catch` method, but we do expect "thenable" - // to take a second callback for the error case. - return completed.then(undefined, (rawError) => { - const error = locatedError(rawError, fieldNodes, pathToArray(path)); - const handledError = handleFieldError(error, returnType, errors); - filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); - return handledError; - }); + return handleAsyncCompletionError( + completed, + exeContext, + returnType, + fieldNodes, + path, + asyncPayloadRecord, + ); } return completed; } catch (rawError) { + const errors = asyncPayloadRecord?.errors ?? exeContext.errors; + const error = locatedError(rawError, fieldNodes, pathToArray(path)); + const handledError = handleFieldError(error, returnType, errors); + filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); + return handledError; + } +} + +async function handleAsyncCompletionError( + promised: Promise, + exeContext: ExecutionContext, + returnType: GraphQLOutputType, + fieldNodes: ReadonlyArray, + path: Path, + asyncPayloadRecord?: AsyncPayloadRecord, +): Promise { + try { + return await promised; + } catch (rawError) { + const errors = asyncPayloadRecord?.errors ?? exeContext.errors; const error = locatedError(rawError, fieldNodes, pathToArray(path)); const handledError = handleFieldError(error, returnType, errors); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); @@ -1211,16 +1237,14 @@ function completeListItemValue( // Note: we don't rely on a `catch` method, but we do expect "thenable" // to take a second callback for the error case. completedResults.push( - completedItem.then(undefined, (rawError) => { - const error = locatedError( - rawError, - fieldNodes, - pathToArray(itemPath), - ); - const handledError = handleFieldError(error, itemType, errors); - filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); - return handledError; - }), + handleAsyncCompletionError( + completedItem, + exeContext, + itemType, + fieldNodes, + itemPath, + asyncPayloadRecord, + ), ); return true; @@ -1273,8 +1297,9 @@ function completeAbstractValue( const runtimeType = resolveTypeFn(result, contextValue, info, returnType); if (isPromise(runtimeType)) { - return runtimeType.then((resolvedRuntimeType) => - completeObjectValue( + return (async () => { + const resolvedRuntimeType = await runtimeType; + return completeObjectValue( exeContext, ensureValidRuntimeType( resolvedRuntimeType, @@ -1289,8 +1314,8 @@ function completeAbstractValue( path, result, asyncPayloadRecord, - ), - ); + ); + })(); } return completeObjectValue( @@ -1385,8 +1410,9 @@ function completeObjectValue( const isTypeOf = returnType.isTypeOf(result, exeContext.contextValue, info); if (isPromise(isTypeOf)) { - return isTypeOf.then((resolvedIsTypeOf) => { - if (!resolvedIsTypeOf) { + return (async () => { + const condition = await isTypeOf; + if (!condition) { throw invalidReturnTypeError(returnType, result, fieldNodes); } return collectAndExecuteSubfields( @@ -1397,7 +1423,7 @@ function completeObjectValue( result, asyncPayloadRecord, ); - }); + })(); } if (!isTypeOf) { @@ -1502,13 +1528,14 @@ export const defaultTypeResolver: GraphQLTypeResolver = } if (promisedIsTypeOfResults.length) { - return Promise.all(promisedIsTypeOfResults).then((isTypeOfResults) => { + return (async () => { + const isTypeOfResults = await Promise.all(promisedIsTypeOfResults); for (let i = 0; i < isTypeOfResults.length; i++) { if (isTypeOfResults[i]) { return possibleTypes[i].name; } } - }); + })(); } }; @@ -1566,11 +1593,12 @@ export function subscribe( > { const maybePromise = experimentalSubscribeIncrementally(args); if (isPromise(maybePromise)) { - return maybePromise.then((resultOrIterable) => - isAsyncIterable(resultOrIterable) + return (async () => { + const resultOrIterable = await maybePromise; + return isAsyncIterable(resultOrIterable) ? mapAsyncIterable(resultOrIterable, ensureSingleExecutionResult) - : resultOrIterable, - ); + : resultOrIterable; + })(); } return isAsyncIterable(maybePromise) ? mapAsyncIterable(maybePromise, ensureSingleExecutionResult) @@ -1648,9 +1676,10 @@ export function experimentalSubscribeIncrementally( const resultOrStream = createSourceEventStreamImpl(exeContext); if (isPromise(resultOrStream)) { - return resultOrStream.then((resolvedResultOrStream) => - mapSourceToResponse(exeContext, resolvedResultOrStream), - ); + return (async () => { + const resolvedResultOrStream = await resultOrStream; + return mapSourceToResponse(exeContext, resolvedResultOrStream); + })(); } return mapSourceToResponse(exeContext, resultOrStream); @@ -1755,7 +1784,13 @@ function createSourceEventStreamImpl( try { const eventStream = executeSubscription(exeContext); if (isPromise(eventStream)) { - return eventStream.then(undefined, (error) => ({ errors: [error] })); + return (async () => { + try { + return await eventStream; + } catch (error) { + return { errors: [error] }; + } + })(); } return eventStream; @@ -1826,9 +1861,13 @@ function executeSubscription( const result = resolveFn(rootValue, args, contextValue, info); if (isPromise(result)) { - return result.then(assertEventStream).then(undefined, (error) => { - throw locatedError(error, fieldNodes, pathToArray(path)); - }); + return (async () => { + try { + return assertEventStream(await result); + } catch (error) { + throw locatedError(error, fieldNodes, pathToArray(path)); + } + })(); } return assertEventStream(result); @@ -1880,10 +1919,14 @@ function executeDeferredFragment( ); if (isPromise(promiseOrData)) { - promiseOrData = promiseOrData.then(null, (e) => { - asyncPayloadRecord.errors.push(e); - return null; - }); + promiseOrData = (async () => { + try { + return await promiseOrData; + } catch (e) { + asyncPayloadRecord.errors.push(e); + return null; + } + })(); } } catch (e) { asyncPayloadRecord.errors.push(e); @@ -1910,22 +1953,25 @@ function executeStreamField( exeContext, }); if (isPromise(item)) { - const completedItems = completePromisedValue( - exeContext, - itemType, - fieldNodes, - info, - itemPath, - item, - asyncPayloadRecord, - ).then( - (value) => [value], - (error) => { + const completedItems = (async () => { + try { + return [ + await completePromisedValue( + exeContext, + itemType, + fieldNodes, + info, + itemPath, + item, + asyncPayloadRecord, + ), + ]; + } catch (error) { asyncPayloadRecord.errors.push(error); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); return null; - }, - ); + } + })(); asyncPayloadRecord.addItems(completedItems); return asyncPayloadRecord; @@ -1960,25 +2006,30 @@ function executeStreamField( } if (isPromise(completedItem)) { - const completedItems = completedItem - .then(undefined, (rawError) => { - const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); - const handledError = handleFieldError( - error, - itemType, - asyncPayloadRecord.errors, - ); - filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); - return handledError; - }) - .then( - (value) => [value], - (error) => { - asyncPayloadRecord.errors.push(error); - filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); - return null; - }, - ); + const completedItems = (async () => { + try { + try { + return [await completedItem]; + } catch (rawError) { + const error = locatedError( + rawError, + fieldNodes, + pathToArray(itemPath), + ); + const handledError = handleFieldError( + error, + itemType, + asyncPayloadRecord.errors, + ); + filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); + return [handledError]; + } + } catch (error) { + asyncPayloadRecord.errors.push(error); + filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); + return null; + } + })(); asyncPayloadRecord.addItems(completedItems); return asyncPayloadRecord; @@ -2024,16 +2075,14 @@ async function executeStreamIteratorItem( ); if (isPromise(completedItem)) { - completedItem = completedItem.then(undefined, (rawError) => { - const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); - const handledError = handleFieldError( - error, - itemType, - asyncPayloadRecord.errors, - ); - filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); - return handledError; - }); + completedItem = handleAsyncCompletionError( + completedItem, + exeContext, + itemType, + fieldNodes, + itemPath, + asyncPayloadRecord, + ); } return { done: false, value: completedItem }; } catch (rawError) { @@ -2086,6 +2135,7 @@ async function executeStreamIterator( asyncPayloadRecord.addItems(null); // entire stream has errored and bubbled upwards if (iterator?.return) { + // eslint-disable-next-line promise/prefer-await-to-then iterator.return().catch(() => { // ignore errors }); @@ -2097,14 +2147,15 @@ async function executeStreamIterator( let completedItems: PromiseOrValue | null>; if (isPromise(completedItem)) { - completedItems = completedItem.then( - (value) => [value], - (error) => { + completedItems = (async () => { + try { + return [await completedItem]; + } catch (error) { asyncPayloadRecord.errors.push(error); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); return null; - }, - ); + } + })(); } else { completedItems = [completedItem]; } @@ -2138,6 +2189,7 @@ function filterSubsequentPayloads( } // asyncRecord path points to nulled error field if (isStreamPayload(asyncRecord) && asyncRecord.iterator?.return) { + // eslint-disable-next-line promise/prefer-await-to-then asyncRecord.iterator.return().catch(() => { // ignore error }); @@ -2279,20 +2331,25 @@ class DeferredFragmentRecord { this._exeContext.subsequentPayloads.add(this); this.isCompleted = false; this.data = null; - this.promise = new Promise | null>((resolve) => { - this._resolve = (promiseOrValue) => { - resolve(promiseOrValue); - }; - }).then((data) => { - this.data = data; + this.promise = (async () => { + this.data = await new Promise | null>((resolve) => { + this._resolve = (promiseOrValue) => { + resolve(promiseOrValue); + }; + }); this.isCompleted = true; - }); + })(); } addData(data: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.(parentData.then(() => data)); + this._resolve?.( + (async () => { + await parentData; + return data; + })(), + ); return; } this._resolve?.(data); @@ -2330,20 +2387,26 @@ class StreamRecord { this._exeContext.subsequentPayloads.add(this); this.isCompleted = false; this.items = null; - this.promise = new Promise | null>((resolve) => { - this._resolve = (promiseOrValue) => { - resolve(promiseOrValue); - }; - }).then((items) => { + this.promise = (async () => { + const items = await new Promise | null>((resolve) => { + this._resolve = (promiseOrValue) => { + resolve(promiseOrValue); + }; + }); this.items = items; this.isCompleted = true; - }); + })(); } addItems(items: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.(parentData.then(() => items)); + this._resolve?.( + (async () => { + await parentData; + return items; + })(), + ); return; } this._resolve?.(items); diff --git a/src/jsutils/promiseReduce.ts b/src/jsutils/promiseReduce.ts index 871b9c3cf7..b8a357c8be 100644 --- a/src/jsutils/promiseReduce.ts +++ b/src/jsutils/promiseReduce.ts @@ -15,9 +15,15 @@ export function promiseReduce( ): PromiseOrValue { let accumulator = initialValue; for (const value of values) { - accumulator = isPromise(accumulator) - ? accumulator.then((resolved) => callbackFn(resolved, value)) - : callbackFn(accumulator, value); + if (isPromise(accumulator)) { + const intermediateValue = accumulator; + accumulator = (async () => { + const resolved = await intermediateValue; + return callbackFn(resolved, value); + })(); + continue; + } + accumulator = callbackFn(accumulator, value); } return accumulator; } From 842ef363c375d62d6a0af0e6234471f4458c6325 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 13 Dec 2022 06:27:05 +0200 Subject: [PATCH 03/16] enable promise/always-return errors avoided by previously avoiding then/catch --- .eslintrc.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index 127a3c91b8..9c5940716e 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -183,7 +183,7 @@ rules: # https://github.com/eslint-community/eslint-plugin-promise/blob/main/README.md ############################################################################## - promise/always-return: off + promise/always-return: error promise/avoid-new: off promise/catch-or-return: error promise/no-callback-in-promise: error From 9cb974504665750df9ac34ddd08638acfed2f6ae Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 13 Dec 2022 06:24:58 +0200 Subject: [PATCH 04/16] enable promise/param-names --- .eslintrc.yml | 2 +- src/execution/__tests__/executor-test.ts | 2 +- src/execution/flattenAsyncIterable.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index 9c5940716e..904c1e2046 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -194,7 +194,7 @@ rules: promise/no-promise-in-callback: off promise/no-return-in-finally: error promise/no-return-wrap: error - promise/param-names: off + promise/param-names: error promise/prefer-await-to-callbacks: off promise/prefer-await-to-then: error promise/valid-params: error diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index a945b4d0b5..3642b7dfb4 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -419,7 +419,7 @@ describe('Execute: Handles basic execution tasks', () => { return new Promise((resolve) => resolve('async')); }, asyncReject() { - return new Promise((_, reject) => + return new Promise((_resolve, reject) => reject(new Error('Error getting asyncReject')), ); }, diff --git a/src/execution/flattenAsyncIterable.ts b/src/execution/flattenAsyncIterable.ts index 22bdb02338..456eff2a8a 100644 --- a/src/execution/flattenAsyncIterable.ts +++ b/src/execution/flattenAsyncIterable.ts @@ -38,8 +38,8 @@ export function flattenAsyncIterable( } // Nobody else is getting it. We should! let resolve: () => void; - waitForCurrentNestedIterator = new Promise((r) => { - resolve = r; + waitForCurrentNestedIterator = new Promise((_resolve) => { + resolve = _resolve; }); const topIteratorResult = await topIterator.next(); if (topIteratorResult.done) { From 081709d5cdd16df93eeb3c7dda5717277b3a7d0f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 14 Dec 2022 23:09:11 +0200 Subject: [PATCH 05/16] pin exact version --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2cc1b9526f..f27b15c14f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,7 +31,7 @@ "eslint-plugin-import": "2.26.0", "eslint-plugin-internal-rules": "file:./resources/eslint-internal-rules", "eslint-plugin-node": "11.1.0", - "eslint-plugin-promise": "^6.1.1", + "eslint-plugin-promise": "6.1.1", "eslint-plugin-react": "7.31.10", "eslint-plugin-react-hooks": "4.6.0", "eslint-plugin-simple-import-sort": "8.0.0", diff --git a/package.json b/package.json index 4edfaba020..e717b4df1a 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ "eslint-plugin-import": "2.26.0", "eslint-plugin-internal-rules": "file:./resources/eslint-internal-rules", "eslint-plugin-node": "11.1.0", - "eslint-plugin-promise": "^6.1.1", + "eslint-plugin-promise": "6.1.1", "eslint-plugin-react": "7.31.10", "eslint-plugin-react-hooks": "4.6.0", "eslint-plugin-simple-import-sort": "8.0.0", From e46f2299b805d65294493a5f12bcb32c5808a918 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 05:49:23 +0200 Subject: [PATCH 06/16] integrate feedback we can't fix then, but we can replace it. instead of using immediately invoked anonymous asynchronous arrow functions, we can replace the then method with then-like helper functions. In fact, we can optimize these small helpers to the exact cases we need. = after() => helper with only onFulfilled callback = catchAfter => helper with onError callback that handles promise rejection = tryAfter => helper with synchronous onFulfilled callback and onError callback that handles promise rejection and onFulfilled exceptions --- src/execution/execute.ts | 216 ++++++++++++++++------------------- src/jsutils/after.ts | 8 ++ src/jsutils/catchAfter.ts | 10 ++ src/jsutils/promiseReduce.ts | 13 +-- src/jsutils/tryAfter.ts | 11 ++ 5 files changed, 130 insertions(+), 128 deletions(-) create mode 100644 src/jsutils/after.ts create mode 100644 src/jsutils/catchAfter.ts create mode 100644 src/jsutils/tryAfter.ts diff --git a/src/execution/execute.ts b/src/execution/execute.ts index bd4ff7fcd7..46109549f0 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1,3 +1,5 @@ +import { after } from '../jsutils/after.js'; +import { catchAfter } from '../jsutils/catchAfter.js'; import { inspect } from '../jsutils/inspect.js'; import { invariant } from '../jsutils/invariant.js'; import { isAsyncIterable } from '../jsutils/isAsyncIterable.js'; @@ -12,6 +14,7 @@ import { addPath, pathToArray } from '../jsutils/Path.js'; import { promiseForObject } from '../jsutils/promiseForObject.js'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; import { promiseReduce } from '../jsutils/promiseReduce.js'; +import { tryAfter } from '../jsutils/tryAfter.js'; import type { GraphQLFormattedError } from '../error/GraphQLError.js'; import { GraphQLError } from '../error/GraphQLError.js'; @@ -291,15 +294,14 @@ export function execute(args: ExecutionArgs): PromiseOrValue { return result; } - return (async () => { - const incrementalResult = await result; + return after(result, (incrementalResult) => { if ('initialResult' in incrementalResult) { return { errors: [new GraphQLError(UNEXPECTED_MULTIPLE_PAYLOADS)], }; } return incrementalResult; - })(); + }); } /** @@ -346,9 +348,9 @@ function executeImpl( try { const result = executeOperation(exeContext); if (isPromise(result)) { - return (async () => { - try { - const data = await result; + return tryAfter( + result, + (data) => { const initialResult = buildResponse(data, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -360,11 +362,12 @@ function executeImpl( }; } return initialResult; - } catch (error) { + }, + (error) => { exeContext.errors.push(error); return buildResponse(null, exeContext.errors); - } - })(); + }, + ); } const initialResult = buildResponse(result, exeContext.errors); @@ -599,10 +602,10 @@ function executeFieldsSerially( return results; } if (isPromise(result)) { - return (async () => { - results[responseName] = await result; + return after(result, (resolvedResult) => { + results[responseName] = resolvedResult; return results; - })(); + }); } results[responseName] = result; return results; @@ -647,15 +650,15 @@ function executeFields( } } catch (error) { if (containsPromise) { - // Ensure that any promises returned by other fields are handled, as they may also reject. - return (async () => { - try { - await promiseForObject(results); + return tryAfter( + promiseForObject(results), + () => { throw error; - } catch { + }, + () => { throw error; - } - })(); + }, + ); } throw error; } @@ -1297,9 +1300,8 @@ function completeAbstractValue( const runtimeType = resolveTypeFn(result, contextValue, info, returnType); if (isPromise(runtimeType)) { - return (async () => { - const resolvedRuntimeType = await runtimeType; - return completeObjectValue( + return after(runtimeType, (resolvedRuntimeType) => + completeObjectValue( exeContext, ensureValidRuntimeType( resolvedRuntimeType, @@ -1314,8 +1316,8 @@ function completeAbstractValue( path, result, asyncPayloadRecord, - ); - })(); + ), + ); } return completeObjectValue( @@ -1410,9 +1412,8 @@ function completeObjectValue( const isTypeOf = returnType.isTypeOf(result, exeContext.contextValue, info); if (isPromise(isTypeOf)) { - return (async () => { - const condition = await isTypeOf; - if (!condition) { + return after(isTypeOf, (resolvedIsTypeOf) => { + if (!resolvedIsTypeOf) { throw invalidReturnTypeError(returnType, result, fieldNodes); } return collectAndExecuteSubfields( @@ -1423,7 +1424,7 @@ function completeObjectValue( result, asyncPayloadRecord, ); - })(); + }); } if (!isTypeOf) { @@ -1528,14 +1529,13 @@ export const defaultTypeResolver: GraphQLTypeResolver = } if (promisedIsTypeOfResults.length) { - return (async () => { - const isTypeOfResults = await Promise.all(promisedIsTypeOfResults); + return after(Promise.all(promisedIsTypeOfResults), (isTypeOfResults) => { for (let i = 0; i < isTypeOfResults.length; i++) { if (isTypeOfResults[i]) { return possibleTypes[i].name; } } - })(); + }); } }; @@ -1593,12 +1593,11 @@ export function subscribe( > { const maybePromise = experimentalSubscribeIncrementally(args); if (isPromise(maybePromise)) { - return (async () => { - const resultOrIterable = await maybePromise; - return isAsyncIterable(resultOrIterable) + return after(maybePromise, (resultOrIterable) => + isAsyncIterable(resultOrIterable) ? mapAsyncIterable(resultOrIterable, ensureSingleExecutionResult) - : resultOrIterable; - })(); + : resultOrIterable, + ); } return isAsyncIterable(maybePromise) ? mapAsyncIterable(maybePromise, ensureSingleExecutionResult) @@ -1676,10 +1675,9 @@ export function experimentalSubscribeIncrementally( const resultOrStream = createSourceEventStreamImpl(exeContext); if (isPromise(resultOrStream)) { - return (async () => { - const resolvedResultOrStream = await resultOrStream; - return mapSourceToResponse(exeContext, resolvedResultOrStream); - })(); + return after(resultOrStream, (resolvedResultOrStream) => + mapSourceToResponse(exeContext, resolvedResultOrStream), + ); } return mapSourceToResponse(exeContext, resultOrStream); @@ -1784,13 +1782,7 @@ function createSourceEventStreamImpl( try { const eventStream = executeSubscription(exeContext); if (isPromise(eventStream)) { - return (async () => { - try { - return await eventStream; - } catch (error) { - return { errors: [error] }; - } - })(); + return catchAfter(eventStream, (error) => ({ errors: [error] })); } return eventStream; @@ -1861,13 +1853,9 @@ function executeSubscription( const result = resolveFn(rootValue, args, contextValue, info); if (isPromise(result)) { - return (async () => { - try { - return assertEventStream(await result); - } catch (error) { - throw locatedError(error, fieldNodes, pathToArray(path)); - } - })(); + return tryAfter(result, assertEventStream, (error) => { + throw locatedError(error, fieldNodes, pathToArray(path)); + }); } return assertEventStream(result); @@ -1919,17 +1907,13 @@ function executeDeferredFragment( ); if (isPromise(promiseOrData)) { - promiseOrData = (async () => { - try { - return await promiseOrData; - } catch (e) { - asyncPayloadRecord.errors.push(e); - return null; - } - })(); + promiseOrData = catchAfter(promiseOrData, (error) => { + asyncPayloadRecord.errors.push(error); + return null; + }); } - } catch (e) { - asyncPayloadRecord.errors.push(e); + } catch (error) { + asyncPayloadRecord.errors.push(error); promiseOrData = null; } asyncPayloadRecord.addData(promiseOrData); @@ -1953,25 +1937,24 @@ function executeStreamField( exeContext, }); if (isPromise(item)) { - const completedItems = (async () => { - try { - return [ - await completePromisedValue( - exeContext, - itemType, - fieldNodes, - info, - itemPath, - item, - asyncPayloadRecord, - ), - ]; - } catch (error) { + const completedItem = completePromisedValue( + exeContext, + itemType, + fieldNodes, + info, + itemPath, + item, + asyncPayloadRecord, + ); + const completedItems = tryAfter( + completedItem, + (resolved) => [resolved], + (error) => { asyncPayloadRecord.errors.push(error); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); return null; - } - })(); + }, + ); asyncPayloadRecord.addItems(completedItems); return asyncPayloadRecord; @@ -2006,11 +1989,11 @@ function executeStreamField( } if (isPromise(completedItem)) { - const completedItems = (async () => { - try { + const completedItems = tryAfter( + completedItem, + (resolved) => [resolved], + (rawError) => { try { - return [await completedItem]; - } catch (rawError) { const error = locatedError( rawError, fieldNodes, @@ -2023,13 +2006,13 @@ function executeStreamField( ); filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); return [handledError]; + } catch (error) { + asyncPayloadRecord.errors.push(error); + filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); + return null; } - } catch (error) { - asyncPayloadRecord.errors.push(error); - filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); - return null; - } - })(); + }, + ); asyncPayloadRecord.addItems(completedItems); return asyncPayloadRecord; @@ -2147,15 +2130,15 @@ async function executeStreamIterator( let completedItems: PromiseOrValue | null>; if (isPromise(completedItem)) { - completedItems = (async () => { - try { - return [await completedItem]; - } catch (error) { + completedItems = tryAfter( + completedItem, + (resolved) => [resolved], + (error) => { asyncPayloadRecord.errors.push(error); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); return null; - } - })(); + }, + ); } else { completedItems = [completedItem]; } @@ -2331,25 +2314,23 @@ class DeferredFragmentRecord { this._exeContext.subsequentPayloads.add(this); this.isCompleted = false; this.data = null; - this.promise = (async () => { - this.data = await new Promise | null>((resolve) => { + this.promise = after( + new Promise | null>((resolve) => { this._resolve = (promiseOrValue) => { resolve(promiseOrValue); }; - }); - this.isCompleted = true; - })(); + }), + (data) => { + this.data = data; + this.isCompleted = true; + }, + ); } addData(data: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.( - (async () => { - await parentData; - return data; - })(), - ); + this._resolve?.(after(parentData, () => data)); return; } this._resolve?.(data); @@ -2387,26 +2368,23 @@ class StreamRecord { this._exeContext.subsequentPayloads.add(this); this.isCompleted = false; this.items = null; - this.promise = (async () => { - const items = await new Promise | null>((resolve) => { + this.promise = after( + new Promise | null>((resolve) => { this._resolve = (promiseOrValue) => { resolve(promiseOrValue); }; - }); - this.items = items; - this.isCompleted = true; - })(); + }), + (items) => { + this.items = items; + this.isCompleted = true; + }, + ); } addItems(items: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.( - (async () => { - await parentData; - return items; - })(), - ); + this._resolve?.(after(parentData, () => items)); return; } this._resolve?.(items); diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts new file mode 100644 index 0000000000..ee5d208a58 --- /dev/null +++ b/src/jsutils/after.ts @@ -0,0 +1,8 @@ +import type { PromiseOrValue } from './PromiseOrValue'; + +export async function after( + promise: Promise, + onFulfilled: (value: T) => PromiseOrValue, +): Promise { + return onFulfilled(await promise); +} diff --git a/src/jsutils/catchAfter.ts b/src/jsutils/catchAfter.ts new file mode 100644 index 0000000000..05d69d28a0 --- /dev/null +++ b/src/jsutils/catchAfter.ts @@ -0,0 +1,10 @@ +export async function catchAfter( + promise: Promise, + onError: (error: any) => U, +): Promise { + try { + return await promise; + } catch (error) { + return onError(error); + } +} diff --git a/src/jsutils/promiseReduce.ts b/src/jsutils/promiseReduce.ts index b8a357c8be..6838fff3bc 100644 --- a/src/jsutils/promiseReduce.ts +++ b/src/jsutils/promiseReduce.ts @@ -1,3 +1,4 @@ +import { after } from './after.js'; import { isPromise } from './isPromise.js'; import type { PromiseOrValue } from './PromiseOrValue.js'; @@ -15,15 +16,9 @@ export function promiseReduce( ): PromiseOrValue { let accumulator = initialValue; for (const value of values) { - if (isPromise(accumulator)) { - const intermediateValue = accumulator; - accumulator = (async () => { - const resolved = await intermediateValue; - return callbackFn(resolved, value); - })(); - continue; - } - accumulator = callbackFn(accumulator, value); + accumulator = isPromise(accumulator) + ? after(accumulator, (resolved) => callbackFn(resolved, value)) + : callbackFn(accumulator, value); } return accumulator; } diff --git a/src/jsutils/tryAfter.ts b/src/jsutils/tryAfter.ts new file mode 100644 index 0000000000..ac7eb80562 --- /dev/null +++ b/src/jsutils/tryAfter.ts @@ -0,0 +1,11 @@ +export async function tryAfter( + promise: Promise, + onFulfilled: (value: T) => R, + onError: (error: any) => U, +): Promise { + try { + return onFulfilled(await promise); + } catch (error) { + return onError(error); + } +} From 85fc0d80c6fcf88d45bb6d9f53f3847f9360ed75 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 06:02:02 +0200 Subject: [PATCH 07/16] inline handleAsyncCompletionError --- src/execution/execute.ts | 67 ++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 46109549f0..ae3a6c33dc 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -746,14 +746,13 @@ function executeField( ); if (isPromise(completed)) { - return handleAsyncCompletionError( - completed, - exeContext, - returnType, - fieldNodes, - path, - asyncPayloadRecord, - ); + return catchAfter(completed, (rawError) => { + const errors = asyncPayloadRecord?.errors ?? exeContext.errors; + const error = locatedError(rawError, fieldNodes, pathToArray(path)); + const handledError = handleFieldError(error, returnType, errors); + filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); + return handledError; + }); } return completed; } catch (rawError) { @@ -765,25 +764,6 @@ function executeField( } } -async function handleAsyncCompletionError( - promised: Promise, - exeContext: ExecutionContext, - returnType: GraphQLOutputType, - fieldNodes: ReadonlyArray, - path: Path, - asyncPayloadRecord?: AsyncPayloadRecord, -): Promise { - try { - return await promised; - } catch (rawError) { - const errors = asyncPayloadRecord?.errors ?? exeContext.errors; - const error = locatedError(rawError, fieldNodes, pathToArray(path)); - const handledError = handleFieldError(error, returnType, errors); - filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); - return handledError; - } -} - /** * TODO: consider no longer exporting this function * @internal @@ -1240,14 +1220,16 @@ function completeListItemValue( // Note: we don't rely on a `catch` method, but we do expect "thenable" // to take a second callback for the error case. completedResults.push( - handleAsyncCompletionError( - completedItem, - exeContext, - itemType, - fieldNodes, - itemPath, - asyncPayloadRecord, - ), + catchAfter(completedItem, (rawError) => { + const error = locatedError( + rawError, + fieldNodes, + pathToArray(itemPath), + ); + const handledError = handleFieldError(error, itemType, errors); + filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); + return handledError; + }), ); return true; @@ -2058,14 +2040,13 @@ async function executeStreamIteratorItem( ); if (isPromise(completedItem)) { - completedItem = handleAsyncCompletionError( - completedItem, - exeContext, - itemType, - fieldNodes, - itemPath, - asyncPayloadRecord, - ); + completedItem = catchAfter(completedItem, (rawError) => { + const errors = asyncPayloadRecord?.errors ?? exeContext.errors; + const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); + const handledError = handleFieldError(error, itemType, errors); + filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); + return handledError; + }); } return { done: false, value: completedItem }; } catch (rawError) { From 0021021ad5c26faf1b33cfd4cf03eb5f13ffce7a Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 06:04:27 +0200 Subject: [PATCH 08/16] reduce diff --- src/execution/execute.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index ae3a6c33dc..d6d658a997 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -369,7 +369,6 @@ function executeImpl( }, ); } - const initialResult = buildResponse(result, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -688,6 +687,7 @@ function executeField( path: Path, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { + const errors = asyncPayloadRecord?.errors ?? exeContext.errors; const fieldName = fieldNodes[0].name.value; const fieldDef = exeContext.schema.getField(parentType, fieldName); if (!fieldDef) { @@ -747,7 +747,6 @@ function executeField( if (isPromise(completed)) { return catchAfter(completed, (rawError) => { - const errors = asyncPayloadRecord?.errors ?? exeContext.errors; const error = locatedError(rawError, fieldNodes, pathToArray(path)); const handledError = handleFieldError(error, returnType, errors); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); @@ -756,7 +755,6 @@ function executeField( } return completed; } catch (rawError) { - const errors = asyncPayloadRecord?.errors ?? exeContext.errors; const error = locatedError(rawError, fieldNodes, pathToArray(path)); const handledError = handleFieldError(error, returnType, errors); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); @@ -2041,9 +2039,8 @@ async function executeStreamIteratorItem( if (isPromise(completedItem)) { completedItem = catchAfter(completedItem, (rawError) => { - const errors = asyncPayloadRecord?.errors ?? exeContext.errors; const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); - const handledError = handleFieldError(error, itemType, errors); + const handledError = handleFieldError(error, itemType, asyncPayloadRecord.errors); filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); return handledError; }); From 37b7f1a7f6e6df44562f3ffc7d896d86111dc1bc Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 06:51:11 +0200 Subject: [PATCH 09/16] prettier --- src/execution/execute.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index d6d658a997..897fef0f0a 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -2040,7 +2040,11 @@ async function executeStreamIteratorItem( if (isPromise(completedItem)) { completedItem = catchAfter(completedItem, (rawError) => { const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); - const handledError = handleFieldError(error, itemType, asyncPayloadRecord.errors); + const handledError = handleFieldError( + error, + itemType, + asyncPayloadRecord.errors, + ); filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord); return handledError; }); From 5e536cc6b7f7d62590459a2ca34bf27e1ab41c7f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 13:14:36 +0200 Subject: [PATCH 10/16] remove unused type --- src/jsutils/after.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index ee5d208a58..ed9737d7bd 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -1,8 +1,8 @@ import type { PromiseOrValue } from './PromiseOrValue'; -export async function after( +export async function after( promise: Promise, onFulfilled: (value: T) => PromiseOrValue, -): Promise { +): Promise { return onFulfilled(await promise); } From d5b10d793b5437b30d382d14151e3b380ffc1d92 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 13:18:22 +0200 Subject: [PATCH 11/16] change @typescript-eslint/return-await to 'always' Motivation: It is faster to await a promise prior to returning it from an async function than to return a promise with `.then()`. See: https://github.com/tc39/proposal-faster-promise-adoption Previously, this eslint rule errored by default only 'in-try-catch'. --- .eslintrc.yml | 2 +- resources/gen-changelog.ts | 2 +- src/execution/__tests__/simplePubSub-test.ts | 2 +- src/execution/__tests__/stream-test.ts | 7 +------ src/execution/execute.ts | 8 +++++--- src/execution/mapAsyncIterable.ts | 6 +++--- src/jsutils/after.ts | 9 +++++++-- 7 files changed, 19 insertions(+), 17 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index 904c1e2046..a75213e82d 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -651,7 +651,7 @@ overrides: ] '@typescript-eslint/no-useless-constructor': error '@typescript-eslint/require-await': error - '@typescript-eslint/return-await': error + '@typescript-eslint/return-await': [error, 'always'] # Disable for JS and TS '@typescript-eslint/init-declarations': off diff --git a/resources/gen-changelog.ts b/resources/gen-changelog.ts index 9d106fbacc..309ba35675 100644 --- a/resources/gen-changelog.ts +++ b/resources/gen-changelog.ts @@ -294,7 +294,7 @@ async function getPRsInfo( let prNumbers = await splitBatches(commits, batchCommitToPR); prNumbers = Array.from(new Set(prNumbers)); // Remove duplicates - return splitBatches(prNumbers, batchPRInfo); + return await splitBatches(prNumbers, batchPRInfo); } // Split commits into batches of 50 to prevent timeouts diff --git a/src/execution/__tests__/simplePubSub-test.ts b/src/execution/__tests__/simplePubSub-test.ts index ecb884d3d7..82209dd94f 100644 --- a/src/execution/__tests__/simplePubSub-test.ts +++ b/src/execution/__tests__/simplePubSub-test.ts @@ -23,7 +23,7 @@ describe('SimplePubSub', () => { }); async function getNextItem(i: AsyncIterator) { - return i.next(); + return await i.next(); } // Read ahead diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index a45c0ed286..b6ad81278d 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -126,7 +126,7 @@ async function completeAsync( for (let i = 0; i < numCalls; i++) { promises.push(iterator.next()); } - return Promise.all(promises); + return await Promise.all(promises); } function createResolvablePromise(): [Promise, (value?: T) => void] { @@ -531,11 +531,6 @@ describe('Execute: stream directive', () => { }, ], }, - ], - hasNext: true, - }, - { - incremental: [ { items: [{ name: 'Leia', id: '3' }], path: ['friendList', 2], diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 897fef0f0a..8f9b4c2f65 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1077,7 +1077,9 @@ async function completeAsyncIteratorValue( } index += 1; } - return containsPromise ? Promise.all(completedResults) : completedResults; + return containsPromise + ? await Promise.all(completedResults) + : completedResults; } /** @@ -2222,7 +2224,7 @@ function yieldSubsequentPayloads( const hasNext = exeContext.subsequentPayloads.size > 0; if (!incremental.length && hasNext) { - return next(); + return await next(); } if (!hasNext) { @@ -2265,7 +2267,7 @@ function yieldSubsequentPayloads( ): Promise> { await returnStreamIterators(); isDone = true; - return Promise.reject(error); + return await Promise.reject(error); }, }; } diff --git a/src/execution/mapAsyncIterable.ts b/src/execution/mapAsyncIterable.ts index 0f6fd78c2d..a60d1c829b 100644 --- a/src/execution/mapAsyncIterable.ts +++ b/src/execution/mapAsyncIterable.ts @@ -36,17 +36,17 @@ export function mapAsyncIterable( return { async next() { - return mapResult(await iterator.next()); + return await mapResult(await iterator.next()); }, async return(): Promise> { // If iterator.return() does not exist, then type R must be undefined. return typeof iterator.return === 'function' - ? mapResult(await iterator.return()) + ? await mapResult(await iterator.return()) : { value: undefined as any, done: true }; }, async throw(error?: unknown) { if (typeof iterator.throw === 'function') { - return mapResult(await iterator.throw(error)); + return await mapResult(await iterator.throw(error)); } throw error; }, diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index ed9737d7bd..593481a132 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -1,8 +1,13 @@ -import type { PromiseOrValue } from './PromiseOrValue'; +import { isPromise } from './isPromise.js'; +import type { PromiseOrValue } from './PromiseOrValue.js'; export async function after( promise: Promise, onFulfilled: (value: T) => PromiseOrValue, ): Promise { - return onFulfilled(await promise); + const result = onFulfilled(await promise); + if (isPromise(result)) { + return await result; + } + return result; } From 54429a745328331ac1dce26b511d81ccbd3403c8 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 13:20:09 +0200 Subject: [PATCH 12/16] add comments to async helpers --- src/jsutils/after.ts | 8 ++++++++ src/jsutils/catchAfter.ts | 8 ++++++++ src/jsutils/tryAfter.ts | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index 593481a132..d890547862 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -1,6 +1,14 @@ import { isPromise } from './isPromise.js'; import type { PromiseOrValue } from './PromiseOrValue.js'; +/** + * Async Helper Function that avoides `.then()` + * + * It is faster to await a promise prior to returning it from an async function + * than to return a promise with `.then()`. + * + * see: https://github.com/tc39/proposal-faster-promise-adoption + */ export async function after( promise: Promise, onFulfilled: (value: T) => PromiseOrValue, diff --git a/src/jsutils/catchAfter.ts b/src/jsutils/catchAfter.ts index 05d69d28a0..2ceda603d1 100644 --- a/src/jsutils/catchAfter.ts +++ b/src/jsutils/catchAfter.ts @@ -1,3 +1,11 @@ +/** + * Async Helper Function that avoides `.then()` + * + * It is faster to await a promise prior to returning it from an async function + * than to return a promise with `.then()`. + * + * see: https://github.com/tc39/proposal-faster-promise-adoption + */ export async function catchAfter( promise: Promise, onError: (error: any) => U, diff --git a/src/jsutils/tryAfter.ts b/src/jsutils/tryAfter.ts index ac7eb80562..f99d8449e4 100644 --- a/src/jsutils/tryAfter.ts +++ b/src/jsutils/tryAfter.ts @@ -1,3 +1,11 @@ +/** + * Async Helper Function that avoides `.then()` + * + * It is faster to await a promise prior to returning it from an async function + * than to return a promise with `.then()`. + * + * see: https://github.com/tc39/proposal-faster-promise-adoption + */ export async function tryAfter( promise: Promise, onFulfilled: (value: T) => R, From abb78f28d1b091a97fbbbb9323314bda4ba94279 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 13:22:53 +0200 Subject: [PATCH 13/16] introduce new `afterMaybeAsync` helper `after` becomes sync, requiring no return type check for the onFulfilled result. --- src/execution/execute.ts | 9 +++++---- src/jsutils/after.ts | 8 +------- src/jsutils/afterMaybeAsync.ts | 21 +++++++++++++++++++++ src/jsutils/promiseReduce.ts | 4 ++-- 4 files changed, 29 insertions(+), 13 deletions(-) create mode 100644 src/jsutils/afterMaybeAsync.ts diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 8f9b4c2f65..1b83d9eb04 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1,4 +1,5 @@ import { after } from '../jsutils/after.js'; +import { afterMaybeAsync } from '../jsutils/afterMaybeAsync.js'; import { catchAfter } from '../jsutils/catchAfter.js'; import { inspect } from '../jsutils/inspect.js'; import { invariant } from '../jsutils/invariant.js'; @@ -1282,7 +1283,7 @@ function completeAbstractValue( const runtimeType = resolveTypeFn(result, contextValue, info, returnType); if (isPromise(runtimeType)) { - return after(runtimeType, (resolvedRuntimeType) => + return afterMaybeAsync(runtimeType, (resolvedRuntimeType) => completeObjectValue( exeContext, ensureValidRuntimeType( @@ -1394,7 +1395,7 @@ function completeObjectValue( const isTypeOf = returnType.isTypeOf(result, exeContext.contextValue, info); if (isPromise(isTypeOf)) { - return after(isTypeOf, (resolvedIsTypeOf) => { + return afterMaybeAsync(isTypeOf, (resolvedIsTypeOf) => { if (!resolvedIsTypeOf) { throw invalidReturnTypeError(returnType, result, fieldNodes); } @@ -2314,7 +2315,7 @@ class DeferredFragmentRecord { addData(data: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.(after(parentData, () => data)); + this._resolve?.(afterMaybeAsync(parentData, () => data)); return; } this._resolve?.(data); @@ -2368,7 +2369,7 @@ class StreamRecord { addItems(items: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.(after(parentData, () => items)); + this._resolve?.(afterMaybeAsync(parentData, () => items)); return; } this._resolve?.(items); diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index d890547862..d5365fd980 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -1,6 +1,3 @@ -import { isPromise } from './isPromise.js'; -import type { PromiseOrValue } from './PromiseOrValue.js'; - /** * Async Helper Function that avoides `.then()` * @@ -11,11 +8,8 @@ import type { PromiseOrValue } from './PromiseOrValue.js'; */ export async function after( promise: Promise, - onFulfilled: (value: T) => PromiseOrValue, + onFulfilled: (value: T) => R, ): Promise { const result = onFulfilled(await promise); - if (isPromise(result)) { - return await result; - } return result; } diff --git a/src/jsutils/afterMaybeAsync.ts b/src/jsutils/afterMaybeAsync.ts new file mode 100644 index 0000000000..7365a8b5fd --- /dev/null +++ b/src/jsutils/afterMaybeAsync.ts @@ -0,0 +1,21 @@ +import { isPromise } from './isPromise.js'; +import type { PromiseOrValue } from './PromiseOrValue.js'; + +/** + * Async Helper Function that avoides `.then()` + * + * It is faster to await a promise prior to returning it from an async function + * than to return a promise with `.then()`. + * + * see: https://github.com/tc39/proposal-faster-promise-adoption + */ +export async function afterMaybeAsync( + promise: Promise, + onFulfilled: (value: T) => PromiseOrValue, +): Promise { + const result = onFulfilled(await promise); + if (isPromise(result)) { + return await result; + } + return result; +} diff --git a/src/jsutils/promiseReduce.ts b/src/jsutils/promiseReduce.ts index 6838fff3bc..3cc2e13a24 100644 --- a/src/jsutils/promiseReduce.ts +++ b/src/jsutils/promiseReduce.ts @@ -1,4 +1,4 @@ -import { after } from './after.js'; +import { afterMaybeAsync } from './afterMaybeAsync.js'; import { isPromise } from './isPromise.js'; import type { PromiseOrValue } from './PromiseOrValue.js'; @@ -17,7 +17,7 @@ export function promiseReduce( let accumulator = initialValue; for (const value of values) { accumulator = isPromise(accumulator) - ? after(accumulator, (resolved) => callbackFn(resolved, value)) + ? afterMaybeAsync(accumulator, (resolved) => callbackFn(resolved, value)) : callbackFn(accumulator, value); } return accumulator; From 71678fd8f24b606de5faa09bcd59a0bbec9273a5 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 16 Dec 2022 13:42:01 +0200 Subject: [PATCH 14/16] spell `avoids` correctly --- src/jsutils/after.ts | 2 +- src/jsutils/afterMaybeAsync.ts | 2 +- src/jsutils/catchAfter.ts | 2 +- src/jsutils/tryAfter.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index d5365fd980..a307610414 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -1,5 +1,5 @@ /** - * Async Helper Function that avoides `.then()` + * Async Helper Function that avoids `.then()` * * It is faster to await a promise prior to returning it from an async function * than to return a promise with `.then()`. diff --git a/src/jsutils/afterMaybeAsync.ts b/src/jsutils/afterMaybeAsync.ts index 7365a8b5fd..1ebae3a8a1 100644 --- a/src/jsutils/afterMaybeAsync.ts +++ b/src/jsutils/afterMaybeAsync.ts @@ -2,7 +2,7 @@ import { isPromise } from './isPromise.js'; import type { PromiseOrValue } from './PromiseOrValue.js'; /** - * Async Helper Function that avoides `.then()` + * Async Helper Function that avoids `.then()` * * It is faster to await a promise prior to returning it from an async function * than to return a promise with `.then()`. diff --git a/src/jsutils/catchAfter.ts b/src/jsutils/catchAfter.ts index 2ceda603d1..932dbab4bf 100644 --- a/src/jsutils/catchAfter.ts +++ b/src/jsutils/catchAfter.ts @@ -1,5 +1,5 @@ /** - * Async Helper Function that avoides `.then()` + * Async Helper Function that avoids `.then()` * * It is faster to await a promise prior to returning it from an async function * than to return a promise with `.then()`. diff --git a/src/jsutils/tryAfter.ts b/src/jsutils/tryAfter.ts index f99d8449e4..6c5e2e5d60 100644 --- a/src/jsutils/tryAfter.ts +++ b/src/jsutils/tryAfter.ts @@ -1,5 +1,5 @@ /** - * Async Helper Function that avoides `.then()` + * Async Helper Function that avoids `.then()` * * It is faster to await a promise prior to returning it from an async function * than to return a promise with `.then()`. From 682005a5bb1b6dc50bde313ac86ed8a28f1c4d13 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 18 Dec 2022 21:11:07 +0200 Subject: [PATCH 15/16] try just one helper --- src/execution/execute.ts | 33 +++++++++++++++------------------ src/jsutils/after.ts | 27 ++++++++++++++++++++++----- src/jsutils/afterMaybeAsync.ts | 21 --------------------- src/jsutils/catchAfter.ts | 18 ------------------ src/jsutils/promiseReduce.ts | 4 ++-- src/jsutils/tryAfter.ts | 19 ------------------- 6 files changed, 39 insertions(+), 83 deletions(-) delete mode 100644 src/jsutils/afterMaybeAsync.ts delete mode 100644 src/jsutils/catchAfter.ts delete mode 100644 src/jsutils/tryAfter.ts diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 1b83d9eb04..e8512626bb 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1,6 +1,4 @@ import { after } from '../jsutils/after.js'; -import { afterMaybeAsync } from '../jsutils/afterMaybeAsync.js'; -import { catchAfter } from '../jsutils/catchAfter.js'; import { inspect } from '../jsutils/inspect.js'; import { invariant } from '../jsutils/invariant.js'; import { isAsyncIterable } from '../jsutils/isAsyncIterable.js'; @@ -15,7 +13,6 @@ import { addPath, pathToArray } from '../jsutils/Path.js'; import { promiseForObject } from '../jsutils/promiseForObject.js'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; import { promiseReduce } from '../jsutils/promiseReduce.js'; -import { tryAfter } from '../jsutils/tryAfter.js'; import type { GraphQLFormattedError } from '../error/GraphQLError.js'; import { GraphQLError } from '../error/GraphQLError.js'; @@ -349,7 +346,7 @@ function executeImpl( try { const result = executeOperation(exeContext); if (isPromise(result)) { - return tryAfter( + return after( result, (data) => { const initialResult = buildResponse(data, exeContext.errors); @@ -650,7 +647,7 @@ function executeFields( } } catch (error) { if (containsPromise) { - return tryAfter( + return after( promiseForObject(results), () => { throw error; @@ -747,7 +744,7 @@ function executeField( ); if (isPromise(completed)) { - return catchAfter(completed, (rawError) => { + return after(completed, undefined, (rawError) => { const error = locatedError(rawError, fieldNodes, pathToArray(path)); const handledError = handleFieldError(error, returnType, errors); filterSubsequentPayloads(exeContext, path, asyncPayloadRecord); @@ -1221,7 +1218,7 @@ function completeListItemValue( // Note: we don't rely on a `catch` method, but we do expect "thenable" // to take a second callback for the error case. completedResults.push( - catchAfter(completedItem, (rawError) => { + after(completedItem, undefined, (rawError) => { const error = locatedError( rawError, fieldNodes, @@ -1283,7 +1280,7 @@ function completeAbstractValue( const runtimeType = resolveTypeFn(result, contextValue, info, returnType); if (isPromise(runtimeType)) { - return afterMaybeAsync(runtimeType, (resolvedRuntimeType) => + return after(runtimeType, (resolvedRuntimeType) => completeObjectValue( exeContext, ensureValidRuntimeType( @@ -1395,7 +1392,7 @@ function completeObjectValue( const isTypeOf = returnType.isTypeOf(result, exeContext.contextValue, info); if (isPromise(isTypeOf)) { - return afterMaybeAsync(isTypeOf, (resolvedIsTypeOf) => { + return after(isTypeOf, (resolvedIsTypeOf) => { if (!resolvedIsTypeOf) { throw invalidReturnTypeError(returnType, result, fieldNodes); } @@ -1765,7 +1762,7 @@ function createSourceEventStreamImpl( try { const eventStream = executeSubscription(exeContext); if (isPromise(eventStream)) { - return catchAfter(eventStream, (error) => ({ errors: [error] })); + return after(eventStream, undefined, (error) => ({ errors: [error] })); } return eventStream; @@ -1836,7 +1833,7 @@ function executeSubscription( const result = resolveFn(rootValue, args, contextValue, info); if (isPromise(result)) { - return tryAfter(result, assertEventStream, (error) => { + return after(result, assertEventStream, (error) => { throw locatedError(error, fieldNodes, pathToArray(path)); }); } @@ -1890,7 +1887,7 @@ function executeDeferredFragment( ); if (isPromise(promiseOrData)) { - promiseOrData = catchAfter(promiseOrData, (error) => { + promiseOrData = after(promiseOrData, undefined, (error) => { asyncPayloadRecord.errors.push(error); return null; }); @@ -1929,7 +1926,7 @@ function executeStreamField( item, asyncPayloadRecord, ); - const completedItems = tryAfter( + const completedItems = after( completedItem, (resolved) => [resolved], (error) => { @@ -1972,7 +1969,7 @@ function executeStreamField( } if (isPromise(completedItem)) { - const completedItems = tryAfter( + const completedItems = after( completedItem, (resolved) => [resolved], (rawError) => { @@ -2041,7 +2038,7 @@ async function executeStreamIteratorItem( ); if (isPromise(completedItem)) { - completedItem = catchAfter(completedItem, (rawError) => { + completedItem = after(completedItem, undefined, (rawError) => { const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); const handledError = handleFieldError( error, @@ -2115,7 +2112,7 @@ async function executeStreamIterator( let completedItems: PromiseOrValue | null>; if (isPromise(completedItem)) { - completedItems = tryAfter( + completedItems = after( completedItem, (resolved) => [resolved], (error) => { @@ -2315,7 +2312,7 @@ class DeferredFragmentRecord { addData(data: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.(afterMaybeAsync(parentData, () => data)); + this._resolve?.(after(parentData, () => data)); return; } this._resolve?.(data); @@ -2369,7 +2366,7 @@ class StreamRecord { addItems(items: PromiseOrValue | null>) { const parentData = this.parentContext?.promise; if (parentData) { - this._resolve?.(afterMaybeAsync(parentData, () => items)); + this._resolve?.(after(parentData, () => items)); return; } this._resolve?.(items); diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index a307610414..613e3d9155 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -1,3 +1,6 @@ +import { isPromise } from './isPromise.js'; +import type { PromiseOrValue } from './PromiseOrValue.js'; + /** * Async Helper Function that avoids `.then()` * @@ -6,10 +9,24 @@ * * see: https://github.com/tc39/proposal-faster-promise-adoption */ -export async function after( +export async function after( promise: Promise, - onFulfilled: (value: T) => R, -): Promise { - const result = onFulfilled(await promise); - return result; + onFulfilled?: (value: T) => PromiseOrValue, + onError?: (error: any) => U, +): Promise { + try { + const result = + onFulfilled === undefined + ? ((await promise) as R) + : onFulfilled(await promise); + if (isPromise(result)) { + return await result; + } + return result; + } catch (error) { + if (onError === undefined) { + throw error; + } + return onError(error); + } } diff --git a/src/jsutils/afterMaybeAsync.ts b/src/jsutils/afterMaybeAsync.ts deleted file mode 100644 index 1ebae3a8a1..0000000000 --- a/src/jsutils/afterMaybeAsync.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { isPromise } from './isPromise.js'; -import type { PromiseOrValue } from './PromiseOrValue.js'; - -/** - * Async Helper Function that avoids `.then()` - * - * It is faster to await a promise prior to returning it from an async function - * than to return a promise with `.then()`. - * - * see: https://github.com/tc39/proposal-faster-promise-adoption - */ -export async function afterMaybeAsync( - promise: Promise, - onFulfilled: (value: T) => PromiseOrValue, -): Promise { - const result = onFulfilled(await promise); - if (isPromise(result)) { - return await result; - } - return result; -} diff --git a/src/jsutils/catchAfter.ts b/src/jsutils/catchAfter.ts deleted file mode 100644 index 932dbab4bf..0000000000 --- a/src/jsutils/catchAfter.ts +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Async Helper Function that avoids `.then()` - * - * It is faster to await a promise prior to returning it from an async function - * than to return a promise with `.then()`. - * - * see: https://github.com/tc39/proposal-faster-promise-adoption - */ -export async function catchAfter( - promise: Promise, - onError: (error: any) => U, -): Promise { - try { - return await promise; - } catch (error) { - return onError(error); - } -} diff --git a/src/jsutils/promiseReduce.ts b/src/jsutils/promiseReduce.ts index 3cc2e13a24..6838fff3bc 100644 --- a/src/jsutils/promiseReduce.ts +++ b/src/jsutils/promiseReduce.ts @@ -1,4 +1,4 @@ -import { afterMaybeAsync } from './afterMaybeAsync.js'; +import { after } from './after.js'; import { isPromise } from './isPromise.js'; import type { PromiseOrValue } from './PromiseOrValue.js'; @@ -17,7 +17,7 @@ export function promiseReduce( let accumulator = initialValue; for (const value of values) { accumulator = isPromise(accumulator) - ? afterMaybeAsync(accumulator, (resolved) => callbackFn(resolved, value)) + ? after(accumulator, (resolved) => callbackFn(resolved, value)) : callbackFn(accumulator, value); } return accumulator; diff --git a/src/jsutils/tryAfter.ts b/src/jsutils/tryAfter.ts deleted file mode 100644 index 6c5e2e5d60..0000000000 --- a/src/jsutils/tryAfter.ts +++ /dev/null @@ -1,19 +0,0 @@ -/** - * Async Helper Function that avoids `.then()` - * - * It is faster to await a promise prior to returning it from an async function - * than to return a promise with `.then()`. - * - * see: https://github.com/tc39/proposal-faster-promise-adoption - */ -export async function tryAfter( - promise: Promise, - onFulfilled: (value: T) => R, - onError: (error: any) => U, -): Promise { - try { - return onFulfilled(await promise); - } catch (error) { - return onError(error); - } -} From 8cebd7ef09c77eeec2cac480983cb902df83f362 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 27 Dec 2022 08:53:26 +0200 Subject: [PATCH 16/16] remove extra await from after! --- src/jsutils/after.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/jsutils/after.ts b/src/jsutils/after.ts index 613e3d9155..6216d87be6 100644 --- a/src/jsutils/after.ts +++ b/src/jsutils/after.ts @@ -15,13 +15,16 @@ export async function after( onError?: (error: any) => U, ): Promise { try { - const result = - onFulfilled === undefined - ? ((await promise) as R) - : onFulfilled(await promise); + if (onFulfilled === undefined) { + return (await promise) as R; + } + + const result = onFulfilled(await promise); + if (isPromise(result)) { return await result; } + return result; } catch (error) { if (onError === undefined) {