Skip to content

Enhancement: [no-throw-literal] option to also check promise rejections #7673

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

Closed
4 tasks done
DetachHead opened this issue Sep 19, 2023 · 4 comments
Closed
4 tasks done
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@DetachHead
Copy link

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/no-throw-literal

Description

a promise rejected with a non-Error value has the same problems as a throw statement with a non-Error value, so it should also check promise rejections

Fail

Promise.reject('asdf')
new Promise((_, reject) => reject('asdf'))

Pass

Promise.reject(new Error('asdf'))
new Promise((_, reject) => reject(new Error('asdf')))

Additional Info

No response

@DetachHead DetachHead added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Sep 19, 2023
@bradzacher
Copy link
Member

Thanks for filing!
This is something that should be proposed upstream as this is an extension rule.
We don't look to break from the checks that the base rule does - only enrich them with type information.

Basic checks for this would not require type information - promise constructors are easy to handle syntactically and they have rules that do this already (eg no-async-promise-executor)

If they accept it and it is implemented - then we can similarly implement it along with the type-aware enhancements. If they do not accept it - then we shan't either.

I will close this for now. If it is accepted upstream before the auto-lock window, feel free to comment and we can reopen and mark it as blocked upstream. When it is implemented we can mark it as accepting PRs.

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2023
@bradzacher bradzacher added external This issue is with another package, not typescript-eslint itself and removed triage Waiting for team members to take a look labels Sep 20, 2023
@DetachHead
Copy link
Author

raised eslint/eslint#17592

@DetachHead
Copy link
Author

@bradzacher turns out there's already an upstream rule for it https://eslint.org/docs/latest/rules/prefer-promise-reject-errors

can we re-open this issue or would you rather i raise a new one?

@DetachHead
Copy link
Author

raised separate issue: #7687

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

2 participants