Skip to content

Drop eslint-plugin-react so we can upgrade to ESLint 10 #2464

Description

@spaceninja

eslint-plugin-react is the only thing keeping this repo on ESLint 9, and it looks like we can drop it outright rather than wait for upstream.

The block

#2458 (ESLint 9 → 10, plus @cloudfour/eslint-config v26) can't install:

npm error ERESOLVE could not resolve
npm error Conflicting peer dependency: eslint@10.9.1
npm error   peer eslint@"^10.6.0" from @cloudfour/eslint-config@26.0.0

eslint-plugin-react@7.37.5 declares:

eslint: ^3 || ^4 || ^5 || ^6 || ^7 || ^8 || ^9.7

7.37.5 is latest, and the next tag is still 7.8.0-rc.0, so there's no prerelease with ESLint 10 support either. @cloudfour/eslint-config v26 dropped ESLint 9, so the two can't coexist.

Worth flagging that every check on #2458 is red, which makes it look worse than it is — the failure is at npm ci, so none of the lint, build, test or Storybook jobs ever ran. There is no evidence yet about whether our code actually passes v26.

What we use it for

One rule. From eslint.config.mjs:

plugins: { react },
rules: {
  'padding-line-between-statements': 'off',
  'react/jsx-uses-vars': 'error',
},

react/jsx-uses-vars marks JSX-referenced identifiers as used, so no-unused-vars doesn't flag them. We need it because of the 84 .mdx files, which all look like this:

import { Canvas, Controls, Meta } from '@storybook/addon-docs/blocks';
import * as AlertStories from './alert.stories.js';

<Meta of={AlertStories} />

Every one of those imports is referenced only inside JSX. Without the rule, no-unused-vars flags all of them.

So we can't just delete the dependency — but that doesn't mean we need a plugin.

Proposal

Drop eslint-plugin-react and turn off no-unused-vars for MDX instead:

{
  files: ['**/*.mdx'],
  rules: { 'no-unused-vars': 'off' },
},

The trade-off is that a genuinely unused import in an .mdx file stops being reported. That seems like a small price for removing the one dependency blocking a major ESLint upgrade — these are documentation files, and a stray import in one is close to harmless.

There's also one .jsx and one .tsx file in the repo that would need checking; they may want the same treatment or may be fine under @typescript-eslint's unused-vars handling.

I haven't verified that this is sufficient — it needs someone to make the change and run the suite. It's possible eslint-plugin-mdx surfaces these through a different rule, in which case the disable needs widening.

Alternatives considered

  • eslint-plugin-react-x / @eslint-react/eslint-plugin — peers eslint: "*" so it installs against 10, but at 5.18.6 it has no jsx-uses-vars equivalent (its unused-related rules are no-unused-props, no-unused-state and no-unused-class-component-members). Not a drop-in.
  • npm overrides to force the peer — would install, but ESLint 10 removed APIs and the plugin's range is probably honest rather than stale. That trades a clear failure for a silent one.
  • Wait for upstream — no signal on when, and it holds up the whole config v26 rollout in the meantime.

Wider context

This isn't only our problem. Because v26 requires eslint ^10.6.0, any repo depending on eslint-plugin-react is currently unable to adopt it. If other repos hit the same wall, the fix here is probably the pattern to copy — see cloudfour/eslint-config#710 for the broader v26 rollout discussion.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions