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

💅 useAwait should be type-aware and require async for functions that return a promise #1161

Closed
1 task done
lensbart opened this issue Dec 12, 2023 · 5 comments
Closed
1 task done

Comments

@lensbart
Copy link

lensbart commented Dec 12, 2023

Hello,

First of all, thanks for maintaining this project. It’s amazingly fast. I hope to one day learn some Rust so that I can contribute.

The useAwait that’s currently in the nursery, seems to be modeled after ESLint’s require-await (judging by #575). I think that in general, it’s better to mimic the behaviour of an equivalent typescript-eslint rule, if available, rather than the (non-type-aware) eslint rule.

In this case, for example, useAwait will flag functions that are marked as async if they don’t contain the await keyword, even if the function returns a promise. In constrast, @typescript-eslint/require-await (or is it @typescript-eslint/promise-function-async?) explicitly adds async in this case, to inform the developer, since the return value must still be awaited.

I’d propose to follow this behaviour in the case of useAwait as well.

Relevant links:

Rule name

useAwait

Playground link

https://biomejs.dev/playground/?lintRules=all&code=YQBzAHkAbgBjACAAZgB1AG4AYwB0AGkAbwBuACAAcgBlAHQAdQByAG4AcwBQAHIAbwBtAGkAcwBlADEAKAApACAAewAKACAAIAByAGUAdAB1AHIAbgAgAFAAcgBvAG0AaQBzAGUALgByAGUAcwBvAGwAdgBlACgAMQApADsACgB9AAoACgBjAG8AbgBzAHQAIABfAHIAZQB0AHUAcgBuAHMAUAByAG8AbQBpAHMAZQAyACAAPQAgACgAKQAgAD0APgAgAHIAZQB0AHUAcgBuAHMAUAByAG8AbQBpAHMAZQAxACgAKQA7AAoACgA%3D

Expected result

No error for functions marked as async that don’t contain await but do return a promise

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@ematipico
Copy link
Member

@lensbart Biome doesn't have the infrastructure yet to deduce types, so it's impossible at the moment to match the rule from TypeScript Eslint.

Unless there's an issue with the current logic of the rule, I advise to close the issue.

We will soon publish more info about this topic.

@lensbart
Copy link
Author

lensbart commented Dec 12, 2023

@ematipico thanks for the quick response! I’m fine with closing this ticket, but will add some additional context for future reference.

I think the ideal setup uses a combination of require-await/useAwait, promise-function-async and no-floating-promises to prevent e.g. forgetting to return the result of an async operation, or “throwing away” (ignoring) the return value of an async operation.

The suggested approach would then be to use void to explicitly ignore the return value.

const problematic1 = () => {
  db.getUsers() // @typescript-eslint/no-floating-promises
}
const problematic2 = () => { // @typescript-eslint/promise-function-async
  return db.getUsers()
}
const problematic3 = async () => {
  const users = db.getUsers() // @typescript-eslint/require-await
}

const okay1 = async () => {
  return db.getUsers()
}
const okay2 = async () => {
  void db.getUsers() // contrived example but just for the sake of argument
}

@mctrafik
Copy link

+1 this feature request.

The biggest reason for it in my company that eslint is slow is because it does type inference and we only need it for this one rule: https://typescript-eslint.io/rules/no-floating-promises/. Without it, every few months someone introduces a race condition and we spend a week debugging it. Blocker for us using from fully switching to biome.

@john-griffin
Copy link

Yeah we would really like no-floating-promises too.

@jpenna

This comment has been minimized.

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

No branches or pull requests

5 participants