build: use typescript-eslint@v6 internally #3541
Conversation
|
This branch is running in CodeSandbox. Use the links below to review this PR faster. |
| @@ -1,18 +1,26 @@ | |||
| module.exports = { | |||
| extends: ['react-app', 'prettier'], | |||
| extends: [ | |||
| 'eslint:recommended', | |||
There was a problem hiding this comment.
Adding 'eslint:recommended' is IME generally a good idea for all JS/TS codebases because it has a lot of good built-in rules. Was it intentional not to have it previously?
| parser: '@typescript-eslint/parser', | ||
| parserOptions: { | ||
| project: true, |
There was a problem hiding this comment.
project: true is already in typescript-eslint@v5. It's just a nice way to enable parserOptions.project 😄
| '@typescript-eslint/dot-notation': 'off', | ||
| 'no-empty': 'off', | ||
| 'prefer-const': 'off', | ||
| 'prefer-rest-params': 'off', |
There was a problem hiding this comment.
Each of these had at least a few existing warnings, so I didn't want to overreach and solve for them without asking. Are any of these rules ones you feel particularly strongly about?
- https://typescript-eslint.io/rules/array-type
- https://typescript-eslint.io/rules/ban-ts-comment
- https://typescript-eslint.io/rules/ban-types
- https://typescript-eslint.io/rules/consistent-indexed-object-style
- https://typescript-eslint.io/rules/consistent-type-definitions
- https://typescript-eslint.io/rules/consistent-type-imports
- https://typescript-eslint.io/rules/dot-notation
- https://typescript-eslint.io/rules/no-confusing-void-expression
- https://typescript-eslint.io/rules/no-empty-function
- https://typescript-eslint.io/rules/no-empty-interface
- https://typescript-eslint.io/rules/no-floating-promises
- https://typescript-eslint.io/rules/no-misused-promises
- https://typescript-eslint.io/rules/no-unsafe-argument
- https://typescript-eslint.io/rules/no-unsafe-assignment
- https://typescript-eslint.io/rules/no-unsafe-call
- https://typescript-eslint.io/rules/no-unsafe-member-access
- https://typescript-eslint.io/rules/no-unsafe-return
- https://typescript-eslint.io/rules/prefer-function-type
- https://typescript-eslint.io/rules/require-await
- https://typescript-eslint.io/rules/restrict-template-expressions
- https://typescript-eslint.io/rules/sort-type-constituents
- https://typescript-eslint.io/rules/unbound-method
- https://eslint.org/docs/latest/rules/no-empty
- https://eslint.org/docs/latest/rules/prefer-const
- https://eslint.org/docs/latest/rules/prefer-rest-params
If not, I can go ahead and fix for all their complaints.
| @@ -12,7 +12,7 @@ | |||
| "react-dom": "^18.1.0", | |||
| "react-redux": "^8.0.2", | |||
| "react-scripts": "5.0.1", | |||
| "typescript": "~4.2.4" | |||
| "typescript": "~4.3.4" | |||
There was a problem hiding this comment.
typescript-eslint@v6 increases the minimum TS range to 4.3.
There was a problem hiding this comment.
Ah, it looks like the Codesandbox environment is running on a version of Node too old for nullish coalescing assignment (??=)
+ node -v (in /tmp/4c6c9715)
v14.21.1
/tmp/4c6c9715/node_modules/@typescript-eslint/eslint-plugin/dist/util/getStringLength.js:16
splitter ??= new grapheme_splitter_1.default();
^^^
SyntaxError: Unexpected token '??='
at new Script (vm.js:102:7)
Per http://kangax.github.io/compat-table/es2016plus/ > 2021 features > Logical Assignment > ??= basic support, Node 16 up is needed.
| [injectEndpointsObjectLiteralExpression] | ||
| ) | ||
| ), | ||
| ], |
There was a problem hiding this comment.
Looks like this file wasn't fully formatted with Prettier before? (will check back in after CI finishes)
✅ Deploy Preview for redux-starter-kit-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Closing as I forgot this was open and there are a bagillion merge conflicts. Let me know if you do have the time for this and I can help out. 😄 |
|
@JoshuaKGoldberg What are you looking for? Time to review? I might be able to help out if it doesn't take too long, though I haven't been actively maintaining so it would be good to at least have a high level approval from someone who is. |
|
Getting approval from an active maintainer would be good, yes please! 😄 I've been swamped lately and the typescript-eslint v6 push was quite a few months ago. So I can't commit to working on this PR anytime soon. But if you folks want help with it, I'm happy to give advice. Once our v7 push rolls around -should be this calendar year! 🤞- I'd be happy to come back around do this again. |
|
for what it's worth, we're working on consolidating our configs including eslint: #4138 we can definitely consider including the update to latest typescript-eslint as part of that, or as follow-up work :) apologies this was missed until now! |
Upgraded typescript-eslint to v6, with reworked ESLint configurations.
You can read https://typescript-eslint.io/blog/announcing-typescript-eslint-v6-beta#user-facing-breaking-changes for the rationale behind the config changes. Essentially, the new recommended starter configs are:
"plugin:@typescript-eslint/recommended-type-checked""plugin:@typescript-eslint/stylistic-type-checked"I've commented any changes ripe for discussion internally. This one was pretty straightforward compared to others though.