chore: enable radix #9563
chore: enable radix #9563
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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9563 +/- ##
=======================================
Coverage 88.45% 88.45%
=======================================
Files 422 422
Lines 14695 14695
Branches 4298 4298
=======================================
Hits 12998 12998
Misses 1372 1372
Partials 325 325
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -160,7 +160,7 @@ type RequireKeys< | |||
| > = ExcludeKeys<Obj, Keys> & { [k in Keys]-?: Exclude<Obj[k], undefined> }; | |||
|
|
|||
| function getEnumNames<T extends string>(myEnum: Record<T, unknown>): T[] { | |||
| return Object.keys(myEnum).filter(x => isNaN(parseInt(x))) as T[]; | |||
| return Object.keys(myEnum).filter(x => isNaN(+x)) as T[]; | |||
There was a problem hiding this comment.
Why wouldn't we use this?
| return Object.keys(myEnum).filter(x => isNaN(+x)) as T[]; | |
| return Object.keys(myEnum).filter(x => isNaN(parseInt(x, 10))) as T[]; |
There was a problem hiding this comment.
I personally prefer using an operator instead of a function when available, especially since the + operator works more safely than parseInt does by default (i.e., + assumes base 10 by default). However, I'm happy to change it if you'd like
There was a problem hiding this comment.
Gotcha, I'm cool with it, just wanted to check to make sure there was intentionality 👍 . I can never quite keep straight the differences between parseInt() and Number()/+ but I trust and defer to your judgement here 🙂.
I would personally say I do much prefer the explicit style....
| return Object.keys(myEnum).filter(x => isNaN(+x)) as T[]; | |
| return Object.keys(myEnum).filter(x => isNaN(Number(x))) as T[]; |
Up to you though! Other team members might have different opinions too 🤷
There was a problem hiding this comment.
Sounds good! I have changed it to Number and added no-implicit-coercion (docs) to enforce the style. That rule does not report on any other code besides those two uses of the + operator.
Let me know if you'd prefer that in a separate PR
PR Checklist
Overview
Enable
radix(docs)