feat: support ESLint v9

[G-Rath]

Resolves #2948

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.07%. Comparing base (55fa203) to head (a5020a6).

Files with missing lines	Patch %	Lines
src/rules/no-default-export.js	66.66%	1 Missing :warning:
src/rules/no-named-export.js	66.66%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2996      +/-   ##
==========================================
- Coverage   95.20%   95.07%   -0.14%     
==========================================
  Files          82       82              
  Lines        3565     3571       +6     
  Branches     1245     1251       +6     
==========================================
+ Hits         3394     3395       +1     
- Misses        171      176       +5     

Flag	Coverage Δ
	95.07% <77.77%> (-0.14%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[G-Rath]

@ljharb I'm probably going to knock off soon - would you mind having a look over both the changes and CI, and leave me some pointers for the coming days?

In addition to just the standard "how do you feel about this code", I'm not sure why some of the builds are failing with "Class constructor RuleTester cannot be invoked without 'new'" as I'm 99% sure my JS syntax is correct so I'd say the compiler is doing it wrong but that seems unlikely...? That could be related to why I found I couldn't use export function even though export default was being used elsewhere...

Also I've not yet looked into the utils package (?) - it looks like there's at least one function call there that will require a new argument.

But hey we've got about ~half~ some of the jobs passing :D

[ljharb]

I've just gotten back from travel, so I'll try to take a look at it in the coming days.

One likely obstacle is that all the tests assume eslintrc, but eslint 9 requires an env var to support it, otherwise it defaults to flat config. The initial support needs to test eslintrc, and it would be fine to add flat config tests in a follow-on if needed.

It's likely that the way eslint < 9's RuleTester works is subtly different than in eslint 9, so we may need an abstraction to handle that.

[G-Rath]

Yup part of this includes switching to a custom rule tester that converts the tests from eslintrc to flat config if they're running on v9, which is what we've been using in eslint-plugin-jest without issue.

[ljharb]

That's great, but testing eslintrc on eslint 9 is also very important.

It'd also be great if there was a commit or two I could land separately, that worked on eslint < 9, so that only the 9-specific parts remained in this PR after that was landed and rebased :-)

[G-Rath]

If you're meaning the context helper stuff, yeah I'm happy to pull them out I just figured you'd have asked them to be tested against ESLint v9 to prove they actually worked :-)

[ljharb]

You figured right :-) but i can just cherry-pick the commits out of this PR once things are working.

[G-Rath]

Right well they're already good for cherry-picking - you want to pick from e31fe5c8...543e3786 (sans f0853cb6 once #2997 is landed), and away you go.

I think you can have the eslintrc-on-v9 support by just running the whole test suite twice with the env enabled and an extra test in the custom rule tester - I'll push that up shortly

[G-Rath]

@ljharb RuleTester does not support eslintrc in v9 - the env variable is only used by the CLI

[ljharb]

well that sucks, we’ll need to file an eslint issue about that gap.

[G-Rath]

I'll leave that to you as I suspect you'll have more pull :-)

[ljharb]

Filed https://github.com/eslint/eslint/issues/18292.

[G-Rath]

Can you help me understand what withoutAutofixOutput is for

That was a helper which already existed in one of the tests, so I decided to pull it out as a utility for the others for consistency and then it can be cherry picked ahead of v9.

As for why it's required, v9 rule tester considers it an error for the output to match the code - using this utility means (once cherry picked) in this PR we can make one change to the utility (to make output null) rather than have to change the tests for every single rule.

---

Also while I don't mind input, just confirming I'm still chipping away at the general list of failures - I plan to come back later to address the finer details like adding comments :)

If someone has the time to look at the failures especially during my night, that's be appreciated - it looks like there's some around regression tests invoking eslint programmatically that I've not been able to figure out how to change while ensuring they're still testing what they're meant to be testing. Even if someone can figure out the general solution, Im happy to handle applying it to all the tests of that type

[ljharb]

Totally, while it's still a draft I'm indeed just chipping away at the review :-) your effort is appreciated!

[pumano]

any news on this PR?

[pumano]

any news on this?

[ljharb]

@pumano please stop spamming. if there is any news it will be posted on the PR.

eslint 9 support likely requires flat config support first. see #2873.

[G-Rath]

Just to add an update from my end: I don't mind continuing to help on this, but I couldn't make progress on the remaining issues at least some of which tie into how ESLint v9 behaviours and what @ljharb would like to see (including things like https://github.com/eslint/eslint/issues/18292)

In particular, from memory there was a section of tests that invoked ESLint programmatically that I think require a decision to be made that should be done with someone with more context on what the actual goal of those tests are, and there is also a section of runtime code around resolving a parser that is throwing because it seems that maybe in ESLint v9 if you use the default parser that path is no longer included (I mentioned this on an ESLint issue, which I now can't find and GitHub doesn't seemed to have linked 😞).

I think overall a good next step would be for @ljharb to review the test suite locally, and provide at least a high-level overview of what the likely best action to explore for each type of failure is (or as much as they're willing/able for now); then people can try to chip away at them.

[karlhorky]

@ljharb would eslint-plugin-import consider publishing a prerelease version with the current version of this PR?

I would be open to upgrading and testing this in our projects.

[ljharb]

@karlhorky that seems premature; we don't even have flat config support yet.

[karlhorky]

Oh interesting, I've been using eslint-plugin-import with our Flat Config for a while now (with ESLint v8 Flat Config support):

import eslintImport from 'eslint-plugin-import';

/** @type {import('@typescript-eslint/utils/ts-eslint').FlatConfig.ConfigArray} */
const configArray = [
  {
    plugins: {
      import: eslintImport,
    },
  },
];

https://github.com/upleveled/eslint-config-upleveled/blob/db1ea93f07173ed9fdaaf8a4cbe8748e3a698c05/index.js#L6

https://github.com/upleveled/eslint-config-upleveled/blob/db1ea93f07173ed9fdaaf8a4cbe8748e3a698c05/index.js#L496

Worked pretty much out of the box.

I did need to add some types for TypeScript, but those were also pretty straightforward:

https://github.com/upleveled/eslint-config-upleveled/blob/db1ea93f07173ed9fdaaf8a4cbe8748e3a698c05/types/eslint-plugin-react.d.ts

[karlhorky]

Workaround (switch to eslint-plugin-import-x)

In case anyone wants to upgrade to ESLint v9 now, an alternative is to switch to the eslint-plugin-import-x fork:

1. Install eslint-plugin-import-x
2. Install @typescript-eslint/parser for TypeScript support

-import eslintImport from 'eslint-plugin-import';
+import eslintImportX from 'eslint-plugin-import-x';

/** @type {import('@typescript-eslint/utils/ts-eslint').FlatConfig.ConfigArray} */
const configArray = [
  {
    plugins: {
-     import: eslintImport,
+     'import-x': eslintImportX,
    },
    settings: {
      'import-x/parsers': {
        '@typescript-eslint/parser': ['.ts', '.tsx'],
      },
      'import-x/resolver': {
        // Load <rootdir>/tsconfig.json
        typescript: true,
        node: true,
      },
    },
    rules: {
      // Error on imports that don't match the underlying file system
-     // https://github.com/import-js/eslint-plugin-import/blob/master/docs/rules/no-unresolved.md
-     'import/no-unresolved': 'error',
+     // https://github.com/un-ts/eslint-plugin-import-x/blob/master/docs/rules/no-unresolved.md
+     'import-x/no-unresolved': 'error',
  },
];

[rishavpandey43]

Hi,

Recently, while upgrading eslint this plugin broke in my project.

Just FYI, I'm getting this error - Parse errors in imported module '@pay/old/api/endpoints': parserPath or languageOptions.parser is required! (undefined:undefined) import/namespace

Official eslint documentation says to file an issue with the particular plugin, so I'm pointing it out here.

Thanks in advance, if this is handled.

[ljharb]

@rishavpandey43 this plugin explicitly does not yet support eslint 9, so you can't upgrade yet.

[bmulholland]

To take a risk, in that I don't fully understand the current status and I haven't talked to anyone, I'm seeing that there hasn't been progress here in a month or two (maybe I'm missing progress that's here or elsewhere, in which case I may well be overstepping here -- apologies if so!). Often, breaking work into smaller chunks can help restart progress. With that in mind, I thought I'd note that, especially with ESLint's compat options, it's possible to separate v9 support from flat config support.

It's even possible to run the plugin on v9, with flat config, in a mode that's intended to (1) patch the config rules for v9 removals, and (2) include eslintrc-style plugins in a flat config. In my experience, this mode works for many plugins, but not for eslint-plugin-import. This presents one possible, incremental, roadmap I wanted to propose:

1. Try eslint-plugin-import with both patches, fix the relatively fewer issues in this mode (assumes they're not coupled to bigger issues...)
2. Pick one of flat config support or v9 support, add that independently
3. Do the other one

Each of these would provide value as a release, as even just the first one would unblock ESLint v9 upgrades for users of this plugin.

For even smaller incremental options, for #1, could start by just releasing a version that temporarily disables/bypasses the rules that don't work, and incrementally fix them. Maybe those could even be small enough for newer contributors to take on?

Anyway, like I say, this all may be hopelessly coupled and impossible, or this work may be actively progressing behind the scenes, or any number of other things. I'm just an outsider chiming in :). Thanks for the plugin and good luck with the migration.

[l3ender]

Thanks for all who have worked on this!

Is it possible to have a prerelease version published following @bmulholland's [well-articulated] advice at https://github.com/import-js/eslint-plugin-import/pull/2996#issuecomment-2178153234? This would allow early adopters the ability to get on v9 while items are being worked.

Thanks!

[Gnuk]

@ljharb Do you know if something continue to block this PR? And if something we can do to help this support?

[cthompson-avb]

Why does this PR not update the package.json to include eslint v9?

  "peerDependencies": {
    "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8"
  },

[michaelfaith]

Now that https://github.com/import-js/eslint-plugin-import/pull/2873 has merged. How can I help see this change through? @G-Rath are you still up for moving this forward? If so, you may want to rebase.

[G-Rath]

How can I help see this change through

There's a number of things that need actioning which I've eluded to in https://github.com/import-js/eslint-plugin-import/pull/2996#issuecomment-2115906241 - there's also some requirements from @ljharb which I don't have a definitive list of but that I know exist and relate to the rest of the ecosystem e.g. having the v9 RuleTester test against both legacy and flat config.

If so, you may want to rebase.

I'll try and get to that next week, though I'm not really rushing at this point given the size of the test suite and number of different areas that I think require @ljharb's input which he's not yet provided - I don't think that means no further work can be done, but from what I've seen unless there's someone else with a lot of experience on this plugin who can confidently make calls around e.g. how the parser finder logic should behave, I think this will require Jordan to have some bandwidth which he's not yet been able to spare.