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

Suggestion: Treat async without await as an error #22024

Closed
AlCalzone opened this issue Feb 18, 2018 · 5 comments
Closed

Suggestion: Treat async without await as an error #22024

AlCalzone opened this issue Feb 18, 2018 · 5 comments
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints

Comments

@AlCalzone
Copy link
Contributor

Search Terms: async, await, __awaiter

In one of my codebases, a bunch of functions that were originally using async/await were refactored to simply return a promise instead of awaiting it. During that refactor, I forgot to remove the async keyword from some of the refactored functions. This was no problem, except for one user who constantly got TypeError: Generator already running when starting the application via child_process.fork().

After removing the unnecessary async keywords from these functions, his problem was gone. IMO the compiler should report an error when an async function is missing the await - just like using await without async is an error.

  • typescript: 2.6 / 2.7
  • target: "es2015"

Code

declare function foo();
async function bar() {
    foo();
}

Expected behavior:
Error: Async functions should contain the "await" keyword at least once.

Actual behavior:
No error.

Playground Link: https://www.typescriptlang.org/play/#src=%2F%2F%20this%20is%20fine%3A%0D%0Adeclare%20function%20foo()%3B%0D%0Aasync%20function%20bar()%20%7B%0D%0A%20%20%20%20foo()%3B%0D%0A%7D%0D%0A%0D%0A%2F%2F%20this%20is%20not%3A%0D%0Afunction%20baz()%20%7B%0D%0A%20%20%20%20await%20foo()%3B%0D%0A%7D

Related Issues:
#13376

@ajafff
Copy link
Contributor

ajafff commented Feb 18, 2018

Having async functions without any await inside is totally valid and even useful in some cases. You can for example implicitly catch synchronously thrown exceptions and turn them into rejected promises.

Warning about this is not the business of the compiler. The is the domain of a linter. In fact there's already a request for such a rule in TSLint, but I still don't see the value of such a rule.

If there's a runtime error, this is either caused by a faulty runtime or there's a bug in the downlevel emit for the async function. But that would be a separate issue.

@AlCalzone
Copy link
Contributor Author

AlCalzone commented Feb 19, 2018

You can for example implicitly catch synchronously thrown exceptions and turn them into rejected promises.

Haven't considered this, but I usually dislike code that implicitly does stuff. Thus I can see the value of the rule you mentioned. It's along the lines of: "I know I'm only using async when I await stuff, now show me where I messed up".

either caused by a faulty runtime

=> NodeJS 6.11.4 on Windows

or there's a bug in the downlevel emit for the async function. But that would be a separate issue.

If that's the case, here's the relevant commit which fixed the issue: AlCalzone/node-tradfri-client@2dc3f72

@mhegazy
Copy link
Contributor

mhegazy commented Feb 20, 2018

We have discussed similar issues in the past, including #13376 and #13847. The conclusion here was that these are not always an error, and the compiler should not force these checks on everyone. Such checks should however be considered as a lint rules that users can opt-in and out from and disable for specific use instances.
For that i think this is currently out of scope of the compiler.

@mhegazy mhegazy added the Out of Scope This idea sits outside of the TypeScript language design constraints label Feb 20, 2018
@AlCalzone
Copy link
Contributor Author

Such checks should however be considered as a lint rules that users can opt-in and out from and disable for specific use instances.

I see, how would one go about getting a rule like this implemented? I don't have enough knowledge about the TS or TSLint internals to do so.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints
Projects
None yet
Development

No branches or pull requests

4 participants