-
-
Notifications
You must be signed in to change notification settings - Fork 32
build: set up typescript-eslint #542
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change adds typescript-eslint to our config and fixes a miss in the original TypeScript migration where we weren't linting the newly renamed `ts` files. This also addresses violations found from both sets of config changes.
32abdfb
to
6a4ab31
Compare
docs/rules/no-property-in-node.md
Outdated
} | ||
}, | ||
}; | ||
}, | ||
}; | ||
} satisfies Rule.RuleModule; |
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.
It seems like we're mixing commonjs syntax and TypeScript in this example, which is a bit odd. Can we choose one:
- commonjs with the jsdoc types
- or ESM TypeScript and include any imports for the types
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.
Definitely agree with you there. The docs are a bit all over the place, with different people writing them at different points in time. It would benefit from someone going through all of them, with a focus on making them consistent (which I'm also happy to do). I don't think that's really in the scope of this change, though, which is just focused on enabling the eslint plugin. I had to touch this file, to address a violation. Would you prefer I change it to js here, and remove the satisfies
?
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.
Leaving it as cjs would be best then.
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.
Updated. I changed the examples to js
and put the type annotation back in.
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.
Thanks!
This change adds typescript-eslint to our config and fixes a miss in the original TypeScript migration where we weren't linting the newly renamed
ts
files.This also addresses violations found from both sets of config changes.
Closes #543