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

Docs: no-misused-promises example for "if promise exists" #9066

Closed
2 tasks done
Tezra opened this issue May 9, 2024 · 3 comments
Closed
2 tasks done

Docs: no-misused-promises example for "if promise exists" #9066

Tezra opened this issue May 9, 2024 · 3 comments
Labels
awaiting response Issues waiting for a reply from the OP or another party documentation Documentation ("docs") that needs adding/updating

Comments

@Tezra
Copy link

Tezra commented May 9, 2024

Before You File a Documentation Request Please Confirm You Have Done The Following...

Suggested Changes

The documentation for "no-misused-promises" doesn't provide any example of how to check that a promise itself exists. That is, when if (promise) was in fact intended.

The standard correct way to check seems to be to check that an object is "thanable", so if (promise?.then). (At the very least it is much clearer about intention)

The docs should be updated with examples/guidance for the case where "if promise is defined" was the intended logic.

Affected URL(s)

https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-misused-promises.mdx

@Tezra Tezra added documentation Documentation ("docs") that needs adding/updating triage Waiting for maintainers to take a look labels May 9, 2024
@auvred
Copy link
Member

auvred commented May 9, 2024

That is, when if (promise) was in fact intended.

This makes sense only when promise is nullable. And that case is currently handled by the rule. In the example below, if (maybePromise) is not reported:

declare const maybePromise: Promise<void> | undefined
if (maybePromise) { }
//  ^?

declare const promise: Promise<void>
if (promise) { }
//  ^?

playground

Could you provide an example where the if (promise) required for non-nullable promise?

@auvred auvred added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look labels May 9, 2024
@Tezra
Copy link
Author

Tezra commented May 9, 2024

On looser typescript settings, the nullable bit isn't required for it to be nullable. I am ok with adding the nullable type being an example of how to fix the issue, even though that makes the case of Promise<boolen> | undefined unclear on whether the if is meant to be for the value or the promise. The rule works as intended; this suggestion is just for adding explicit examples/guidance to users for how to handle that use case without using eslint-disable.

@bradzacher
Copy link
Member

bradzacher commented May 10, 2024

On looser typescript settings

In general we don't support "looser" settings and assume you have at least strictNullChecks turned on. Because if you don't have it turned on then your code is provably unsound. Because turning it on is the industry baseline.

Our rules are powered by the types - so if your types are unsound then the rules are in an undefined state. It's usually impossible for us to report "correctly" because things like null can be removed from unions.
How can we possibly understand your code if your types are incorrect?

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP or another party documentation Documentation ("docs") that needs adding/updating
Projects
None yet
Development

No branches or pull requests

3 participants