-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[no-misused-promises] non-thenable thenable union react component attribute does not accept thenable function #4650
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
Comments
Wasn't this fixed in #4620, @JoshuaKGoldberg? |
To me it rather looks like #4620 introduced this bug. |
Indeed - this is another edge case I hadn't thought of. 😞 |
What would be the right way forward? Just wonder if I should refactor my React callbacks or temporarily disable the rule. Downgrading |
With any issue in opened in this project - it either has a visible progress in the form of an attached PR, or it has no progress. We are a community run project. The volunteer maintainers spend most of their time triaging issues and reviewing PRs. This means that most issues will not progress unless a member of the community steps up and champions it. If this issue is important to you - consider being that champion. If not - please just subscribe to the issue and wait patiently. |
Thanks for your comment @bradzacher and sorry for spamming – I did not mean to. The reason why I asked for some clarification was because I was not sure what the resolution was. Should #4620 be reverted? Or should there be some new Because #4620 is a breaking change in a minor version, my intuition would be to revert it in the next patch and think about a smart solution to the problem later. Once the way forward is clear, I’m happy to try submitting PR! |
No - we aren't in the business of reverting commits unless they create catastrophic breakages. Given this issue is almost a month old with less than 10 users interactions - it's definitely not catastrophic. This issue is tagged with "accepting PRs" - the way forward is to fix the bug.
Lint rule enhancements can cause additional lint errors to be revealed in your codebase. This is not a breaking change as we define it. Reviewing the issue again - the lint rule is actually working just fine here. You're passing a function Component({onClick}: {onClick: () => void}) {
return <div onClick={() => {
onClick();
doSomethingImmediately();
}} />;
}
<Component onClick={async () => { throw new Error() }} />;
// the error gets lost to the void. #4623 added a way for you to opt-out of this behaviour granularly using rule config, in case you want to allow the unsafe behaviour: { checksVoidReturns: { attributes: false } } |
@bradzacher From my reading of this issue I don't think your explanation is quite correct. In this issue you're passing a |
@esetnik - I think you've misunderstood the OP. Passing |
Note that
|
@bradzacher just wanted to let you know I tested in v5.19.0 and it's still generating the spurious error. I believe this issue needs to be reopened. |
I agree, this issue is still persisting. |
Fix in review here: #4841 Since it's a bit buried in the thread, bumping that you can disable the new check added in v4.6 with the settings described at https://typescript-eslint.io/rules/no-misused-promises/#checksvoidreturn: "rules": {
"@typescript-eslint/no-misused-promises": ["error", {
"checksVoidReturn": {
"arguments": false
}
}]
} |
@JoshuaKGoldberg I see your fix and that makes a lot of sense. Just curious, do you have any idea why I'm not able to demonstrate this error in the playground? |
I had to use: "rules": {
"@typescript-eslint/no-misused-promises": ["error", {
"checksVoidReturn": {
"arguments": false,
"attributes": false
}
}]
} to silence all the situations affected by #4650 |
Repro
Expected Result
No linting error since
onEvent
accepts both a thenable and a non-thenable function.Actual Result
Additional Information
This bug was introduced in
5.14.0
.5.13.0
ist working fine.Note that you can circumvent the bug by using
instead.
Versions
@typescript-eslint/eslint-plugin
5.14.0
@typescript-eslint/parser
5.14.0
TypeScript
4.6.2
ESLint
8.10.0
node
14.18.3
The text was updated successfully, but these errors were encountered: