react icon indicating copy to clipboard operation
react copied to clipboard

eslint-plugin-react-hooks & "Flat Config" (ESLint 9)

Open JoshuaKGoldberg opened this issue 1 year ago • 84 comments

👋 Coming over from https://github.com/eslint/eslint/issues/18093: ESLint is migrating to a new "flat config" format that will be the default in ESLint v9.

It doesn't look like eslint-plugin-react-hooks has documented support yet. But, based on searching around (e.g. https://github.com/vercel/next.js/discussions/49337), ESLint v9 is basically supported if you wire it up manually in your config:

// eslint.config.js
import eslint from "@eslint/js";
import hooksPlugin from "eslint-plugin-react-hooks";

export default [
  eslint.configs.recommended,
  {
    plugins: {
      "react-hooks": hooksPlugin,
    },
    rules: hooksPlugin.configs.recommended.rules,
  },
];

Most community plugins provide a more convenient wrapper. For example, eslint-plugin-jsdoc provides a jsdoc.configs['flat/recommended'] object:

// eslint.config.js
import jsdoc from 'eslint-plugin-jsdoc';

export default [
  jsdoc.configs['flat/recommended'],
];

Would the React team be open to a PR adding in a preset object like that? And either way, updating the docs on https://www.npmjs.com/package/eslint-plugin-react-hooks?

Note: this was also filed as https://github.com/reactjs/react.dev/issues/6430.

I'm posting this issue here as a reference & cross-linking it to the table in https://github.com/eslint/eslint/issues/18093. If there's anything technical blocking the extension from working with flat configs, please let us know - we'd be happy to try to help! 💜

Additional resources:

(sorry for not using the issue templates - I wasn't sure whether this would count as a bug)

JoshuaKGoldberg avatar Feb 13 '24 14:02 JoshuaKGoldberg

Thank you for reaching out. What would be involved in creating such a wrapper? Does it make more sense to file a PR and then see how it'll look?

We're only on ESLint 7 though so I guess we need to upgrade that first to test.

eps1lon avatar Feb 13 '24 15:02 eps1lon

What would be involved in creating such a wrapper?

It's not too much work (famous last words, I know). In theory the plugin should be able to get away with exporting an object containing configs, meta, and rules. https://eslint.org/docs/latest/extend/plugin-migration-flat-config#adding-plugin-meta-information shows a very minimal object.

make more sense to file a PR and then see how it'll look?

👍

We're only on ESLint 7 though so I guess we need to upgrade that first to test.

Yeah it feels like it'd probably be cleanest to upgrade to ESLint 8 first, just in case any breaking changes impact the plugin.

JoshuaKGoldberg avatar Feb 13 '24 15:02 JoshuaKGoldberg

Note that even wiring up the plugin manually doesn't enable it to work with the now-released ESLint 9: the plugin currently uses APIs like context.getSource which don't exist any more (https://github.com/eslint/eslint/issues/17520), so there'll need to be some upgrade work.

e.g. https://github.com/facebook/react/blob/2acfb7b60922bdc8376dd144ca7bc532df78254b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L529

RoystonS avatar Apr 05 '24 23:04 RoystonS

Eslint officially released v9.0.0 today. However, the eslint-plugin-react-hooks plugin still restricts eslint to v8 in the peerDependencies list, so nobody who uses the plugin can download v9 without breaking dependency resolution.

Hope you'll add support for v9 soon!

JstnMcBrd avatar Apr 06 '24 06:04 JstnMcBrd

Should we raise a separate issue for v9 API support?

RoystonS avatar Apr 06 '24 14:04 RoystonS

Should we raise a separate issue for v9 API support?

Currently under discussion in jsx-eslint/eslint-plugin-react#3699.

yhx-12243 avatar Apr 06 '24 15:04 yhx-12243

ESLint v9 support will be done in https://github.com/facebook/react/pull/28773

eps1lon avatar Apr 06 '24 15:04 eps1lon

@eps1lon how far away from merge/release is this update to support eslint v9?

josiahgallen avatar Apr 18 '24 16:04 josiahgallen

ESLint v9 support will be done in #28773

Hey @eps1lon, greetings!

I just wanted to ask, is #28773 part of the 4.6.0 release that was published on npm 3 days ago or it's planned for a future release?

I am currently in the process of migrating to flat config, and the plugin is giving me an error.

Oops! Something went wrong! :(

ESLint: 9.1.0

TypeError: context.getSource is not a function
Occurred while linting /home/user/projects/eslint-flat-config-migration/src/index.tsx:8
Rule: "react-hooks/exhaustive-deps"
    at visitFunctionWithDependencies (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1704:42)
    at visitCallExpression (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1759:11)
    at ruleErrorHandler (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1115:48)
    at /home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at runRules (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1154:40)

If needed, here is a my repository where the issue can be reproduced by running npx eslint src/index.tsx

https://github.com/virtuallyunknown/eslint-flat-config-migration

Relevant files: https://github.com/virtuallyunknown/eslint-flat-config-migration/blob/master/eslint.config.js https://github.com/virtuallyunknown/eslint-flat-config-migration/blob/master/configs/react.js

Cheers!

virtuallyunknown avatar Apr 22 '24 14:04 virtuallyunknown

ESLint v9 support will be done in #28773

Hey @eps1lon, greetings!

I just wanted to ask, is #28773 part of the 4.6.0 release that was published on npm 3 days ago or it's planned for a future release?

I am currently in the process of migrating to flat config, and the plugin is giving me an error.

Oops! Something went wrong! :(

ESLint: 9.1.0

TypeError: context.getSource is not a function
Occurred while linting /home/user/projects/eslint-flat-config-migration/src/index.tsx:8
Rule: "react-hooks/exhaustive-deps"
    at visitFunctionWithDependencies (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1704:42)
    at visitCallExpression (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1759:11)
    at ruleErrorHandler (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1115:48)
    at /home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at runRules (/home/user/projects/eslint-flat-config-migration/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1154:40)

If needed, here is a my repository where the issue can be reproduced by running npx eslint src/index.tsx

https://github.com/virtuallyunknown/eslint-flat-config-migration

Relevant files: https://github.com/virtuallyunknown/eslint-flat-config-migration/blob/master/eslint.config.js https://github.com/virtuallyunknown/eslint-flat-config-migration/blob/master/configs/react.js

Cheers!

got the same error and I turn off the rules about react-hooks...

JacobZyy avatar Apr 22 '24 17:04 JacobZyy

The issue seems to have been resolved in the canary release.

If you are using eslint-plugin-react, some rules such as react/display-name or other recommended ones don't work: https://github.com/jsx-eslint/eslint-plugin-react/issues/3699.

smaven avatar Apr 25 '24 05:04 smaven

If you are using eslint-plugin-react

eslint-plugin-react is not authored by the React team. Please file an issue for this plugin in https://github.com/jsx-eslint/eslint-plugin-react instead.

ESLint v9 support is available in the Canary release channel (earliest version 5.1.0-canary-cb151849e1-20240424) and will be released as stable with the stable release of React 19.

eps1lon avatar Apr 25 '24 09:04 eps1lon

I am not sure if this feedback is of any value, but it looks like the types from eslint and typescript-eslint are signaling about potential issues.

// @ts-check

// @typescript-eslint/utils: 7.7.1
// eslint-plugin-react-hooks: 5.1.0-canary-cb151849e1-20240424

import eslintPluginReactHooks from 'eslint-plugin-react-hooks';

/** @type {import('eslint').Linter.FlatConfig[]} */
export const configuration1 = [
    {
        plugins: {
            'react-hooks': eslintPluginReactHooks
        }
    }
];

/** @type {import('@typescript-eslint/utils').TSESLint.FlatConfig.ConfigFile} */
export const configuration2 = [
    {
        plugins: {
            'react-hooks': eslintPluginReactHooks
        }
    }
];

I don't want to paste the error message because it's a giant wall of text, but feel free to try it on your own.

This appears to be a type error only, and as far as I can tell the plugin is working as expected, so thanks for the updates, much appreciated!

virtuallyunknown avatar Apr 25 '24 13:04 virtuallyunknown

I'm getting this error using v4.6.2 with ESLint 9.1.1 and React 18.2.0:

ESLint: 9.1.1

TypeError: context.getScope is not a function
Occurred while linting /Users/me/Documents/frontend/src/components/CTable/CTable.tsx:1013
Rule: "react-hooks/exhaustive-deps"
    at visitCallExpression (/Users/me/Documents/frontend/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1780:34)
    at ruleErrorHandler (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/linter.js:1115:48)
    at /Users/me/Documents/frontend/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at runRules (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/linter.js:1154:40)
    at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/Users/me/Documents/frontend/node_modules/eslint/lib/linter/linter.js:1880:31)

movestill avatar May 08 '24 13:05 movestill

@movestill I think you need to use canary builds until React 19 launches if you want compatibility with ESLint 9.

https://github.com/facebook/react/issues/28313#issuecomment-2076798972

virtuallyunknown avatar May 08 '24 18:05 virtuallyunknown

@movestill I think you need to use canary builds until React 19 launches if you want compatibility with ESLint 9.

#28313 (comment)

Thanks for clarifying!

movestill avatar May 09 '24 13:05 movestill

@eps1lon Thanks for the PR https://github.com/facebook/react/pull/28773. Do you have an example of how to configure it in a flat config?

rwu823 avatar May 10 '24 12:05 rwu823

Thanks for the PR https://github.com/facebook/react/pull/28773. Do you have an example of how to configure it in a flat config?

No, since we haven't worked on supporting flat config yet. Would love if somebody can take a look. The linked PR only ensured compat with ESLint v9 and the old config format.

eps1lon avatar May 10 '24 13:05 eps1lon

@eps1lon Thanks for the PR #28773. Do you have an example of how to configure it in a flat config?

I think you may find my flat configuration migration repository useful. Take a look at these files:

React config: https://github.com/virtuallyunknown/eslint-flat-config-migration/blob/master/configs/react.js

Main config: https://github.com/virtuallyunknown/eslint-flat-config-migration/blob/master/eslint.config.js

And please do note that I am using canary release, this will not work with version 4.6.2 of the react-hooks eslint plugin.

Also, note that I am just enabling a few rules here and there for testing purposes. If you want to have all rules or recommended rules from some plugin, you can do it like this:

/** @type {import('@typescript-eslint/utils').TSESLint.FlatConfig.ConfigFile} */
export const eslintPluginReactConfig = [
    {
        languageOptions: {
            parserOptions: {
                ecmaFeatures: {
                    jsx: true
                }
            }
        },
        plugins: {
            'react': eslintPluginReact,
            'react-hooks': eslintPluginReactHooks
        },

        // here include all rules you want to have enabled,
        // for example eslintPluginReactHooks.configs.recommended.rules
        // you may have to verify that this works for all plugins

        rules: {
            'react/void-dom-elements-no-children': 'error',
            ...eslintPluginReactHooks.configs.recommended.rules
        }
    }
];

virtuallyunknown avatar May 10 '24 14:05 virtuallyunknown

The new ESLint Compatibility Utilities seems to work well for me using [email protected]

GalindoSVQ avatar May 14 '24 15:05 GalindoSVQ

works for me:

"@eslint/compat": "^1.0.1",
"eslint-plugin-react-hooks": "^4.6.2",
"eslint": "9.x",
...

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

 {
   plugins: {
     'react-hooks': fixupPluginRules(eslintPluginReactHooks)
   },
   rules: {
     ...eslintPluginReactHooks.configs.recommended.rules,
   }
 },
...

rom-object avatar May 19 '24 09:05 rom-object

The following configuration uses typescript-eslint, eslint-plugin-react, eslint-plugin-react-hooks, and eslint-plugin-prettier(eslint-config-prettier). It works well:

// @ts-check

import globals from 'globals';
import eslint from '@eslint/js';
import tseslint from 'typescript-eslint';
import { fixupPluginRules } from '@eslint/compat';
import eslintPluginReact from 'eslint-plugin-react';
import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended';
import eslintPluginReactHooks from 'eslint-plugin-react-hooks';

export default tseslint.config(
  eslint.configs.recommended,
  ...tseslint.configs.recommended,
  eslintPluginPrettierRecommended,

  { files: ['**/*.{js,mjs,cjs,ts,jsx,tsx}'] },
  {
    languageOptions: {
      parserOptions: { ecmaFeatures: { jsx: true } },
      globals: { ...globals.browser },
    },
  },
  {
    plugins: {
      'react': eslintPluginReact,
      'react-hooks': fixupPluginRules(eslintPluginReactHooks),
    },
  },
  {
    rules: {
      // ...
      ...eslintPluginReactHooks.configs.recommended.rules,
    },
  },
);

romantech avatar Jun 20 '24 15:06 romantech

@romantech when i try to do this i get a missing type declaration error at import eslintPluginReactHooks from 'eslint-plugin-react-hooks';. i think this package needs to provide type declarations

pauliesnug avatar Jun 26 '24 15:06 pauliesnug

@pauliesnug

i think this package needs to provide type declarations

  • https://github.com/facebook/react/issues/30119

Yeah it doesn't, and there's no @types/... package for it either. You'll need to create your own type declaration like this:

declare module 'eslint-plugin-react-hooks' {
  ...,
}

I have an example in a personal project here, if that helps.

JstnMcBrd avatar Jun 26 '24 18:06 JstnMcBrd

What's the status? It still doesn't work with the newest nightly build.

yf-hk avatar Jul 25 '24 01:07 yf-hk

What's the status? It still doesn't work with the newest nightly build.

What's not working? Did you follow https://github.com/facebook/react/issues/28313#issuecomment-2180984628?

eps1lon avatar Jul 25 '24 08:07 eps1lon

What's the status? It still doesn't work with the newest nightly build.

What's not working? Did you follow #28313 (comment)?

My guess is most of us watching this issue are waiting to be able to simply update the version in our package file, not apply an unofficial temporary workaround with a new package. I know that's where we stand.

ziemkowski avatar Jul 25 '24 14:07 ziemkowski

Getting the Definition for rule 'react-hooks/exhaustive-deps' was not found react-hooks/exhaustive-deps error in the console does anyone have the same issue?

I have tried using both canary and 4.6.2.

My config file looks like this (eslint.config.mjs):

// @ts-check
import globals from "globals";
import pluginJs from "@eslint/js";
import pluginReact from "eslint-plugin-react";
import tseslint from "typescript-eslint";
import { fixupPluginRules } from "@eslint/compat";
import reactHooks from "eslint-plugin-react-hooks";
import eslintPluginPrettierRecommended from "eslint-plugin-prettier/recommended";
import eslintConfigPrettier from "eslint-config-prettier";

import tsParser from "@typescript-eslint/parser";
import tsPlugin from "@typescript-eslint/eslint-plugin";

export default [
  pluginJs.configs.recommended,
  pluginReact.configs.flat.recommended,
  ...tseslint.configs.recommended,
  {
    files: ["**/*.{js,mjs,cjs,ts,jsx,tsx}"],

    languageOptions: {
      globals: globals.browser,
      parser: tsParser
    },
    plugins: {
      react: pluginReact,
      "@typescript-eslint": tsPlugin,
      "react-hooks": fixupPluginRules(reactHooks)
    },

    rules: {
      "no-unused-vars": [
        "warn",
        {
          varsIgnorePattern: "^_",
          argsIgnorePattern: "^_"
        }
      ],
      "react/react-in-jsx-scope": "off",
      ...reactHooks.configs.recommended.rules
    }
  },
  eslintPluginPrettierRecommended,
  eslintConfigPrettier
];

Efekahya avatar Jul 25 '24 14:07 Efekahya

My guess is most of us watching this issue are waiting to be able to simply update the version in our package file, not apply an unofficial temporary workaround with a new package. I know that's where we stand.

I understand and that's not what I was suggesting. Just trying to find out if we're actually compatible. If people cannot get the Canary to work, we couldn't release a stable. My suggestion was targeted at the person reporting the issue which conflicts with prior reports that it was working.

eps1lon avatar Jul 25 '24 14:07 eps1lon

@eps1lon 5.1.0-rc.0 works well with ESLint v9 and flat config without @eslint/compat. However, I think it would be better to merge #29770 before creating a new version to make the migration process smoother.

latin-1 avatar Jul 28 '24 09:07 latin-1