chore: setup typescript-eslint #90
Conversation
|
@snitin315 there are some conflicts now |
There was a problem hiding this comment.
Great timing! I'd been hoping to get to the typescript-eslint task soon. Reviewing a PR is even better. 😄
Most of my comments here are on existing code, so it's understandable if they're out of scope for this PR. I'd expect them to be. So for anything that can't be done here I'm requesting updating the comment to reference a filed TODO to refactor the types.
The one blocker is I'm requesting changes on is using the recommended teslint.config with extends to simplify the eslint.config.js block.
Exciting!
snitin315
force-pushed
the
chore/setup-typescript-eslint
branch
from
d534a1c
to
541d614
Compare
|
@JoshuaKGoldberg I've addressed your comments. PTAL |
eslint.config.js
Outdated
| @@ -53,4 +54,10 @@ export default [ | |||
| }, | |||
| }, | |||
| }, | |||
|
|
|||
| // Typescript | |||
There was a problem hiding this comment.
[Typo]
| // Typescript | |
| // TypeScript |
package.json
Outdated
| @@ -33,6 +33,8 @@ | |||
| "got": "^14.4.1", | |||
| "lint-staged": "^15.2.0", | |||
| "prettier": "^3.1.1", | |||
| "typescript": "^5.5.3", | |||
| "typescript-eslint": "^8.0.0-alpha.45", | |||
There was a problem hiding this comment.
We've released 8 as stable since last review! 🚀
| "typescript-eslint": "^8.0.0-alpha.45", | |
| "typescript-eslint": "^8.0.0", |
packages/object-schema/src/types.ts
Outdated
| validate: BuiltInValidationStrategy | ((value: any) => void); | ||
|
|
||
| /** | ||
| * The schema for the object value of this property. | ||
| */ | ||
| // eslint-disable-next-line no-use-before-define -- Ignore for recursive type |
There was a problem hiding this comment.
[Bug] As a general ideology, if a core ESLint rule is acting poorly on TypeScript syntax, it's likely there's a typescript-eslint rule corresponding to it. @typescript-eslint/no-use-before-define is what you'd want to use in this case.
It's not enabled in any of typescript-eslint's recommended configs because it mostly acts as a stylistic rule when you already have TypeScript type checking enabled. My recommendation would be to turn off no-use-before-define and leave it at that. But if you really want to have a lint rule, you'll want to go with https://typescript-eslint.io/rules/no-use-before-define/#how-to-use.
There was a problem hiding this comment.
I've disabled the no-use-before-define rule.
|
For this PR, let's focus on getting typescript-eslint enabled and working without modifying too much code. We can address my naive approaches to typing in follow-up PRs. :) |
Prerequisites checklist
What is the purpose of this pull request?
Enable linting for typescript files.
What changes did you make? (Give an overview)
no-use-before-definerule.@typescript-eslint/no-explicit-anyfor some generic types.Related Issues
Fix #69
Is there anything you'd like reviewers to focus on?
Currently
8.0.0-alpha.45supports ESLint v9, we will need to update it later once a stable version is released.