feat(remix-eslint-config): extend from typescript-eslint/recommended #6248
Conversation
|
| @@ -1,4 +1,3 @@ | |||
| /* eslint-disable no-unreachable */ | |||
There was a problem hiding this comment.
This rule is explicitly turned off in typescript-eslint configs because it's covered by TypeScript.
| ecmaFeatures: { | ||
| jsx: true, | ||
| }, | ||
| warnOnUnsupportedTypeScriptVersion: true, |
There was a problem hiding this comment.
warnOnUnsupportedTypeScriptVersion is enabled by default: https://typescript-eslint.io/architecture/parser/#warnonunsupportedtypescriptversion
| parser: "@typescript-eslint/parser", | ||
| parserOptions: { | ||
| sourceType: "module", | ||
| ecmaVersion: 2019, | ||
| ecmaFeatures: { | ||
| jsx: true, | ||
| }, |
There was a problem hiding this comment.
ecmaFeatures.jsx is enabled by default for .tsx files.
| @@ -21,17 +21,14 @@ export type DeferFunction = <Data extends Record<string, unknown>>( | |||
| init?: number | ResponseInit | |||
| ) => TypedDeferredData<Data>; | |||
|
|
|||
| export type JsonFunction = <Data extends unknown>( | |||
There was a problem hiding this comment.
https://typescript-eslint.io/rules/no-unnecessary-type-constraint - extends unknown is redundant for these type parameters.
There was a problem hiding this comment.
The docs are talking about any only, but this had unknown
Does the same apply?
If so, I guess it would be a good idea to update the docs?
There was a problem hiding this comment.
Great point 😄 thanks! Filed typescript-eslint/typescript-eslint#6973.
|
@JoshuaKGoldberg What would it take to enable |
|
Yeah enabling plugin:@typescript-eslint/recommended-requiring-type-checking would be a very good next step after this. If the powers-that-be are particularly interested, it could certainly be done either as a followup or as part of this PR. The main reason I didn't add type checked rules in this PR was type aware rule performance.
I'd want (someone or myself) to investigate how to improve that specifically for Remix's setup. E.g. typescript-eslint/typescript-eslint#6754 |
|
Sorry for taking a couple weeks to respond to your comments @brophdawg11! I lost track of this while traveling 😅. But I'm glad you're looking at it! I responded to each thread inline - for each, let me know and I'm happy to make the change. |
|
@JoshuaKGoldberg It seems like you missed something that causes an ESLint error Do you want to fix that one please? |
|
Aha, introduced merging from |
brophdawg11
changed the base branch from
main
to
brophdawg11/typescript-eslint-recommended
|
This should go to |
brophdawg11
merged commit 4a617f6
into
remix-run:brophdawg11/typescript-eslint-recommended
…6248) Co-authored-by: Matt Brophy <matt@brophy.org>
Wipes out most of the existing
typescript.jslint rules, in favor of directly extendingplugin:@typescript-eslint/recommended. Remaining rule configurations are grouped with comments:rules/core.jsthat aren't needed for TypeScript codeAddresses: #6126
[ ] Tests:yarn lintstill passes, yes?Testing Strategy: I'd encourage folks to try this out on any large Remix repository written with TypeScript. And let me know what those are so I can test them out too? 😄
Note that this does not (yet?) tackle two good next steps:
plugin:@typescript-eslint/recommended-requiring-type-checkingAny code changes are commented inline.