chore: upgrade `@typescript-eslint` #852

lesha1201 · 2023-12-06T12:50:25Z

eslint-plugin-testing-library has an outdated dependency - @typescript-eslint/utils and because of it, in modern setups, it might result in duplicate @typescript-eslint/parser packages.

This PR upgrades all @typescript-eslint dependencies as well as TypeScript.

Because @typescript-eslint@6 uses package.json exports, we had to upgrade some dependencies which didn't support exports (see typescript-eslint/typescript-eslint#7284). Affected dependencies:

jest

I also dropped support for Node.js versions lower than 16 because they're not maintained anymore (v16 also not maintained but decided to leave it for now).

`@typescript-eslint@6`

https://typescript-eslint.io/blog/announcing-typescript-eslint-v6

Upgrading @typescript-eslint brought new rules. The rules I fixed:

`eslint-config-kentcdodds` uses some outdated packages which block us from upgrading TypeScript dependencies.

Because `@typescript-eslint` uses package.json `exports`, we had to upgrade some dependencies which didn't support `exports` (see typescript-eslint/typescript-eslint#7284). Affected dependencies: - `jest`

lesha1201 · 2023-12-06T12:52:44Z

.eslintrc.js

 	extends: [
-		'kentcdodds',


eslint-config-kentcdodds uses some outdated packages which block us from upgrading TypeScript dependencies properly. It didn't bring many rules so decided just use eslint:recommended, plugin:import/recommended and configure some rules manually.

I like this change! We should apply it anyway.

lesha1201 · 2023-12-06T12:53:53Z

.github/workflows/pipeline.yml

@@ -53,7 +53,7 @@ jobs:
      fail-fast: false
      matrix:
        # The .x indicates "the most recent one"
-        node: [19.x, 18.x, 17.x, 16.x, 14.x, 14.17.0, 12.x, 12.22.0]


Dropped support for Node.js versions lower than 16. They're not maintained for a long time and don't support package.json exports which is used at least in @typescript-eslint.

lesha1201 · 2023-12-06T12:54:43Z

.prettierignore

+# Files
+package-lock.json
+
+# Folders
+node_modules/
+dist/


Add .prettierignore to ignore dist folder.

lesha1201 · 2023-12-06T13:01:19Z

.eslintrc.js

@@ -44,8 +55,8 @@ module.exports = {
 				project: ['./tsconfig.eslint.json'],
 			},
 			extends: [
-				'plugin:@typescript-eslint/recommended',
-				'plugin:@typescript-eslint/recommended-requiring-type-checking',
+				'plugin:@typescript-eslint/strict-type-checked',


We can also add plugin:@typescript-eslint/stylistic-type-checked which brings some useful rules but I wasn't sure what rules should be used in the project.

lesha1201 · 2023-12-06T13:02:58Z

.eslintrc.js

@@ -54,6 +65,25 @@ module.exports = {
 					{ argsIgnorePattern: '^_' },
 				],
 				'@typescript-eslint/no-use-before-define': 'off',
+				'@typescript-eslint/no-unnecessary-condition': 'off',


Disabled that rule because we have a lot of errors from it in the project and fixing it might cause some issues. If we want to enable this rule, we need to fix it carefully.

lesha1201 · 2023-12-06T13:04:53Z

lib/rules/no-await-sync-events.ts

+						// @ts-expect-error -- TODO: fix me
 						property.init.value > 0


Previously, we had suppressImplicitAnyIndexErrors: true in tsconfig.json which allowed such code. But in TypeScript v5, it complains about this configuration so I removed it.

I think we should fix such places but it's better to do it in a separate PR.

lesha1201 · 2023-12-06T13:06:49Z

tsconfig.json

+		"module": "Node16",
+		"moduleResolution": "Node16",


We need this to support exports from package.json (typescript-eslint/typescript-eslint#7284).

lesha1201 · 2023-12-06T13:08:05Z

tsconfig.json

 		"esModuleInterop": true,
 		"resolveJsonModule": true,
 		"forceConsistentCasingInFileNames": true,
 		"noImplicitAny": true,
 		"outDir": "./dist",
 		"removeComments": true,
 		"skipLibCheck": true,
-		"sourceMap": false,
-		"suppressImplicitAnyIndexErrors": true
+		"sourceMap": false
 	},
 	"include": ["./lib/**/*.ts"]


I think we should include all TS files by default so IDE can process all TS files and have a separate TS config for building the package. Decided to not do this in this PR.

Belco90 · 2023-12-11T10:32:03Z

Hi @lesha1201. Thanks for your contribution, but I'm afraid I have to reject this PR until ESLint v9 is released, and we bump the plugin to the next major:

We don't want to drop support for Node.js lower than v18 yet. We are aware they are not officially supported, but we want to support the same supported version as ESLint. They'll drop support for Node prior to v18.18 in ESLint v9, so we will do it once ESLint v9 is released.
For the previous reason, we are keeping @typescript-eslint/utils in v5. We will upgrade it in the next major together with dropping non-supported Node.js versions.
Upgrading TS to v5 should be addressed in a separate PR, it's too much for mixing it with other stuff. Same for other dependencies like jest.

lesha1201 · 2023-12-12T08:42:11Z

@Belco90

We don't want to drop support for Node.js lower than v18 yet. We are aware they are not officially supported, but we want to support the same supported version as ESLint. They'll drop support for Node prior to v18.18 in ESLint v9, so we will do it once ESLint v9 is released.

We only dropped support for Node.js lower than v16 in this PR. And it might work fine in v12 and v14 because they also support package.json exports but I didn't test it (https://nodejs.org/api/packages.html#exports) since typescript-eslint dropped official support for v12 and v14.

The main issue that motivated me to open this PR is that a lot of popular ESLint plugins uses typescript-eslint@6 and the most of TS projects use it as well which results in duplicated @typescript-eslint/parser when using it with eslint-plugin-testing-library.

ESLint v9 won't be released soon. We can create a major release now that upgrades TS related packages and then create another major release that upgrades ESLint v9 and something else. Users who rely on old Node.js versions can use the current version of eslint-plugin-testing-library and if they report some issue that can be patched, we can create a patch release for them. Another option is to create alpha release so the users can start using it eliminating the issue with duplicated @typescript-eslint/parser.

Should I open separate PRs for some changes? If so can you please write a list of changes that you would like to see in separate PRs? Also do you have other tasks I can work on? For example, we can upgrade Prettier.

Belco90 · 2023-12-12T16:10:23Z

We only dropped support for Node.js lower than v16 in this PR

That's already a breaking change, but also it's not in the same page as versions supported by ESLint v8.

ESLint v9 won't be released soon. We can create a major release now that upgrades TS related packages and then create another major release that upgrades ESLint v9 and something else.

We could indeed. No idea when ESLint will be released, but I want to avoid releasing a major just for the sake of upgrading typescript-eslint.

I get the motivation for all the updates you did (I was planning to start working on it during Christmas break, so any help is appreciated). However, we can't perform them all at once, and some will have to wait a bit. In order to keep discussing it, I'll open a new issue.

lesha1201 added 4 commits December 6, 2023 12:17

chore: remove eslint-config-kentcdodds

b6443b6

`eslint-config-kentcdodds` uses some outdated packages which block us from upgrading TypeScript dependencies.

chore: upgrade TypeScript

20c87e8

chore!: drop support of unmaintained Node.js versions

9526274

chore: upgrade @typescript-eslint

611e827

Because `@typescript-eslint` uses package.json `exports`, we had to upgrade some dependencies which didn't support `exports` (see typescript-eslint/typescript-eslint#7284). Affected dependencies: - `jest`

lesha1201 commented Dec 6, 2023

View reviewed changes

Belco90 assigned lesha1201 Dec 11, 2023

Belco90 self-requested a review December 11, 2023 10:06

Belco90 closed this Dec 11, 2023

Belco90 mentioned this pull request Dec 12, 2023

Upgrade core dependencies #855

Open

8 tasks

lesha1201 mentioned this pull request Dec 12, 2023

chore: remove eslint-config-kentcdodds #856

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade `@typescript-eslint` #852

chore: upgrade `@typescript-eslint` #852

lesha1201 commented Dec 6, 2023 •

edited

Loading

lesha1201 Dec 6, 2023

Belco90 Dec 11, 2023

lesha1201 Dec 6, 2023 •

edited

Loading

lesha1201 Dec 6, 2023

lesha1201 Dec 6, 2023

lesha1201 Dec 6, 2023

lesha1201 Dec 6, 2023 •

edited

Loading

lesha1201 Dec 6, 2023

lesha1201 Dec 6, 2023

Belco90 commented Dec 11, 2023

lesha1201 commented Dec 12, 2023

Belco90 commented Dec 12, 2023 •

edited

Loading

chore: upgrade @typescript-eslint #852

chore: upgrade @typescript-eslint #852

Conversation

lesha1201 commented Dec 6, 2023 • edited Loading

@typescript-eslint@6

lesha1201 Dec 6, 2023

Choose a reason for hiding this comment

Belco90 Dec 11, 2023

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023

Choose a reason for hiding this comment

lesha1201 Dec 6, 2023

Choose a reason for hiding this comment

Belco90 commented Dec 11, 2023

lesha1201 commented Dec 12, 2023

Belco90 commented Dec 12, 2023 • edited Loading

chore: upgrade `@typescript-eslint` #852

chore: upgrade `@typescript-eslint` #852

lesha1201 commented Dec 6, 2023 •

edited

Loading

`@typescript-eslint@6`

lesha1201 Dec 6, 2023 •

edited

Loading

lesha1201 Dec 6, 2023 •

edited

Loading

Belco90 commented Dec 12, 2023 •

edited

Loading