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

Stricter handling of promises/async functions #55791

Closed
5 tasks done
JavaScriptBach opened this issue Sep 19, 2023 · 6 comments
Closed
5 tasks done

Stricter handling of promises/async functions #55791

JavaScriptBach opened this issue Sep 19, 2023 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@JavaScriptBach
Copy link

JavaScriptBach commented Sep 19, 2023

πŸ” Search Terms

"promise", "dangling promise", "async", "floating promise"

βœ… Viability Checklist

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

I think it would be valuable for TypeScript to have an option enabling stricter checks on async functions and promises. Things like:

  1. Disallowing promises that aren't handled or awaited
  2. Disallowing async functions to stand in for functions that are clearly expected to be synchronous. A common bug we run into is passing async functions into Array.forEach():
async function foo() {
  ["foo.txt", "bar.txt"].forEach(async (path) {
    const contents = await fs.readFile(path);
  }); // returns immediately, instead of waiting until all files are read!
}

1) is currently handled by https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-floating-promises.md.

2) isn't handled by any common tooling that I'm aware of.

Promises and async/await are common patterns used in JavaScript, and there are a whole class of possible bugs that could be eliminated from stricter TypeScript checking. Some of these are currently handled by third-party linters like ESLint, but misuse of promises are so common, and so likely to be bugs, that I think they deserve to be brought into TypeScript as compile errors. Another benefit is that it prevents users from having to run 2 different tools, each constructing their own ASTs, to get compile-time safety.

To avoid breaking existing TypeScript code, we could gate this change behind a flag for users to decide when/if to opt in.

πŸ“ƒ Motivating Example

Stricter handling of promises to avoid writing bugs in async code.

Note these kind of checks exist in other statically typed languages and type checkers, such as:

πŸ’» Use Cases

What do you want to use this for?
Detect entire classes of common async bugs at compile time.
What shortcomings exist with current approaches?
ESLint has a no-floating-promises rule, but it's slow and sometimes has buggy false negatives.
What workarounds are you using in the meantime?
ESLint

@jcalz
Copy link
Contributor

jcalz commented Sep 19, 2023

  1. see async/await: nowait keyword?Β #13376
  2. see Warn on forEach(async Β #28595 (at least specifically for forEach())

@fatcerberus
Copy link

You should continue to use no-floating-promises. The TypeScript team will most likely reject this as it doesn't really have much to do with type checking and last time I heard they don't want to add new lint checks to the compiler.

@fatcerberus
Copy link

fatcerberus commented Sep 19, 2023

Hm, there appears to be more interest from the team than I suspected, however see discussion starting here, this is apparently a bigger can of worms than it looks at first: #13376 (comment)

Also: #13376 (comment)

@chriskrycho
Copy link

With the new daemon mode, the type-aware ESLint rule is a good move at minimum, but I agree that there’s a serious need here. It’s not in the usual correctness bucket, but in practice dangling promises are one of the major contributors to memory leaks, especially in cases where you expect a short-lived request on a server and "close" it by whatever means, but the promise is still dangling. (Ask me how I know, he says, shuddering to remember the first 4 months of the year.)

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Sep 20, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 23, 2023
@JavaScriptBach
Copy link
Author

@RyanCavanaugh can you clarify which issue(s) this is a duplicate of?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants