From 353bb302c77d53fed1dcd27ffd7cb5da4fbe2324 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 15:25:01 -0600 Subject: [PATCH 01/10] check for-await loop iteree --- .../docs/rules/await-thenable.mdx | 74 +++++++++++++++- .../eslint-plugin/src/rules/await-thenable.ts | 51 ++++++++++- .../await-thenable.shot | 62 ++++++++++++++ .../tests/rules/await-thenable.test.ts | 85 +++++++++++++++++++ 4 files changed, 269 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/await-thenable.mdx b/packages/eslint-plugin/docs/rules/await-thenable.mdx index 3ad4c18de91d..1378e8470b5b 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,76 @@ 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), which are designed for iterating over async-iterable objects. +If the value being iterated over is not async-iterable, an ordinary `for...of` statement is preferable, even if the value is an iterable of Thenables. + +### 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 promise of arrayOfPromises) { + // Note that the promise must be explicitly awaited if the promised value is needed. + const promisedValue = await promise; + 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..707ad47580ef 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -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 AsyncIterable.', removeAwait: 'Remove unnecessary `await`.', + convertToOrdinaryFor: 'Convert to an ordinary `for...of` loop.', }, schema: [], type: 'problem', @@ -62,6 +72,45 @@ export default createRule({ }); } }, + + ForOfStatement(node): void { + if (!node.await) { + return; + } + + const type = services.getTypeAtLocation(node.right); + if (isTypeAnyType(type) || isTypeUnknownType(type)) { + return; + } + + const asyncIteratorSymbol = tsutils.getWellKnownSymbolPropertyOfType( + type, + 'asyncIterator', + checker, + ); + + // if there is an async iterator symbol, but it doesn't have the correct + // shape, TS will report a type error, so we only need to check if the + // symbol exists. + if (asyncIteratorSymbol == null) { + context.report({ + loc: getForStatementHeadLoc(context.sourceCode, node), + messageId: 'forAwaitOfNonThenable', + suggest: [ + { + 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..81b468a75dab 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,65 @@ 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 AsyncIterable. + 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 AsyncIterable. + 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 promise of arrayOfPromises) { + // Note that the promise must be explicitly awaited if the promised value is needed. + const promisedValue = await promise; + 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..459d61594e37 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -198,6 +198,23 @@ const doSomething = async ( await callback?.(); }; `, + { + code: ` +Promise.race(...[1, 'a', 3]); + `, + }, + { + code: ` +async function* yieldNumbers() { + yield 1; + yield 2; + yield 3; +} +for await (const value of yieldNumbers()) { + console.log(value); +} + `, + }, ], invalid: [ @@ -378,5 +395,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); +} + `, + }, + ], + }, + ], + }, ], }); From e4ae52b43b615a30865586a1994b94fdec7e1672 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 16:04:02 -0600 Subject: [PATCH 02/10] test fixup --- .../eslint-plugin/tests/rules/await-thenable.test.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index 459d61594e37..2093bf08a333 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -200,17 +200,12 @@ const doSomething = async ( `, { code: ` -Promise.race(...[1, 'a', 3]); - `, - }, - { - code: ` -async function* yieldNumbers() { +async function* asyncYieldNumbers() { yield 1; yield 2; yield 3; } -for await (const value of yieldNumbers()) { +for await (const value of asyncYieldNumbers()) { console.log(value); } `, From cb20a7fdb8b6cf8bf351ea8cf456a039f2df125c Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 17:10:29 -0600 Subject: [PATCH 03/10] better docs and stuff --- .../docs/rules/await-thenable.mdx | 20 ++++++++++++++----- .../eslint-plugin/src/rules/await-thenable.ts | 10 ++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/await-thenable.mdx b/packages/eslint-plugin/docs/rules/await-thenable.mdx index 1378e8470b5b..0f881b234b7c 100644 --- a/packages/eslint-plugin/docs/rules/await-thenable.mdx +++ b/packages/eslint-plugin/docs/rules/await-thenable.mdx @@ -42,8 +42,20 @@ 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), which are designed for iterating over async-iterable objects. -If the value being iterated over is not async-iterable, an ordinary `for...of` statement is preferable, even if the value is an iterable of Thenables. +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 it is valid JavaScript to use `for await...of` with synchronous iterables (it falls back to synchronous iteration), it is inadvisable to do so. +Synchronous iteration over thenables and asynchronous iteration are fundamentally different things, and the `for await...of` has several behaviors that are specifically designed for operating on async-iterables, that do not apply to sync-iterables. + +For one, the `for await...of` loop accesses its fulfilled values sequentially, which is an appropriate behavior for async-iterables, but is not always desirable for sync-iterables of Thenables. + +Because of this, the error-handling of the `for await...of` loop works properly for async-iterables, but subtly permits unclosed generators and unhandled promise rejections when operating on sync-iterables of Thenables. 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). + +Instead of using the `for await...of` loop with sync-iterables of Thenables, or using `await` on promises within the body of a `for...of` (which also permits floating promise rejections), 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 robust behavior. + +::: ### Examples @@ -87,9 +99,7 @@ async function syncIterableOfPromises() { Promise.resolve(2), Promise.resolve(3), ]; - for (const promise of arrayOfPromises) { - // Note that the promise must be explicitly awaited if the promised value is needed. - const promisedValue = await promise; + for (const promisedValue of await Promise.all(arrayOfPromises)) { console.log(promisedValue); } } diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index 707ad47580ef..ecfc5c526ff9 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -97,6 +97,16 @@ export default createRule<[], MessageId>({ loc: getForStatementHeadLoc(context.sourceCode, node), messageId: 'forAwaitOfNonThenable', suggest: [ + // This suggestion causes broken code for sync iterables of promises, since + // the loop variable will not be awaited. + // + // Ideally, if the iterable yields promises, we would offer a suggestion to + // fix the for loop to `for (const value of await Promise.all(iterable))`. + // However, I don't think we can do that with the TS API for now, since we + // don't have access to `getIterationTypesOfType` or similar. + // + // If that becomes available to us, we should provide an alternate suggestion + // to fix the code to `for (const value of await Promise.all(iterable))` { messageId: 'convertToOrdinaryFor', fix(fixer): TSESLint.RuleFix { From 03517492b0e3fb9872755a00cb2e24ccaa9dc813 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 17:14:02 -0600 Subject: [PATCH 04/10] tweak --- packages/eslint-plugin/docs/rules/await-thenable.mdx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/await-thenable.mdx b/packages/eslint-plugin/docs/rules/await-thenable.mdx index 0f881b234b7c..5bd55f884889 100644 --- a/packages/eslint-plugin/docs/rules/await-thenable.mdx +++ b/packages/eslint-plugin/docs/rules/await-thenable.mdx @@ -50,8 +50,9 @@ While it is valid JavaScript to use `for await...of` with synchronous iterables Synchronous iteration over thenables and asynchronous iteration are fundamentally different things, and the `for await...of` has several behaviors that are specifically designed for operating on async-iterables, that do not apply to sync-iterables. For one, the `for await...of` loop accesses its fulfilled values sequentially, which is an appropriate behavior for async-iterables, but is not always desirable for sync-iterables of Thenables. +For instance, sync-iterables may need to manipulate their Thenable entries directly, without accessing their fulfillment values at all (that is to say, without awaiting them). -Because of this, the error-handling of the `for await...of` loop works properly for async-iterables, but subtly permits unclosed generators and unhandled promise rejections when operating on sync-iterables of Thenables. 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). +Because of this, the error-handling protocol of the `for await...of` loop works properly for async-iterables, but subtly permits unclosed generators and unhandled promise rejections when operating on sync-iterables of Thenables. 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). Instead of using the `for await...of` loop with sync-iterables of Thenables, or using `await` on promises within the body of a `for...of` (which also permits floating promise rejections), 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 robust behavior. From a19dc08ed1738e3b1a52396fbc3178e79fbae8d6 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 17 Sep 2024 19:48:26 -0600 Subject: [PATCH 05/10] change comment --- .../eslint-plugin/src/rules/await-thenable.ts | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index ecfc5c526ff9..e1e4dd688b12 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -30,7 +30,7 @@ export default createRule<[], MessageId>({ messages: { await: 'Unexpected `await` of a non-Promise (non-"Thenable") value.', forAwaitOfNonThenable: - 'Unexpected `for await...of` of a value that is not AsyncIterable.', + 'Unexpected `for await...of` of a value that is not async iterable.', removeAwait: 'Remove unnecessary `await`.', convertToOrdinaryFor: 'Convert to an ordinary `for...of` loop.', }, @@ -89,24 +89,13 @@ export default createRule<[], MessageId>({ checker, ); - // if there is an async iterator symbol, but it doesn't have the correct - // shape, TS will report a type error, so we only need to check if the - // symbol exists. if (asyncIteratorSymbol == null) { context.report({ loc: getForStatementHeadLoc(context.sourceCode, node), messageId: 'forAwaitOfNonThenable', suggest: [ - // This suggestion causes broken code for sync iterables of promises, since - // the loop variable will not be awaited. - // - // Ideally, if the iterable yields promises, we would offer a suggestion to - // fix the for loop to `for (const value of await Promise.all(iterable))`. - // However, I don't think we can do that with the TS API for now, since we - // don't have access to `getIterationTypesOfType` or similar. - // - // If that becomes available to us, we should provide an alternate suggestion - // to fix the code to `for (const value of await Promise.all(iterable))` + // 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 { From b3aafe66bb7ce193f0dbe057837e8148960dc560 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 19 Sep 2024 17:06:34 -0600 Subject: [PATCH 06/10] await=true Co-authored-by: Joshua Chen --- packages/eslint-plugin/src/rules/await-thenable.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index e1e4dd688b12..e69359bd2926 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -73,7 +73,7 @@ export default createRule<[], MessageId>({ } }, - ForOfStatement(node): void { + 'ForOfStatement[await=true]'(node): void { if (!node.await) { return; } From 128c7a119ab2d6534e28654a0ae17a0b906c3a24 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 19 Sep 2024 17:11:13 -0600 Subject: [PATCH 07/10] type and remove unnecessary condition --- packages/eslint-plugin/src/rules/await-thenable.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index e69359bd2926..05e47d744897 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 { @@ -73,11 +73,7 @@ export default createRule<[], MessageId>({ } }, - 'ForOfStatement[await=true]'(node): void { - if (!node.await) { - return; - } - + 'ForOfStatement[await=true]'(node: TSESTree.ForOfStatement): void { const type = services.getTypeAtLocation(node.right); if (isTypeAnyType(type) || isTypeUnknownType(type)) { return; From 89a6ab8f4882a4a30d067a2de69ad09c66c802c2 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 19 Sep 2024 17:16:17 -0600 Subject: [PATCH 08/10] Use Josh Cena's writeup instead Co-authored-by: Joshua Chen --- .../eslint-plugin/docs/rules/await-thenable.mdx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/await-thenable.mdx b/packages/eslint-plugin/docs/rules/await-thenable.mdx index 5bd55f884889..f9905bfcf80a 100644 --- a/packages/eslint-plugin/docs/rules/await-thenable.mdx +++ b/packages/eslint-plugin/docs/rules/await-thenable.mdx @@ -46,15 +46,14 @@ This rule also inspects [`for await...of` statements](https://developer.mozilla. :::info[Why does the rule report on `for await...of` loops used on an array of Promises?] -While it is valid JavaScript to use `for await...of` with synchronous iterables (it falls back to synchronous iteration), it is inadvisable to do so. -Synchronous iteration over thenables and asynchronous iteration are fundamentally different things, and the `for await...of` has several behaviors that are specifically designed for operating on async-iterables, that do not apply to sync-iterables. +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. -For one, the `for await...of` loop accesses its fulfilled values sequentially, which is an appropriate behavior for async-iterables, but is not always desirable for sync-iterables of Thenables. -For instance, sync-iterables may need to manipulate their Thenable entries directly, without accessing their fulfillment values at all (that is to say, without awaiting them). +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). -Because of this, the error-handling protocol of the `for await...of` loop works properly for async-iterables, but subtly permits unclosed generators and unhandled promise rejections when operating on sync-iterables of Thenables. 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). - -Instead of using the `for await...of` loop with sync-iterables of Thenables, or using `await` on promises within the body of a `for...of` (which also permits floating promise rejections), 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 robust behavior. +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. ::: From de03f50bb529404dabdcdd3ae8a11ca5cc6c8069 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 19 Sep 2024 17:32:49 -0600 Subject: [PATCH 09/10] snapshot --- .../docs-eslint-output-snapshots/await-thenable.shot | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 81b468a75dab..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 @@ -28,7 +28,7 @@ exports[`Validating rule docs await-thenable.mdx code examples ESLint output 3`] async function syncIterable() { const arrayOfValues = [1, 2, 3]; for await (const value of arrayOfValues) { - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unexpected \`for await...of\` of a value that is not AsyncIterable. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unexpected \`for await...of\` of a value that is not async iterable. console.log(value); } } @@ -40,7 +40,7 @@ async function syncIterableOfPromises() { Promise.resolve(3), ]; for await (const promisedValue of arrayOfPromises) { - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unexpected \`for await...of\` of a value that is not AsyncIterable. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unexpected \`for await...of\` of a value that is not async iterable. console.log(promisedValue); } } @@ -63,9 +63,7 @@ async function syncIterableOfPromises() { Promise.resolve(2), Promise.resolve(3), ]; - for (const promise of arrayOfPromises) { - // Note that the promise must be explicitly awaited if the promised value is needed. - const promisedValue = await promise; + for (const promisedValue of await Promise.all(arrayOfPromises)) { console.log(promisedValue); } } From 7cd8ba7060d4ba2b03c73f6aafd50eb31df15919 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 30 Sep 2024 10:09:10 -0600 Subject: [PATCH 10/10] cov --- packages/eslint-plugin/src/rules/await-thenable.ts | 2 +- .../eslint-plugin/tests/rules/await-thenable.test.ts | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index 05e47d744897..4c5a2e6dd171 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -75,7 +75,7 @@ export default createRule<[], MessageId>({ 'ForOfStatement[await=true]'(node: TSESTree.ForOfStatement): void { const type = services.getTypeAtLocation(node.right); - if (isTypeAnyType(type) || isTypeUnknownType(type)) { + if (isTypeAnyType(type)) { return; } diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index 2093bf08a333..7d88bea824a0 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -207,6 +207,16 @@ async function* asyncYieldNumbers() { } 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); + } } `, },