jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.86k stars 2.75k forks source link

feat: Add eslint v9 support #3743

Open MatiPl01 opened 2 months ago

MatiPl01 commented 2 months ago

Description

This PR adds eslint v9 support. It also adjusts respective tests to use v9-style config.

TODO

Fixing tests

I got most of tests working by simply changing the config used in tests or removing duplicate test cases. Below is the list of rules which tests still are failing and fixing them will require a bit more effort. I will continue fixing tests in my free time.

Fixing eslint config

After upgrading eslint to v9, the .eslintrc config used in the eslint-plugin-react repo is no longer valid. We should either migrate from .eslintrc config to eslint.config.js, or use old version of eslint just for linting in this repository.

Because .eslintrc config is no longer valid, CI shows that all tests are failing. I ran test files directly with the following command while fixing test issues:

./node_modules/mocha/bin/_mocha tests/lib/rules/<rule-name>.js
MatiPl01 commented 2 months ago

We can’t support anything we’re not testing, and this doesn’t add v9 to tests.

I know, that's why this PR is still a draft. I just started working on eslint v9 adjustments based on [this](https://eslint.org/blog/2023/09/preparing-custom-rules-eslint-v9/#context.getscope()) article which explains what should be changed in eslint v9.

If there is already an ongoing work or the support of the eslint v9 is not intended to be added in the near future, I can close this PR.

ljharb commented 2 months ago

Support is definitely intended :-) #3726 is another attempt.

socket-security[bot] commented 2 months ago

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/eslint@9.1.1 environment, filesystem Transitive: eval, shell, unsafe +76 9.05 MB eslintbot

View full report↗︎

MatiPl01 commented 2 months ago

@ljharb

I fixed most of failing tests and adjusted them to eslint v9 style. There are 5 remaining rules which tests still don't pass. I can get back to fixing them later.

I wanted to ask you, if you have an idea how to handle linting in this repo? I saw that .eslintrc extends airbnb configs which still don't support eslint v9, so we would have to somehow use eslint v9 and v8 at the same time, v8 for linting code in this repo and v9 in tests.

I wonder if creating yarn/npm workspaces with different eslint versions would be a valid solution for you and if it will solve the problem.

ljharb commented 2 months ago

Don't worry about the linting yet; the import plugin has to come first for that.

However, we already have to retain compatibility for every eslint version we currently support, so we can't switch things over to just work on v9, or just on flat config.

If we used workspaces, we'd only use npm, but I don't think that's necessary - the way we support multiple eslint versions now is by manually altering the deps in CI, and we'd do that here as well.

MatiPl01 commented 2 months ago

Don't worry about the linting yet; the import plugin has to come first for that.

However, we already have to retain compatibility for every eslint version we currently support, so we can't switch things over to just work on v9, or just on flat config.

If we used workspaces, we'd only use npm, but I don't think that's necessary - the way we support multiple eslint versions now is by manually altering the deps in CI, and we'd do that here as well.

I see, thanks for clarification. I still wonder what will be the best way to ensure that tests work in all eslint versions. v9 moved parserOptions to the new languageOptions property so we can't simply add v9 to the list of tested eslint versions on CI, because parserOptions can no longer be passed to the RuleTester and all tests will fail.

ljharb commented 2 months ago

We'll probably need a helper function that detects the eslint version and adjusts appropriately.

The smallest change for now would be to leave the tests largely as-is, and have the helper change them to the v9 format when applicable - later, we'd probably want to invert that, but that can wait.