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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: port lowercase-name to TypeScript #258
Conversation
|
||
const { RuleTester } = require('eslint'); | ||
const rule = require('../lowercase-name'); | ||
const RuleTester: TSESLint.RuleTester = ESLintRuleTester as any; |
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.
|
||
const ruleTester = new RuleTester({ | ||
parser: '@typescript-eslint/parser', |
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.
I find it odd they require this - even if the rule is written in TS, it might not necessarily only run with that parser. Might be needed for all I know 馃し鈥嶁檪
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.
I glanced at the typing of the code (and ended up writing a wall of text on type guards 馃槅). @SimenB If you want me to look in any other specific places, feel free to tell me :)
@jeysal thanks for the review! I reimplemented this now with what I think is a better approach, with my own internal types that are more specific, and type guarding for them. I like this approach, could you take a look? |
Typings look nice! 馃槄 (hell yeah, statically verify everything 馃挭) |
Thanks @jeysal! |
馃帀 This PR is included in version 22.6.0 馃帀 The release is available on: Your semantic-release bot 馃摝馃殌 |
For #256. Includes #257, so just review the second commit.
The diff looks bigger than it is since I had to rewrite all the helpers to include type guards. The implementation of the rule itself is identical.
I can extract the changes to
index
etc to a separate PR if wanted so this is focused on just the rule porting@jeysal any help here would be greatly appreciated 馃檪