chore: enable unicorn/prefer-ternary
#9886
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. |
|
I thought from the issue that pretty much everyone were negative/ambivalent on this rule? I personally wouldn't want it enforced though I may enjoy its style. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9886 +/- ##
==========================================
- Coverage 88.12% 88.11% -0.02%
==========================================
Files 406 406
Lines 13951 13920 -31
Branches 4076 4073 -3
==========================================
- Hits 12294 12265 -29
+ Misses 1344 1342 -2
Partials 313 313
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Sure thing! I wanted to open a PR since I felt it might help to see the actual changes recommended and gather a bit more feedback as to whether it would be beneficial to enforce. I'm also open to removing the lint rule and keeping the changes, a la #9640 |
There was a problem hiding this comment.
I'm kind of surprised this doesn't check this code:
which could just be converted to
const typeArgumentsText =
node.typeArguments == null
? ''
: sourceCode.getText(node.typeArguments);There was a problem hiding this comment.
| @@ -193,46 +193,20 @@ export default createRule<Options, MessageIds>({ | |||
| const allExportNames = report.typeBasedSpecifiers.map( | |||
| specifier => specifier.local.name, | |||
| ); | |||
|
|
|||
| if (allExportNames.length === 1) { | |||
| const exportNames = allExportNames[0]; | |||
There was a problem hiding this comment.
This line was lost in the transition.
There was a problem hiding this comment.
this was actually not needed, because formatWordList is able to handle a 1-length array
|
@typescript-eslint/triage-team - do we have the consensus to reject this rule? Based on thumbs up I think we do but just confirming before I close. |
|
+1 to reject |
PR Checklist
eslint-plugin-unicornrules internally #9623Overview
Enable
unicorn/prefer-ternary(docs).I believe many of these cases are much simpler than they were previously. Several
lets could be changed toconsts — always a good thing.I recommend viewing the diff with whitespace hidden 😉