Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change @typescript-eslint/return-await to in-try-catch? #130

Closed
achingbrain opened this issue Feb 3, 2023 · 2 comments · Fixed by #133
Closed

Change @typescript-eslint/return-await to in-try-catch? #130

achingbrain opened this issue Feb 3, 2023 · 2 comments · Fixed by #133
Labels

Comments

@achingbrain
Copy link
Member

achingbrain commented Feb 3, 2023

The default setting for @typescript-eslint/return-await is ['error', 'always'].

This disallows code like:

async function myAsyncFunction () {
  return someOtherAsyncFunction()
}

instead demanding:

async function myAsyncFunction () {
  return await someOtherAsyncFunction()
}

It doesn't seem to make any difference to the stack traces and we have other rules that catch floating promises so I'm not sure what value it adds, and it occasionally raises some eyebrows.

The rule comes from standard but it's not even clear that the standard maintainers are for this version of the rule.

It does have another mode which is "in-try-catch", this disallows this sort of thing, demanding and await before the return, which seems a bit more useful, as it looks like the author might think any rejection would be caught which is not the case:

try {
  return someOtherAsyncFunction()
} catch (err) {
  // phew!
}

Shall we switch this rule to "in-try-catch"?

Cc @whizzzkid @SgtPooki

achingbrain added a commit that referenced this issue Feb 28, 2023
… blocks

Returning unawaited promises from inside try/catch blocks is usually
a bug as the author more than likely intends the catch part to handle
errors.

The configuration in this PR disallows `await` before `return` unless
it is inside a `try/catch` in which case it is required.

`await` before `return` doesn't cause any harm but I don't think we
can configure the rule any other way.

This is a breaking change but one that is fixable with `npm run lint -- --fix`

Fixes: #130

BREAKING CHANGE: `await` is not allowed before `return` unless it is inside a `try/catch` in which case it is required
@SgtPooki
Copy link
Member

@achingbrain I'm for this

achingbrain added a commit that referenced this issue Apr 5, 2023
… blocks (#133)

Returning unawaited promises from inside try/catch blocks is usually
a bug as the author more than likely intends the catch part to handle
errors.

The configuration in this PR disallows `await` before `return` unless
it is inside a `try/catch` in which case it is required.

`await` before `return` doesn't cause any harm but I don't think we
can configure the rule any other way.

This is a breaking change but one that is fixable with `npm run lint -- --fix`

Fixes: #130

BREAKING CHANGE: `await` is not allowed before `return` unless it is inside a `try/catch` in which case it is required

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
achingbrain added a commit that referenced this issue Apr 5, 2023
… blocks (#133)

Returning unawaited promises from inside try/catch blocks is usually
a bug as the author more than likely intends the catch part to handle
errors.

The configuration in this PR disallows `await` before `return` unless
it is inside a `try/catch` in which case it is required.

`await` before `return` doesn't cause any harm but I don't think we
can configure the rule any other way.

This is a breaking change but one that is fixable with `npm run lint -- --fix`

Fixes: #130

BREAKING CHANGE: `await` is not allowed before `return` unless it is inside a `try/catch` in which case it is required

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
github-actions bot pushed a commit that referenced this issue Apr 5, 2023
## [4.0.0](v3.1.7...v4.0.0) (2023-04-05)

### ⚠ BREAKING CHANGES

* `await` is not allowed before `return` unless it is inside a `try/catch` in which case it is required

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
* previously jsx and tsx files were not linted, now they are
* add import sorting rule (#126)

### Features

* add import sorting rule ([#126](#126)) ([1462d09](1462d09))
* lint jsx ([#140](#140)) ([fb5e5b6](fb5e5b6))

### Bug Fixes

* only require `await`ing promises returned from inside try/catch blocks ([#133](#133)) ([a6e4a7b](a6e4a7b)), closes [#130](#130)

### Trivial Changes

* convert to aegir project ([81d8ba3](81d8ba3))
* **deps:** bump eslint-config-standard-with-typescript from 27.0.1 to 34.0.1 ([#135](#135)) ([fc9aa6f](fc9aa6f))
* **deps:** bump eslint-plugin-jsdoc from 39.9.1 to 40.1.1 ([#139](#139)) ([bd1422b](bd1422b))
* fix linting ([adffc58](adffc58))
* use unified ci ([2674c56](2674c56))
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants