eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

ESLint v9 contains breaking API changes

Open james-yeoman opened this issue 1 year ago • 60 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues and my issue is unique
  • [X] My issue appears in the command-line and not only in the text editor

Description Overview

Upon bumping to the ESLint beta for v9, I was met with several errors in my monorepo during the linting test-run.

Namely:

  • Error: context.getScope is not a function
    • Rule: "react/no-string-refs"
  • Error: context.getFirstTokens is not a function
    • Rule: "react/display-name"

I get that it's still only a beta, but these API changes were announced in september 2023.

Additionally, there are some rule structure changes outlined separately that may be worth ensuring are in compliance.

Expected Behavior

Given that ESLint v9 is now in beta, I wasn't expecting to find any plugins that haven't yet addressed the API changes

eslint-plugin-react version

v7.34.0

eslint version

v9.0.0-beta.1

node version

v18.12.0

james-yeoman avatar Mar 04 '24 14:03 james-yeoman

That expectation is flawed; the point of it being in beta is for plugins to begin to address the changes - announcements are irrelevant. Thanks for the report.

A fix for this will also need to add eslint 9 into the test matrix.

ljharb avatar Mar 04 '24 15:03 ljharb

I put up a branch running tests on eslint 9; https://github.com/ljharb/eslint-plugin-react/actions/runs/8147742013/job/22269131527#step:5:21 is failing because the jsx-no-undef and jsx-uses-react and jsx-uses-vars tests call linter.defineRule. @bmish, any thoughts on an approach here?

ljharb avatar Mar 04 '24 22:03 ljharb

Additionally, when I comment out those 3 lines, i get 22166 failures because Error: ESLint configuration in rule-tester is invalid: Key "parserOptions": This appears to be in eslintrc format rather than flat config format.. Does this mean eslint 9 drops support for eslintrc, or it's just no longer the default?

If the former, how can we run the same tests with both normal eslintrc stuff on eslint 8, and also with flat config on eslint 9?

ljharb avatar Mar 04 '24 22:03 ljharb

ESLint 9 makes the flat config the default. It renames the ESLintRC style classes to be LegacyESLint and the Linter requires an option of {configType: "eslintrc"}, and in ESLint 10, they plan on dropping the legacy config altogether.

There's a migration guide that might be a good place to start in terms of compiling a list of required tasks for this

james-yeoman avatar Mar 05 '24 09:03 james-yeoman

Awesome, thanks for the pointers.

That will help get us unblocked for eslint 9, but we'll still have a lot of work to support eslint 10.

ljharb avatar Mar 06 '24 01:03 ljharb

Is there any update on this yet? Or is this still quite far off? The first release candidate for v9 released last week

james-yeoman avatar Apr 02 '24 08:04 james-yeoman

@james-yeoman it is almost never the case that all the plugins in the eslint ecosystem support a new major until awhile after the final release is out. It'd be great to beat that, but if there'd been any update, it'd be in this issue :-)

ljharb avatar Apr 02 '24 16:04 ljharb

Eslint officially released v9.0.0 today. Hope this plugin will support it soon!

JstnMcBrd avatar Apr 06 '24 06:04 JstnMcBrd

Getting TypeError: context.getScope is not a function here

nodegin avatar Apr 08 '24 07:04 nodegin

I'd like to continue using this plugin, but this bug is unfortunately stopping me.

I hope we get to see a fix soon!

chelsea6502 avatar Apr 10 '24 23:04 chelsea6502

Nobody’s forcing you to upgrade to eslint 9 right away ¯\_(ツ)_/¯ new eslint majors always take months before everything supports them, and this one will take longer because it’s changing the default config format.

ljharb avatar Apr 11 '24 04:04 ljharb

and this one will take longer because it’s changing the default config format.

Having worked on a couple of other plugins, I want to elaborate that I think there's two parts to this upgrade.

The real breaking change for v9 is the fact that v9 has removed APIs - it looks like this is being covered in #3727.

The second change is flat config compatibility. Consumers can use the plugin as-is (as long as the API issues are resolved). It is slightly more work, but it is possible. Of course, supporting the flat config natively makes it easier, and ESLint has a good migration guide should anyone wanting to upgrade to v9 feel like contributing a patch (I might eventually, but I have several other plugins that I'm focussing on).

Standard8 avatar Apr 11 '24 07:04 Standard8

That expectation is flawed; the point of it being in beta is for plugins to begin to address the changes - announcements are irrelevant. Thanks for the report.

A fix for this will also need to add eslint 9 into the test matrix.

@ljharb Does this mean that eslint 9 is still raw for full use?

root9464 avatar Apr 15 '24 06:04 root9464

it means you can’t use eslint 9 with this plugin yet. It often takes many months when a new eslint major comes out before enough of the ecosystem supports it - you shouldn’t expect to upgrade right away.

ljharb avatar Apr 15 '24 15:04 ljharb

Thanks for your work on this plugin, @ljharb! Seems totally reasonable to take some time to support ESLint 9, and anyone who reeeeeeally needs to be on the latest and greatest should consider hiring you (if you're available for such work) or volunteering their time to make it happen. Since I'm not able to do that myself at the moment, I'm happy to wait to upgrade ESLint.

graue avatar Apr 19 '24 06:04 graue

Scott, that's a perfect answer :) Thanks all!

danielweck avatar Apr 19 '24 07:04 danielweck

The same problem occurs with eslint-plugin-solid. fyi.

TypeError: context.getScope is not a function Occurred while linting /data/data/com.termux/files/home/LearnSolidJS/FirstApp/src/FibCounter.tsx:15 Rule: "solid/reactivity" at checkForReactiveAssignment (/data/data/com.termux/files/home/LearnSolidJS/node_modules/eslint-plugin-solid/dist/rules/reactivity.js:445:69) at VariableDeclarator (/data/data/com.termux/files/home/LearnSolidJS/node_modules/eslint-plugin-solid/dist/rules/reactivity.js:768:11) at ruleErrorHandler (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/linter.js:1115:48) at /data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/safe-emitter.js:45:58 at Array.forEach () at Object.emit (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/safe-emitter.js:45:38) at NodeEventGenerator.applySelector (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/node-event-generator.js:297:26) at NodeEventGenerator.applySelectors (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/node-event-generator.js:326:22) at NodeEventGenerator.enterNode (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/node-event-generator.js:340:14) at runRules (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/linter.js:1154:40)

linusjf avatar Apr 22 '24 15:04 linusjf

The same problem occurs with eslint-plugin-solid. fyi.

TypeError: context.getScope is not a function Occurred while linting /data/data/com.termux/files/home/LearnSolidJS/FirstApp/src/FibCounter.tsx:15 Rule: "solid/reactivity" at checkForReactiveAssignment (/data/data/com.termux/files/home/LearnSolidJS/node_modules/eslint-plugin-solid/dist/rules/reactivity.js:445:69) at VariableDeclarator (/data/data/com.termux/files/home/LearnSolidJS/node_modules/eslint-plugin-solid/dist/rules/reactivity.js:768:11) at ruleErrorHandler (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/linter.js:1115:48) at /data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/safe-emitter.js:45:58 at Array.forEach () at Object.emit (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/safe-emitter.js:45:38) at NodeEventGenerator.applySelector (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/node-event-generator.js:297:26) at NodeEventGenerator.applySelectors (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/node-event-generator.js:326:22) at NodeEventGenerator.enterNode (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/node-event-generator.js:340:14) at runRules (/data/data/com.termux/files/usr/lib/node_modules/eslint/lib/linter/linter.js:1154:40)

Reverted my global installation of eslint to 8.57.0. A case of TLDR;

linusjf avatar Apr 22 '24 23:04 linusjf

Indeed, you simply can't upgrade to eslint 9 until all of your eslint plugins/configs have explicit peer dep support for it. (this is true for every eslint major)

ljharb avatar Apr 23 '24 00:04 ljharb

Hey, the reason for error (and backward's compatible implementation) can be found here: https://eslint.org/blog/2023/09/preparing-custom-rules-eslint-v9/#context.getscope()

As for sourceCode.getFirstTokens() it's here: https://eslint.org/blog/2023/09/preparing-custom-rules-eslint-v9/#from-context-to-sourcecode

olaf-cichocki avatar Apr 25 '24 20:04 olaf-cichocki

Also getting the error for these rules:

    'react/no-direct-mutation-state': 0,
    'react/require-render-return': 0,
    'react/no-string-refs': 0,
    'react/jsx-uses-react': 0,
    'react/react-in-jsx-scope': 0,
    'react/no-danger-with-children': 0,
    'react/jsx-no-undef': 0,
    'react/jsx-uses-vars': 0,

lucasprag avatar May 02 '24 18:05 lucasprag

Thanks for all your hard work on this, maintainers.

Until support for eslint v9 is ready, you can create a fresh v8 config with:

npm install -D eslint@8
npm init @eslint/config@0

sethlivingston avatar May 04 '24 17:05 sethlivingston

Hi, I'm listing all the problematic rules that I faced when migrated to eslint 9 with the eslint-plugin-react:

react/no-string-refs react/display-name react/no-direct-mutation-state react/require-render-return react/jsx-no-undef react/jsx-uses-vars react/no-typos react/no-array-index-key react/prefer-stateless-function react/no-access-state-in-setstate react/jsx-fragments react/jsx-max-depth react/jsx-no-constructed-context-values react/no-unstable-nested-components react/function-component-definition react/destructuring-assignment react/no-unused-prop-types

alaa-paramount avatar May 08 '24 05:05 alaa-paramount

I'm not a maintainer here, but listing the details of what's failing is not going to help this move forward. The list will be obvious to anyone running the tests with v9. They effectively amount to "me too" comments that are only really serving to spam everyone with email.

If you look around, you'll see there is ongoing work towards v9 by contributors - #3743 and #3727. Be patient or help out. Adding extra comments here isn't going to help.

Standard8 avatar May 08 '24 07:05 Standard8

I found the ESLint Compatibility Utilities worked for me. CodeSandbox example: https://codesandbox.io/p/devbox/billowing-tree-wp33ds

npm install --save-dev @eslint/compat

eslint.config.mjs:

import { fixupConfigRules } from "@eslint/compat";
import reactRecommended from "eslint-plugin-react/configs/recommended.js";
import globals from "globals";

export default [
  ...fixupConfigRules(reactRecommended),
  {
    languageOptions: {
      globals: {
        ...globals.browser,
      },
      parserOptions: {
        ecmaFeatures: {
          jsx: true,
        },
      },
    },
    rules: {
      "react/react-in-jsx-scope": "off",
    },
    settings: {
      react: {
        version: "detect",
      },
    },
  },
];

package.json:

{
  "scripts": {
    "lint": "eslint"
  },
  "dependencies": {
    "globals": "^15.2.0",
    "react": "^18",
    "react-dom": "^18"
  },
  "devDependencies": {
    "@eslint/compat": "^1.0.1",
    "eslint": "^9.2.0",
    "eslint-plugin-react": "^7.34.1"
  }
}

saltycrane avatar May 16 '24 21:05 saltycrane

Even with the use of @eslint/compat, installation remains an issue. Shall we continue to install eslint-plugin-react using npm install --force?

onlywei avatar May 17 '24 20:05 onlywei

@onlywei no, you should continue to hold off upgrading to eslint 9 until all of the eslint-related packages you use support it.

ljharb avatar May 17 '24 21:05 ljharb

I found the ESLint Compatibility Utilities worked for me.

@saltycrane thanks for your comment above, this was helpful!

For anyone who is not using the recommended config but rather just configuring the plugin and its rules manually, the fixupPluginRules compatibility util from @eslint/compat may be what you're looking for.

First, install @eslint/compat as a dev dependency. Then, use fixupPluginRules in your config:

eslint.config.mjs

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

export default [
  {
    languageOptions: {
      globals: {
        ...globals.browser,
      },
      parserOptions: {
        ecmaFeatures: {
          jsx: true,
        },
      },
    },
    plugins: {
      react: fixupPluginRules(react),
    },
    rules: {
      // Error on creating components within components
      // https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-unstable-nested-components.md
      'react/no-unstable-nested-components': 'error',
    },
    settings: {
      react: {
        version: 'detect',
      },
    },
  },
];

karlhorky avatar May 18 '24 15:05 karlhorky

@ljharb it looks like the code changes have been merged, and the only task left is to make the rule tests work with ESLint v9?

mdjermanovic avatar May 19 '24 12:05 mdjermanovic

@mdjermanovic those were just the deprecations, which are the easy part. next we need tested flat config support, and then figuring out how to test on eslint 9 since we need to simultaneously test on eslintrc in a bunch of older versions (unfortunately since RuleTester doesn’t obey the eslintrc env var, this work can’t be delayed until after shipping v9 support)

ljharb avatar May 19 '24 15:05 ljharb