diff --git a/packages/eslint-plugin/docs/rules/await-thenable.mdx b/packages/eslint-plugin/docs/rules/await-thenable.mdx index 3ad4c18de91d..f9905bfcf80a 100644 --- a/packages/eslint-plugin/docs/rules/await-thenable.mdx +++ b/packages/eslint-plugin/docs/rules/await-thenable.mdx @@ -10,9 +10,9 @@ import TabItem from '@theme/TabItem'; > See **https://typescript-eslint.io/rules/await-thenable** for documentation. A "Thenable" value is an object which has a `then` method, such as a Promise. -The `await` keyword is generally used to retrieve the result of calling a Thenable's `then` method. +The [`await` keyword](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await) is generally used to retrieve the result of calling a Thenable's `then` method. -If the `await` keyword is used on a value that is not a Thenable, the value is directly resolved immediately. +If the `await` keyword is used on a value that is not a Thenable, the value is directly resolved, but will still pause execution until the next microtask. While doing so is valid JavaScript, it is often a programmer error, such as forgetting to add parenthesis to call a function that returns a Promise. ## Examples @@ -40,6 +40,86 @@ await createValue(); +## Async Iteration (`for await...of` Loops) + +This rule also inspects [`for await...of` statements](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of), and reports if the value being iterated over is not async-iterable. + +:::info[Why does the rule report on `for await...of` loops used on an array of Promises?] + +While `for await...of` can be used with synchronous iterables, and it will await each promise produced by the iterable, it is inadvisable to do so. +There are some tiny nuances that you may want to consider. + +The biggest difference between using `for await...of` and using `for...of` (plus awaiting each result yourself) is error handling. +When an error occurs within the loop body, `for await...of` does _not_ close the original sync iterable, while `for...of` does. +For detailed examples of this, see the [MDN documentation on using `for await...of` with sync-iterables](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of#iterating_over_sync_iterables_and_generators). + +Also consider whether you need sequential awaiting at all. Using `for await...of` may obscure potential opportunities for concurrent processing, such as those reported by [`no-await-in-loop`](https://eslint.org/docs/latest/rules/no-await-in-loop). Consider instead using one of the [promise concurrency methods](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#promise_concurrency) for better performance. + +::: + +### Examples + + + + +```ts +async function syncIterable() { + const arrayOfValues = [1, 2, 3]; + for await (const value of arrayOfValues) { + console.log(value); + } +} + +async function syncIterableOfPromises() { + const arrayOfPromises = [ + Promise.resolve(1), + Promise.resolve(2), + Promise.resolve(3), + ]; + for await (const promisedValue of arrayOfPromises) { + console.log(promisedValue); + } +} +``` + + + + +```ts +async function syncIterable() { + const arrayOfValues = [1, 2, 3]; + for (const value of arrayOfValues) { + console.log(value); + } +} + +async function syncIterableOfPromises() { + const arrayOfPromises = [ + Promise.resolve(1), + Promise.resolve(2), + Promise.resolve(3), + ]; + for (const promisedValue of await Promise.all(arrayOfPromises)) { + console.log(promisedValue); + } +} + +async function validUseOfForAwaitOnAsyncIterable() { + async function* yieldThingsAsynchronously() { + yield 1; + await new Promise(resolve => setTimeout(resolve, 1000)); + yield 2; + } + + for await (const promisedValue of yieldThingsAsynchronously()) { + console.log(promisedValue); + } +} +``` + + + + ## When Not To Use It If you want to allow code to `await` non-Promise values. diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index 084ea2447e89..4c5a2e6dd171 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -1,4 +1,4 @@ -import type { TSESLint } from '@typescript-eslint/utils'; +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import { @@ -10,8 +10,15 @@ import { nullThrows, NullThrowsReasons, } from '../util'; +import { getForStatementHeadLoc } from '../util/getForStatementHeadLoc'; -export default createRule({ +type MessageId = + | 'await' + | 'forAwaitOfNonThenable' + | 'removeAwait' + | 'convertToOrdinaryFor'; + +export default createRule<[], MessageId>({ name: 'await-thenable', meta: { docs: { @@ -22,7 +29,10 @@ export default createRule({ hasSuggestions: true, messages: { await: 'Unexpected `await` of a non-Promise (non-"Thenable") value.', + forAwaitOfNonThenable: + 'Unexpected `for await...of` of a value that is not async iterable.', removeAwait: 'Remove unnecessary `await`.', + convertToOrdinaryFor: 'Convert to an ordinary `for...of` loop.', }, schema: [], type: 'problem', @@ -62,6 +72,40 @@ export default createRule({ }); } }, + + 'ForOfStatement[await=true]'(node: TSESTree.ForOfStatement): void { + const type = services.getTypeAtLocation(node.right); + if (isTypeAnyType(type)) { + return; + } + + const asyncIteratorSymbol = tsutils.getWellKnownSymbolPropertyOfType( + type, + 'asyncIterator', + checker, + ); + + if (asyncIteratorSymbol == null) { + context.report({ + loc: getForStatementHeadLoc(context.sourceCode, node), + messageId: 'forAwaitOfNonThenable', + suggest: [ + // Note that this suggestion causes broken code for sync iterables + // of promises, since the loop variable is not awaited. + { + messageId: 'convertToOrdinaryFor', + fix(fixer): TSESLint.RuleFix { + const awaitToken = nullThrows( + context.sourceCode.getFirstToken(node, isAwaitKeyword), + NullThrowsReasons.MissingToken('await', 'for await loop'), + ); + return fixer.remove(awaitToken); + }, + }, + ], + }); + } + }, }; }, }); diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/await-thenable.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/await-thenable.shot index 10fdfdb9d0e2..176f8e64da8e 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/await-thenable.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/await-thenable.shot @@ -21,3 +21,63 @@ const createValue = async () => 'value'; await createValue(); " `; + +exports[`Validating rule docs await-thenable.mdx code examples ESLint output 3`] = ` +"Incorrect + +async function syncIterable() { + const arrayOfValues = [1, 2, 3]; + for await (const value of arrayOfValues) { + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unexpected \`for await...of\` of a value that is not async iterable. + console.log(value); + } +} + +async function syncIterableOfPromises() { + const arrayOfPromises = [ + Promise.resolve(1), + Promise.resolve(2), + Promise.resolve(3), + ]; + for await (const promisedValue of arrayOfPromises) { + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unexpected \`for await...of\` of a value that is not async iterable. + console.log(promisedValue); + } +} +" +`; + +exports[`Validating rule docs await-thenable.mdx code examples ESLint output 4`] = ` +"Correct + +async function syncIterable() { + const arrayOfValues = [1, 2, 3]; + for (const value of arrayOfValues) { + console.log(value); + } +} + +async function syncIterableOfPromises() { + const arrayOfPromises = [ + Promise.resolve(1), + Promise.resolve(2), + Promise.resolve(3), + ]; + for (const promisedValue of await Promise.all(arrayOfPromises)) { + console.log(promisedValue); + } +} + +async function validUseOfForAwaitOnAsyncIterable() { + async function* yieldThingsAsynchronously() { + yield 1; + await new Promise(resolve => setTimeout(resolve, 1000)); + yield 2; + } + + for await (const promisedValue of yieldThingsAsynchronously()) { + console.log(promisedValue); + } +} +" +`; diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index ebb8c6ef9537..7d88bea824a0 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -198,6 +198,28 @@ const doSomething = async ( await callback?.(); }; `, + { + code: ` +async function* asyncYieldNumbers() { + yield 1; + yield 2; + yield 3; +} +for await (const value of asyncYieldNumbers()) { + console.log(value); +} + `, + }, + { + code: ` +declare const anee: any; +async function forAwait() { + for await (const value of anee) { + console.log(value); + } +} + `, + }, ], invalid: [ @@ -378,5 +400,73 @@ declare const obj: { a: { b: { c?: () => void } } } | undefined; }, ], }, + { + code: ` +function* yieldNumbers() { + yield 1; + yield 2; + yield 3; +} +for await (const value of yieldNumbers()) { + console.log(value); +} + `, + errors: [ + { + messageId: 'forAwaitOfNonThenable', + line: 7, + endLine: 7, + column: 1, + endColumn: 42, + suggestions: [ + { + messageId: 'convertToOrdinaryFor', + output: ` +function* yieldNumbers() { + yield 1; + yield 2; + yield 3; +} +for (const value of yieldNumbers()) { + console.log(value); +} + `, + }, + ], + }, + ], + }, + { + code: ` +function* yieldNumberPromises() { + yield Promise.resolve(1); + yield Promise.resolve(2); + yield Promise.resolve(3); +} +for await (const value of yieldNumberPromises()) { + console.log(value); +} + `, + errors: [ + { + messageId: 'forAwaitOfNonThenable', + suggestions: [ + { + messageId: 'convertToOrdinaryFor', + output: ` +function* yieldNumberPromises() { + yield Promise.resolve(1); + yield Promise.resolve(2); + yield Promise.resolve(3); +} +for (const value of yieldNumberPromises()) { + console.log(value); +} + `, + }, + ], + }, + ], + }, ], });