chore: enable unicorn/no-array-reduce #9640
Conversation
|
Thanks for the PR, @abrahamguo! 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. |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 6b79534. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
|
Any refactors that look like have been removed, and either restored to use |
| (['public', 'protected', 'private', '#private'] as const).forEach( | ||
| accessibility => { | ||
| if ( | ||
| const allMemberTypes = [ |
There was a problem hiding this comment.
Wow this is a giant one. Definitely an improvement.
| const MAX_RULE_NAME_LENGTH = Math.max( | ||
| ...Object.keys(eslintPlugin.rules).map(name => name.length), | ||
| ); |
There was a problem hiding this comment.
I believe the original intention is that argument count must be <65536. Not sure it can be hit here 😅
| ], | ||
| }, | ||
| // AssignmentExpression | ||
| ...(skipAssignmentExpression |
There was a problem hiding this comment.
still think these types of cases should be explicit like below rather than using lots of ... operator to conditionally push within an expression.
const mappedTests = [
// ... lots of cases
]
if (skipAssignmentExpression !== true) {
mappedTests.push(/* etc */);
}(also note that skipAssignmentExpression !== true isn't equivalent to if (!skipAssignmentExpression) here - let's preserve that).
1216fee
PR Checklist
eslint-plugin-unicornrules internally #9623Overview
Enable
unicorn/no-array-reduce(docs).Note that
no-array-reducedoes allow "simple operations", which it defines as a body that is only aBinaryExpression, so those were left unchanged.I strongly feel that every place that was refactored has better readability than it did before.
In most places, there was a more appropriate method to use, such as
Array.flatMaporObject.fromEntries.In other places, it was unnecessarily confusing to return the accumulator every iteration when the accumulator never changes, and so it was better to use a
constvariable in conjunction with afor..ofloop.I recommend selecting
Hide whitespacewhen looking at this diff 😄