-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin): [no-floating-promises] flag result of .map(async) #7897
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
feat(eslint-plugin): [no-floating-promises] flag result of .map(async) #7897
Conversation
Thanks for the PR, @kirkwaiblinger! 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 on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], | ||
}, | ||
{ | ||
code: ` |
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.
test case from the issue report
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.
The tests are great—I cannot devise new test cases. I'll also admit I don't completely understand the problem domain. Just a nit; the error message looks fine to me.
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.
packages/eslint-plugin/tests/rules/no-floating-promises.test.ts
Outdated
Show resolved
Hide resolved
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.
typescript-eslint#7897) * flag arrays of promises * remove comment * trying to trigger CI * Substantial simplification * add some grody test cases for generics * flip if/else * add detail to test case * switch to isArrayLikeType * Revert "switch to isArrayLikeType" This reverts commit 2afe794. * add checks for tuples * One more test case! * else if * Remove unnecessary whitespace * Remove invalid test case * Add to docs * Add test cases around array that's fine but tuple that isn't * tweaks
PR Checklist
.map(async)
result #5958Overview
This PR adds functionality to the rule to check for unawaited arrays of promises, where we might expect to use Promise.all/Promise.allSettled, etc to handle the result.
I haven't attempted to add editor suggestions for fixes because the fixes seem rather complicated, depending on how you ended up with an array of promises and what you want to do with it. For example, we might fix the above code with
This seems like enough of an edge case that maybe the user should be responsible for manually resolving it. Please advise if adding editor suggestion would be a requirement for this PR.