style(eslint): enable spaced-comment, @typescript-eslint/no-unused-vars
#1261
Conversation
spaced-comment, @typescript-eslint/no-unused-vars & no-shadow
b5ad7aa to
29323cd
Compare
Generated by 🚫 dangerJS |
There was a problem hiding this comment.
I don't agree with enabling no-shadow.
One of the purposes of scopes is to give you some level of encapsulation. Although it can sometimes lead to a better naming it often results in using misleading or artificial names and makes code more difficult to read.
One example:
| } | ||
|
|
||
| private handleSelectedItemRemove( | ||
| e: React.SyntheticEvent, | ||
| item: ShorthandValue, | ||
| predefinedProps: DropdownSelectedItemProps, | ||
| DropdownSelectedItemProps: DropdownSelectedItemProps, | ||
| dropdownSelectedItemProps: DropdownSelectedItemProps, |
I can't agree with you there: yep, sometimes it will require changes in names, but in the most cases you will clearly understand what happens and it will save you from accidental bugs. Actually, this case is noop and it will be removed in a future PR: https://github.com/stardust-ui/react/pull/1268/files#diff-ad7603568c43fa756c43f87b68b69161R82. However, if I will remove |
|
speaking of The reason is because its value is not clearly seen at this moment. Ideally, I would want to see real issue (say, in app's logic) that this rule helps to prevent, before introducing it. Also, as a point towards deferring it is that it introduces some friction points in code, without clear value for our code seen at the moment |
spaced-comment, @typescript-eslint/no-unused-vars & no-shadowspaced-comment, @typescript-eslint/no-unused-vars
Codecov Report
@@ Coverage Diff @@
## master #1261 +/- ##
=======================================
Coverage 72.08% 72.08%
=======================================
Files 736 736
Lines 5627 5627
Branches 1646 1646
=======================================
Hits 4056 4056
Misses 1566 1566
Partials 5 5
Continue to review full report at Codecov.
|
This PR enables following rules:
spaced-comment@typescript-eslint/no-unused-varsPlease see #600 for more context.