eslint-plugin-react [New] support eslint v9

Refs https://github.com/jsx-eslint/eslint-plugin-react/issues/3699

Adds utility to automatically transform tests to ESLint v9 format when ESLint v9 is used.

I have all tests still passing with ESLint v8 locally (we'll see what happens with the older versions in CI).

Around ~500 tests are failing with ESLint v9 due to new RuleTester checks. Some because of problems in tests (e.g., duplicate tests or missing some now-mandatory test case properties), some because of problems in rules (e.g., no-op options schema or unsubstituted placeholders in messages). I'll try to fix that in separate commits in this PR.

Also fixed bugs that ESLint v9 RuleTester caught in rules jsx-closing-bracket-location and no-invalid-html-attribute.

May 19 '24 19:05 mdjermanovic

CI with ESLint v9 fails here:

******> npm ls >/dev/null
npm error code ELSPROBLEMS
npm error invalid: [email protected] /home/runner/work/eslint-plugin-react/eslint-plugin-react/node_modules/eslint
[email protected] /home/runner/work/eslint-plugin-react/eslint-plugin-react
├── @babel/[email protected]
├── @babel/[email protected]
├── @babel/[email protected]
├── @babel/[email protected]
├── @babel/[email protected]
├── @babel/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @typescript-eslint/[email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected] invalid: "^3 || ^4 || ^5 || ^6 || ^7 || ^8" from the root project
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2024-05-19T19_26_11_701Z-debug-0.log
got status code 1
1

Seems I'll have to update peerDependencies in root package.json in this PR to make it reach the test step?

May 19 '24 19:05 mdjermanovic

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.62%. Comparing base (417e1ca) to head (ef6f9c5). Report is 14 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3759      +/-   ##
==========================================
- Coverage   97.79%   97.62%   -0.17%     
==========================================
  Files         134      134              
  Lines        9613     9615       +2     
  Branches     3486     3486              
==========================================
- Hits         9401     9387      -14     
- Misses        212      228      +16

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

May 19 '24 20:05 codecov[bot]

yes, you'll need to update the peerDeps and devDeps ranges to include eslint 9.

May 20 '24 04:05 ljharb

In https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/d8f31860a7b3198bf2ef5a180c11a7d61ccf13c5, https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/9a86bc84ced7ce3232c8ac7d7f101c27a44f3415 & https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/85eed3279f724f76742e8a585d12a4532240f9f3, I updated several tests where output was the same as code to use output: null to assert that there's no autofix expected. ESLint v9 RuleTester reports output === code as a possible error (by writing the full output code, it looks like the author is expecting an autofix). If you want, you can doublecheck whether in those test cases it is really expected that there is no autofix.

May 20 '24 09:05 mdjermanovic

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

Package	New capabilities	Transitives	Size	Publisher
npm/[email protected]	environment Transitive: eval, filesystem, shell, unsafe	`+80`	9.56 MB	eslintbot

View full report↗︎

May 20 '24 11:05 socket-security[bot]

yes, you'll need to update the peerDeps and devDeps ranges to include eslint 9.

Done in https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/a1402a2d624f740ffc1a35ce03abaf3522ecebff & https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/cda54a334d82e8d440f254179680e53105c3b705. I expected the dev deps update to be problematic for linting since this project uses eslintrc config, but npm still installs ESLint v8 (I guess because of the plugins' peer deps) so it's okay. I can now see in CI the same tests that are failing for me locally with ESLint v9. Working on the fixes.

May 20 '24 11:05 mdjermanovic

Actually, it isn't okay everywhere in CI. pretest fails with:

Oops! Something went wrong! :(

ESLint: 9.3.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

https://github.com/jsx-eslint/eslint-plugin-react/actions/runs/9157980135/job/25175410426?pr=3759#logs

May 20 '24 12:05 mdjermanovic

All checks are green now, except for linting because it installs ESLint v9.

Perhaps we could remove ESlint v9 from devDeps and update .github/workflows/node-18+.yml to install ESLint with --save-dev instead of --no-save?

https://github.com/jsx-eslint/eslint-plugin-react/blob/8e1a94b67d081fdc132e9a7e175db3fbf2e02956/.github/workflows/node-18%2B.yml#L43-L48

May 21 '24 00:05 mdjermanovic

For 5801156c39469050595f83cf1ce145d720b00f6e, can we use something like messageId, so the text doesn't have to be duplicated?

May 21 '24 04:05 ljharb

For 5801156, can we use something like messageId, so the text doesn't have to be duplicated?

Sure, now I updated all tests to use messageId in https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/17c4df7495eb54b3a745bb4289e31f24b06e0f12. I was using desc only because the existing tests were using desc too.

Forgot to note about this change: ESLint v9 RuleTester requires that suggestion test objects must have messageId or desc specified.

May 21 '24 09:05 mdjermanovic

I see some commits from this branch are already merged into master. Anything else I could do to help finish eslint v9 support?

I think the only remaining issue here was a technical problem of how to ensure that eslint v8 is used for linting this project (https://github.com/jsx-eslint/eslint-plugin-react/pull/3759#issuecomment-2121476913).

May 30 '24 18:05 mdjermanovic

I've rebased this, and we'll see what tests are still failing.

May 30 '24 19:05 ljharb

For me locally, npm installs eslint v8.

But in pretest CI, it installs eslint v9. Maybe because of this:

https://github.com/jsx-eslint/eslint-plugin-react/blob/417e1ca292788c75618dc994b084c3a57c483fce/.github/workflows/node-pretest.yml#L15-L16

Perhaps we could remove it?

May 30 '24 20:05 mdjermanovic

aha. if we remove it i suspect npm install will fail, but it's worth a shot!

May 30 '24 21:05 ljharb

All the checks are green, except for pending codecov statuses, which it seems will not arrive.

May 30 '24 22:05 mdjermanovic

Indeed; codecov is broken rn (https://github.com/codecov/feedback/issues/374) so we don't have to worry about that.

How can we also test that this plugin works in someone else's flat config?

May 31 '24 00:05 ljharb

@ljharb Is it possible to upload this to npm with a tag for others to test? If so I am sure many users would be willing to test it with their setup and report any issues or problems, if not the other option is to create multiple configs and test them manually or automate it which might still miss some variations and be quite time consuming.

May 31 '24 04:05 DysektAI

@DysektAI since this package doesn't have a build process, you can just npm install the PR - npm i jsx-eslint/eslint-plugin-react#pull/3759, i believe :-)

May 31 '24 04:05 ljharb

So far in my testing everything works fine as it should even while using a few other plugins and a bunch of rules no errors or issues using the flat config and VS Code ESLint extension others will probably need to test and ensure consistency but looks good on my end no hacks or customization needed to make it work like react-hooks which requires using this currently and the canary version

eslint.config.js

import { fixupPluginRules } from "@eslint/compat";
import js from '@eslint/js';
import react from 'eslint-plugin-react';
import hooks from 'eslint-plugin-react-hooks';
import globals from "globals";

export default [
    {
        name: "base",
        files: ["**/*.js", "**/*.jsx"],
        ignores: ["node_modules/**"],
        languageOptions: {
            ecmaVersion: "latest",
            sourceType: "module",
            globals: {
                ...globals.browser,
            },
            parserOptions: {
                ecmaFeatures: {
                    jsx: true,
                },
            },
        },
        linterOptions: {
            reportUnusedDisableDirectives: "error",
            noInlineConfig: true,
        },
        plugins: {
            react,
            'react-hooks': fixupPluginRules(hooks)
        },
        rules: {
            'react/jsx-uses-react': 'error',
            'react/jsx-uses-vars': 'error',
            ...js.configs.recommended.rules,
            ...hooks.configs.recommended.rules,
            // Possible Errors
            "for-direction": "error",
            "getter-return": "error",
            "no-async-promise-executor": "error",
            "no-await-in-loop": "warn",
            "no-compare-neg-zero": "error",
            "no-cond-assign": "error",
            // Best Practices
            "accessor-pairs": "warn",
            "array-callback-return": "error",
            "block-scoped-var": "error",
            "class-methods-use-this": "warn",
            "complexity": ["warn", 11], // Adjust the threshold as needed
            // Variables
            "no-delete-var": "error",
            "no-label-var": "error",
            "no-restricted-globals": ["error", "event", "fdescribe"],
            "no-shadow": "error",
            "no-shadow-restricted-names": "error",
            // Stylistic Issues
            "array-bracket-newline": ["error", "consistent"],
            "array-bracket-spacing": ["error", "never"],
            "array-element-newline": ["error", "consistent"],
            "block-spacing": ["error", "always"],
            "brace-style": ["error", "1tbs", { "allowSingleLine": true }],
            // ECMAScript 6
            "arrow-body-style": ["error", "as-needed"],
            "arrow-parens": ["error", "as-needed"],
            "arrow-spacing": ["error", { "before": true, "after": true }],
            "generator-star-spacing": ["error", { "before": false, "after": true }],
            "no-confusing-arrow": "error",
            "react/jsx-key": "error",             // Enforce unique keys for elements in arrays
            "react/no-unescaped-entities": "error", // Prevent unescaped HTML entities in JSX
            "react/prop-types": "off",             // Disable if using TypeScript
            "react/no-unused-state": "error",     // Warn when state variables are unused
            // Additional recommended rules:
            "react/jsx-no-useless-fragment": "error",     // Avoid unnecessary fragments
            "react/self-closing-comp": "error",         // Enforce self-closing tags
            "react/button-has-type": "error",          // Enforce type attribute for buttons
            "react/no-unknown-property": "error",        // Prevent usage of unknown DOM properties
            "react/jsx-boolean-value": ["error", "never"], // Disallow unnecessary boolean value literals in JSX
            "react/jsx-curly-brace-presence": ["error", "never"], // Enforce consistent use of curly braces in JSX
            "react/jsx-no-duplicate-props": "error",     // Disallow duplicate props in JSX
        },
        settings: {
            "react": {
                "version": "detect"
            }
        }
    }
];

May 31 '24 09:05 DysektAI

How can we also test that this plugin works in someone else's flat config?

I've added a test setup and one example in https://github.com/jsx-eslint/eslint-plugin-react/pull/3759/commits/ef6f9c5dbaca93f45b19e763e73b585a9161ac3a. Let me know if this looks good, and I can add examples with plugin configs (recommended, all, jsx-runtime) as well.

May 31 '24 10:05 mdjermanovic

What are we waiting for?

Jul 02 '24 03:07 fisker

@fisker https://github.com/jsx-eslint/eslint-plugin-react/pull/3759#issuecomment-2141031324

Jul 02 '24 03:07 ljharb

@ljharb I was waiting for feedback on https://github.com/jsx-eslint/eslint-plugin-react/pull/3759#issuecomment-2141695605 here. In the meantime, all those tests have been added in https://github.com/jsx-eslint/eslint-plugin-react/pull/3694, which is I believe ready for review. Once https://github.com/jsx-eslint/eslint-plugin-react/pull/3694 is merged, we can remove https://github.com/jsx-eslint/eslint-plugin-react/commit/ef6f9c5dbaca93f45b19e763e73b585a9161ac3a from this PR.

Please let me know if anything is waiting for me.

Jul 02 '24 05:07 mdjermanovic

Thanks, seems like we're really blocked on #3694 then.

Jul 02 '24 15:07 ljharb

Since https://github.com/jsx-eslint/eslint-plugin-react/pull/3694 has been merged, I removed https://github.com/jsx-eslint/eslint-plugin-react/commit/ef6f9c5dbaca93f45b19e763e73b585a9161ac3a from this PR and this is now ready for review.

Jul 14 '24 19:07 mdjermanovic

All tests are passing except for codecov and Automatic Rebase (I'm not sure why, it reports conflicts in files that haven't been modified in this PR).

Jul 14 '24 20:07 mdjermanovic

awesome, you don't have to worry about those.

Jul 15 '24 04:07 ljharb

eslint-plugin-react eslint-plugin-react copied to clipboard

[New] support eslint v9

Codecov Report

eslint-plugin-react
eslint-plugin-react copied to clipboard