rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[eslint-patch] eslint-bulk-suppressions feature is incompatible with ESLint 9

Open Akaiyo opened this issue 9 months ago • 4 comments

Summary

The eslint-bulk-suppressions feature of eslint-patch scans for specific code only present in ESLint < 9.x which makes it incompatible with this version.

Repro steps

I've tried to update ESLint to 9.x and ran into problems using the new flat config system. (As have other people, e.g. https://github.com/microsoft/rushstack/issues/5049, https://github.com/microsoft/rushstack/issues/4965). So instead I tried to update to 9.x using the legacy config system (setting env variable ESLINT_USE_FLAT_CONFIG=false). Loading the patch works but the patch process itself fails:

Oops! Something went wrong! :(

ESLint: 9.0.0

Error: Cannot read config file: << replaced-project-path >>.eslintrc.cjs

    at scanUntilMarker (<< replaced-project-path >>node_modules/@rushstack/eslint-patch/lib/eslint-bulk-suppressions/generate-patched-file.js:41:15)
    at generatePatchedLinterJsFileIfDoesNotExist (<< replaced-project-path >>node_modules/@rushstack/eslint-patch/lib/eslint-bulk-suppressions/generate-patched-file.js:177:19)
Error: Unexpected end of input while looking for "nodeQueue.forEach(traversalInfo => {"

    at Object.<anonymous> (<< replaced-project-path >>node_modules/@rushstack/eslint-patch/lib/eslint-bulk-suppressions/index.js:22:71)
    at Module._compile (node:internal/modules/cjs/loader:1369:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)
    at Module.load (node:internal/modules/cjs/loader:1206:32)
    at Module._load (node:internal/modules/cjs/loader:1022:12)
    at Module.require (node:internal/modules/cjs/loader:1231:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (<< replaced-project-path >>node_modules/@rushstack/eslint-patch/eslint-bulk-suppressions.js:1:1)
(node:22446) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details.
(Use `node --trace-warnings ...` to show where the warning was created)

The above run was with ESLint 9.0.0 but I've tried several 9.x versions, all fail.

Details

The reason for the failure is a code change in eslint which breaks the patch. The patch scans for the text nodeQueue.forEach(traversalInfo => { which exists in eslint versions < 9.x in the linter.js file as part of the runRules function. The location of the scanning code is eslint-patch: generate-patched-file.ts#L210. The location of the expected code is eslint: linter.js#L1106 On ESLint 9.x versions this line simply does not exist anymore and the patch breaks.

What makes matters worse is that version 1.10.3 of the eslint-patch package contains the following release note:

[eslint-patch] Allow use of ESLint v9

This makes it seem that it is compatible when in reality it is not. It was actually introduced by non other than the original creator of eslint himself @nzakas (thank you for your work on ESLint). However it seems the suppress feature was never actually tested on ESLint 9 and in my opinion the lines of the change (commit#777e9f86911e28e9eb293bc982f5cd7826780eb2) claiming it has been tested for 9.x should be reverted.

Standard questions

Question Answer
Package name: @rushstack/eslint-patch
Package version? 1.11.0
Operating system? Mac
Would you consider contributing a PR?
Node.js version (node -v)?

Akaiyo avatar Mar 12 '25 14:03 Akaiyo

I can confirm it doesn't support ESLint 9. I think the solution here would be to add a flag that disables the feature entirely for people using ESLint 9.

kevin-y-ang avatar Mar 16 '25 02:03 kevin-y-ang

FYI: We are working on adding suppressions directly into ESLint v9: https://github.com/eslint/eslint/pull/19159

Hopefully will ship soon.

nzakas avatar Mar 17 '25 14:03 nzakas

@nzakas - the RFC mentioned in the PR is somewhat significantly less granular than our bulk suppressions feature. The feature in @rushstack/eslint-patch scopes suppressions to AST nodes, not whole files. If the ESLint feature is going to lose that granularity, we may consider upgrading our patch to support ESLint 9.

iclanton avatar Mar 24 '25 21:03 iclanton

@iclanton we don't have plans to support AST nodes. You can always open an issue suggesting that enhancement.

I do just want to point out that there's going to be a lot of internal changes coming to ESLint over the next year, and so monkey-patching internal APIs is going to become more fraught.

nzakas avatar Mar 25 '25 14:03 nzakas