Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(eslint-plugin): [no-misused-promises] check more places for checksVoidReturn#4541
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nx-cloudbot commentedFeb 12, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for the PR,@uhyo! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day. |
netlifybot commentedFeb 12, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
❌ Deploy Preview fortypescript-eslint failed. 🔨 Explore the source changes:42701f5 🔍 Inspect the deploy log:https://app.netlify.com/sites/typescript-eslint/deploys/6217b76f95b0d100079e1af4 |
codecovbot commentedFeb 12, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #4541 +/- ##==========================================+ Coverage 94.62% 94.65% +0.03%========================================== Files 149 150 +1 Lines 8072 8162 +90 Branches 2581 2617 +36 ==========================================+ Hits 7638 7726 +88- Misses 239 241 +2 Partials 195 195
Flags with carried forward coverage won't be shown.Click here to find out more.
|
JoshuaKGoldberg commentedFeb 12, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Generally looks great to me so far, thanks for tackling all these cases@uhyo!
Requesting changes on the error message I commented on. Also, codecov (now that it's done changing its mind) is correct, there are some untested lines of code. Please do fill those in.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
@JoshuaKGoldberg Thank you for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry, one more bug? 😄
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🔥
thanks for the improvement@uhyo ! I'm wondering, is there a way to disable only the functionMyButton(){constonClick=async()=>{constb=awaitdoSomething();doSomethingElse(b);}return<buttononClick={onClick}>Clickme!</button>;} and now that error is being (correctly!) detected on those places. I know I could do |
As per our contributing guide - please don't comment on closed PRs. Instead please use issues to start new discussions and find existing ones like#4619 or#4609 |
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
The
checksVoidReturn: true
option only checks function arguments now. This PR extends the rule so that misused Promises at other places are checked too. Namely, variable assignments, object properties, return statements and JSX props are newly checked.See the added tests for examples of newly-incorrect code.