From e2957f545078d7dfe4b6cd435fd0ca9ae072724a Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Fri, 29 Nov 2024 14:41:44 -0700 Subject: [PATCH 1/3] make return-await standalone --- .../eslint-plugin/docs/rules/return-await.mdx | 39 +++++++++++++++++-- .../src/configs/disable-type-checked.ts | 2 +- .../src/configs/strict-type-checked-only.ts | 2 +- .../eslint-plugin/src/rules/return-await.ts | 1 - .../src/configs/disable-type-checked.ts | 2 +- .../src/configs/strict-type-checked-only.ts | 2 +- .../src/configs/strict-type-checked.ts | 2 +- tools/scripts/generate-configs.mts | 4 ++ 8 files changed, 44 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/return-await.mdx b/packages/eslint-plugin/docs/rules/return-await.mdx index 34173fae9265..abdaecd67be4 100644 --- a/packages/eslint-plugin/docs/rules/return-await.mdx +++ b/packages/eslint-plugin/docs/rules/return-await.mdx @@ -9,15 +9,46 @@ import TabItem from '@theme/TabItem'; > > See **https://typescript-eslint.io/rules/return-await** for documentation. -This rule builds on top of the [`eslint/no-return-await`](https://eslint.org/docs/rules/no-return-await) rule. -It expands upon the base rule to add support for optionally requiring `return await` in certain cases. +In `async` functions, it is valid to return a promise-wrapped value or a value directly, both of which ultimately produce a promise with the same fulfillment value. -The extended rule is named `return-await` instead of `no-return-await` because the extended rule can enforce the positive or the negative. Additionally, while the core rule is now deprecated, the extended rule is still useful in many contexts: +```ts +async function foo(): Promise { + // value + return 42; +} +foo().then(x => console.log(x)); // logs 42 + +async function bar(): Promise { + // promise-wrapped value + return Promise.resolve(42); +} +bar().then(x => console.log(x)); // logs 42 + +async function baz(): Promise { + // value, since `await Promise.resolve(42)` is 42 + return await Promise.resolve(42); +} +baz().then(x => console.log(x)); // logs 42 +``` + +Returning a value rather than a promise-wrapped value can have several subtle benefits: - Returning an awaited promise [improves stack trace information](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await#improving_stack_trace). -- When the `return` statement is in `try...catch`, awaiting the promise also allows the promise's rejection to be caught instead of leaving the error to the caller. +- When the `return` statement is in `try...catch`, awaiting the promise allows the promise's rejection to be caught instead of leaving the error to the caller. - Contrary to popular belief, `return await promise;` is [at least as fast as directly returning the promise](https://github.com/tc39/proposal-faster-promise-adoption). +This rule enforces consistent handling of whether to await promises before returning them. + +:::info + +This rule used to be considered an extension of the (now-deprecated) ESLint core rule [`no-return-await`](https://eslint.org/docs/latest/rules/no-return-await#options). +Without type information, the only situations that could be flagged by `no-return-await` were of neutral-to-negative value, which eventually led to its deprecation. +In contrast, with access to type information, `@typescript-eslint/return-await` delivers enough positive value to earn it a spot in our strict preset. + +If you previously used `no-return-await`, this rule's `in-try-catch` option has the closest behavior to the `no-return-await` rule. + +::: + ## Options ```ts diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 061df20cdd65..95efeca0cd5b 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -8,7 +8,7 @@ import type { ClassicConfig } from '@typescript-eslint/utils/ts-eslint'; export = { - parserOptions: { project: false, program: null, projectService: false }, + parserOptions: { program: null, project: false, projectService: false }, rules: { '@typescript-eslint/await-thenable': 'off', '@typescript-eslint/consistent-return': 'off', diff --git a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts index c235c02e7b81..e5be4357f195 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts @@ -61,10 +61,10 @@ export = { { allowAny: false, allowBoolean: false, + allowNever: false, allowNullish: false, allowNumber: false, allowRegExp: false, - allowNever: false, }, ], 'no-return-await': 'off', diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 18cc6e9e420a..a040bbb9463e 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -37,7 +37,6 @@ export default createRule({ type: 'problem', docs: { description: 'Enforce consistent awaiting of returned promises', - extendsBaseRule: 'no-return-await', recommended: { strict: ['error-handling-correctness-only'], }, diff --git a/packages/typescript-eslint/src/configs/disable-type-checked.ts b/packages/typescript-eslint/src/configs/disable-type-checked.ts index eeb80399882c..8ab7a8df71f6 100644 --- a/packages/typescript-eslint/src/configs/disable-type-checked.ts +++ b/packages/typescript-eslint/src/configs/disable-type-checked.ts @@ -76,6 +76,6 @@ export default ( '@typescript-eslint/use-unknown-in-catch-callback-variable': 'off', }, languageOptions: { - parserOptions: { project: false, program: null, projectService: false }, + parserOptions: { program: null, project: false, projectService: false }, }, }); diff --git a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts index c9efc88da6c3..ef29e1006bc4 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked-only.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked-only.ts @@ -74,10 +74,10 @@ export default ( { allowAny: false, allowBoolean: false, + allowNever: false, allowNullish: false, allowNumber: false, allowRegExp: false, - allowNever: false, }, ], 'no-return-await': 'off', diff --git a/packages/typescript-eslint/src/configs/strict-type-checked.ts b/packages/typescript-eslint/src/configs/strict-type-checked.ts index 4f354baf7e40..e5c7c4c22062 100644 --- a/packages/typescript-eslint/src/configs/strict-type-checked.ts +++ b/packages/typescript-eslint/src/configs/strict-type-checked.ts @@ -107,10 +107,10 @@ export default ( { allowAny: false, allowBoolean: false, + allowNever: false, allowNullish: false, allowNumber: false, allowRegExp: false, - allowNever: false, }, ], 'no-return-await': 'off', diff --git a/tools/scripts/generate-configs.mts b/tools/scripts/generate-configs.mts index f7887774df0b..010227cefc92 100644 --- a/tools/scripts/generate-configs.mts +++ b/tools/scripts/generate-configs.mts @@ -89,6 +89,10 @@ async function main(): Promise { ), ); + // special case - return-await used to be an extension, but no longer is. + // See https://github.com/typescript-eslint/typescript-eslint/issues/9517 + BASE_RULES_TO_BE_OVERRIDDEN.set('return-await', 'no-return-await'); + type RuleEntry = [string, ESLintPluginRuleModule]; const allRuleEntries: RuleEntry[] = Object.entries(eslintPlugin.rules).sort( From fdd89ccad39b4fcf42a8c1188f978205c77dcb1d Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Fri, 29 Nov 2024 15:05:56 -0700 Subject: [PATCH 2/3] when not to use it --- packages/eslint-plugin/docs/rules/return-await.mdx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/return-await.mdx b/packages/eslint-plugin/docs/rules/return-await.mdx index abdaecd67be4..eb061d93779e 100644 --- a/packages/eslint-plugin/docs/rules/return-await.mdx +++ b/packages/eslint-plugin/docs/rules/return-await.mdx @@ -357,3 +357,5 @@ async function validNever3() { + +{/* Intentionally Omitted: When Not To Use It */} From 456eef19762d36c32feaaa1d1feebb4aece2a388 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 30 Dec 2024 18:54:14 -0700 Subject: [PATCH 3/3] terser --- .../eslint-plugin/docs/rules/return-await.mdx | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/return-await.mdx b/packages/eslint-plugin/docs/rules/return-await.mdx index eb061d93779e..db4f2f5fe5b7 100644 --- a/packages/eslint-plugin/docs/rules/return-await.mdx +++ b/packages/eslint-plugin/docs/rules/return-await.mdx @@ -9,29 +9,7 @@ import TabItem from '@theme/TabItem'; > > See **https://typescript-eslint.io/rules/return-await** for documentation. -In `async` functions, it is valid to return a promise-wrapped value or a value directly, both of which ultimately produce a promise with the same fulfillment value. - -```ts -async function foo(): Promise { - // value - return 42; -} -foo().then(x => console.log(x)); // logs 42 - -async function bar(): Promise { - // promise-wrapped value - return Promise.resolve(42); -} -bar().then(x => console.log(x)); // logs 42 - -async function baz(): Promise { - // value, since `await Promise.resolve(42)` is 42 - return await Promise.resolve(42); -} -baz().then(x => console.log(x)); // logs 42 -``` - -Returning a value rather than a promise-wrapped value can have several subtle benefits: +In `async` functions, it is valid to return a promise-wrapped value or a value directly, both of which ultimately produce a promise with the same fulfillment value. Returning a value rather than a promise-wrapped value can have several subtle benefits: - Returning an awaited promise [improves stack trace information](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await#improving_stack_trace). - When the `return` statement is in `try...catch`, awaiting the promise allows the promise's rejection to be caught instead of leaving the error to the caller.