-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Rule proposal: no-misused-promises #508
Comments
I would call the first example "invalid" because that code will clearly fail when actually used, because the types will not match up. Though if they're using async/await very very wrong, then I can understand it: async const myFunction(...args: string[]) {
args.forEach(async arg => await doSomething(arg));
console.log('Done!');
} Another use I can understand is: async function something() {}
if (something()) {
console.log("I forgot to await");
} This looks like it'd be very specific to promises, but I'm open to it. One thing to note is that latter example requires type information, but the former doesn't. |
Yeah sorry I was mostly trying to demonstrate the type aspect of it. The example you provided with an async function is more akin to what I generally see people do by mistake. They are definitely misuses of async/await, but I actually see a decent number of these issues for new JS/TS developers. The boolean check of a promise is actually another example discussed in the linked tslint issue. It was proposed there that it could be addressed by adding an option to
While there are cases that can be caught without type information, I think a rule for the function return case becomes much more powerful with it. From a type perspective, any function returning a thenable value which is provided as an argument for a function parameter with return type void should probably be flagged. Without type information, you have to pick specific problematic functions (e.g. |
Coming back around to this, having a single rule to represent both of the cases described in the conversation above is feeling more reasonable to me. Either rule by itself would be very specific to a single use case and the "returning a promise in a parameter with void return type" one is really hard to succintly describe in a rule. The broader intent of avoiding promises being accidentally provided in places where they probably don't belong seems like a better umbrella goal. With that, here's my thinking. We add a
Since there will be cases where people want to have only a subset of these, the rule can be configured as such (default shown, tweak by setting the {
"@typescript-eslint/no-misused-promises": [
"error",
{
"checks": ["conditional", "void-return"]
}
]
} If this sounds reasonable, I can work to put together a PR in the next few days. |
The "No promise conditionals" part the previous comment was already proposed in #365. |
While
no-floating-promises
can be used to catch cases where a promise stands by itself in a statement, it does not help for situations where the promise is passed to code that will not handle it. Example:This example will compile because Typescript explicitly allows returning any value to a function expecting a
void
return. While this is alright for most values, if the returned value is a Promise that rejects, it will almost certainly be unhandled.This rule would also handle a situation that I encounter distressingly often with people learning
async
/await
and promises:There was previously a discussion about this kind of situation in palantir/tslint#3983, which seemed to garner support, but never made it into an actual rule. I have borrowed the rule name from that issue for now, but I actually think it would be better to name it something more specific as I think it makes sense to implement the parameter return type portion of that proposal as its own rule.
The text was updated successfully, but these errors were encountered: