chore: update to ESLint v9 - #34812
Conversation
ESLint has been updated to v9 and the configuration converted to the new "flat" config format. Related packages were updated to versions that support ESLint v9. - `.eslintrc.js` becomes `eslint.config.mjs`, and `.eslintignore` becomes the global `ignores` entry. The config has to be an ES module because `eslint-plugin-import-x` ships separate CommonJS and ESM builds, and the plugin object registered here has to be identical to the one `@metamask/eslint-config-typescript` registers. - `tests/framework/.eslintrc.js` becomes `tests/framework/eslint-config.js`, with each glob rooted at `tests/` since `extends` inside an override used to intersect the outer and inner file globs. - `@react-native/eslint-config` is still eslintrc-only, so it is translated with `FlatCompat`. - Flat config no longer skips dot-directories, so `.github`, `.storybook`, and `.yarn` are ignored explicitly to keep the linted file set unchanged. - A few rules newly enabled by the dependency updates are turned off for now: jsdoc's `recommended-typescript-error` preset, and import-x's `no-rename-default` and `no-named-as-default-member`. `yarn lint` reports the same 0 errors as before, and the same warnings apart from the four `import-x/no-named-as-default-member` ones. Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning MetaMask internal reviewing guidelines:
|
…-v9-91b5 Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
There was a problem hiding this comment.
@cursor please check the diff between this file and the deleted tests/framework/.eslintrc.js
Is this an almost identical port, what has changed?
reply to this comment
There was a problem hiding this comment.
It's an almost identical port. I diffed the new file against the deleted tests/framework/.eslintrc.js, and every change is structural — no rule, severity, selector, or message text differs.
Concretely:
- Lines 1–111 are byte-identical. The
dualFrameworkRestrictedImportOptionsobject, the burndown imports, and all the messages are untouched. module.exports = { overrides: [...] }→module.exports = [...]. Flat config is a plain array of config objects, so theoverrideswrapper goes away. Same six entries, same order.excludedFiles→ignores. Same values, just the flat-config spelling.- Globs are now rooted at
tests/(**/specs/**→tests/**/specs/**, and so on). This is the one change that needed thought rather than mechanical translation, and it exists to preserve the old behaviour rather than change it.
On point 4: in the old setup these entries were reached through the root config's
{ files: ['tests/**/*.{js,ts}'], extends: ['./tests/framework/.eslintrc.js'] }When eslintrc pulls a config with its own overrides in through an extends inside an override, it ANDs the outer and inner file globs. So **/specs/**/*.{js,ts} only ever applied to files that were also under tests/. Flat config has no such nesting, so the entries are spread directly into the root array and the tests/ prefix is written out explicitly to reproduce that intersection. Without it these rules would have started applying to app/** too. The burndown arrays (tests/flows/..., tests/page-objects/...) were already repo-root-relative, so they are unchanged.
Verified empirically: no-restricted-imports reports 327 warnings both before and after, on the same files and lines — that rule is only configured by this file and the two root overrides, so the count matching exactly is a good signal the port is behaviour-preserving.
Note the burndown entries produce two warnings on a couple of smoke-appium lines (e.g. perps-add-funds.spec.ts:13), where one import matches two patterns in the same rule config. That happens identically on main — it's pre-existing, not introduced here.
There was a problem hiding this comment.
@cursor please validate the diff between this file and the deleted .eslintrc.js. Tell me how much is still the same vs what was changed.
Reply to this comment.
There was a problem hiding this comment.
I validated this by resolving the config rather than reading the diff, since a textual diff of a format migration isn't very informative. I checked out main in a second worktree, ran eslint --print-config for nine representative files in both trees, and compared the resulting rule maps (severity + options, normalising for the fact that v9 prints [0] where v8 printed ["off"]).
Result: ~500 rules per file resolve identically. Eight rules differ, and only two of those are my choice.
Files sampled: app/util/abTest.ts, app/util/bytes.test.ts, app/components/Nav/Main/index.js, app/components/Views/confirmations/utils/send.ts, app/components/UI/Ramp/utils/getOrderAmount.ts, app/components/hooks/useAsyncResult.ts, tests/framework/Assertions.ts, tests/page-objects/Browser/BrowserView.ts, scripts/react-compiler.js — chosen to hit the TS block, the JS block, the tests/framework block, the burndown/route-isolation blocks, and the scripts block.
Forced by ESLint v9's eslint:recommended (not my choice)
| Rule | Change |
|---|---|
no-new-symbol |
removed (deprecated, replaced below) |
no-extra-semi, no-inner-declarations, no-mixed-spaces-and-tabs |
dropped from recommended |
no-constant-binary-expression, no-empty-static-block, no-new-native-nonconstructor, no-unused-private-class-members |
added to recommended |
no-constant-binary-expression is the one that found a real issue, the gated-off {false && ...} in PerpsProOrderBookPanel. The other three new rules have zero violations repo-wide.
Deliberate, and called out in the PR description
import-x/no-named-as-default-member: warn → off. import-x v4 resolves where v0.5 didn't, taking this from 4 warnings to 841, nearly allReact.forwardRef-style namespace access. typescript-eslint recommends disabling it under TS.ft-flow/define-flow-type,ft-flow/use-flow-type: warn → off.eslint-plugin-ft-flow@2callscontext.getAllComments(), removed in v9, so it hard-crashes the run. No source file here is Flow-annotated.
Not a real difference
The comparison initially flagged ~36 rules as "options changed" — all false positives. ESLint v9's --print-config fills in each rule's schema defaults, so no-eval = ["error"] becomes no-eval = [2, {"allowIndirect": false}]. Same effective config.
One real bug this caught
Thanks for asking for this — the exercise found something I'd have missed. jsdoc/check-syntax and jsdoc/check-indentation had silently gone error → off. I build the "disable the newly-enabled jsdoc preset" list from jsdoc/flat/recommended-typescript-error, but that preset enumerates every jsdoc rule and only turns 27 on, so keying off Object.keys also clobbered the two that @metamask/eslint-config-typescript sets directly. Fixed in 0ed14ce by filtering to the rules the preset actually enables; both are back at error and yarn lint is still at 0 errors.
What is not captured by rule maps
Two things --print-config can't show, verified separately:
- File selection. Flat config stopped skipping dot-directories, which would have newly linted
.github/scripts,.storybook, and.yarn/plugins(217quoteserrors and 42 parse errors, since none of them are intsconfig.json). They're ignored explicitly to keep the linted set unchanged. - Rules with zero repo-wide violations, where an identical rule map still wouldn't prove the plugin is wired up. I tripped each deliberately with scratch files:
@metamask/design-tokens/color-no-hex,tailwindcss/classnames-order,tailwindcss/no-custom-classname, and theimport-x/no-restricted-pathsBN.js fence all fire.
The end-to-end check is that yarn lint reports 0 errors and 5935 warnings against 0 errors and 5939 on the main commit this branched from, with per-rule counts identical apart from the four no-named-as-default-member ones above.
There was a problem hiding this comment.
Following up with the "how much" half of the question — my earlier reply covered what changed behaviourally but didn't quantify the file itself.
File content: 76% carried over verbatim
Comparing the deleted .eslintrc.js against eslint.config.mjs, normalising indentation (a lot of the file got re-indented when overrides: [...] was unwrapped):
| Lines | |
|---|---|
Old .eslintrc.js |
948 |
New eslint.config.mjs |
1044 |
| Carried over verbatim | 722 — 76% of the old file, 69% of the new |
| Removed | 226 |
| Added | 322 |
What those ~550 changed lines actually are
Almost all of it is the eslintrc → flat translation rather than content:
- Structural churn. Unwrapping
module.exports = { overrides: [...] }into a top-level array, and wrapping parser settings inlanguageOptions: {}, accounts for most of it — the single largest groups of changed lines are},(16),{(5),],(5) and blank lines (14). - Keyword renames, same values on both sides:
excludedFiles:→ignores:(×3),plugins: ['jest']→plugins: { jest: jestPlugin }(×2),parser: '@babel/eslint-parser'→parser: babelParser(×2). - New header: 17 ESM
importlines and a 4-lineFlatCompatsetup. Flat config needs plugins passed as objects, so everything that used to be a bare string now needs an import. - New global
ignoresblock (~30 lines), replacing the deleted.eslintignore(19 lines) plus the oldignorePatterns. The extra lines are the dot-directory entries that flat config no longer skips by default, and comments explaining why. - Two new small blocks: a 6-line
linterOptions(forreportUnusedDisableDirectives) and an 8-line helper that builds the jsdoc disable map. - Reworded TODO comments on the two "shared config rules pending suppressions" blocks, so they refer to
@metamask/eslint-configv9 rather than reading as if they meant ESLint v9.
Genuinely new configuration: 5 rule lines and 2 parser options
Everything else in the file is the old content, reshaped. The only lines that change what gets reported:
282 'import-x/no-named-as-default-member': 'off',
286 'import-x/no-rename-default': 'off',
357 'ft-flow/define-flow-type': 'off',
358 'ft-flow/use-flow-type': 'off',
456 ...disabledJsdocRecommendedRules,
plus projectService: false and tsconfigRootDir in the TypeScript block — those exist to keep the old behaviour, pinning type-aware linting to the root tsconfig.json instead of letting @metamask/eslint-config-typescript@14's projectService: true infer a project per file.
Each of the five is justified in the PR description, and all five exist because a dependency changed underneath us, not because the migration needed them.
Cross-check
This lines up with the resolved-config comparison in my earlier reply: ~500 rules per file identical, 8 differing, and 6 of those 8 are ESLint v9's own eslint:recommended changes rather than anything in this file. So a 24% textual delta corresponds to a ~1.6% behavioural delta, and most of that isn't ours.
Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
The disable list was built from every key in jsdoc's `recommended-typescript-error` preset, but that preset lists all jsdoc rules and only turns 27 of them on. Disabling all of them also clobbered `check-syntax` and `check-indentation`, which `@metamask/eslint-config-typescript` sets directly and which were enforced before this upgrade. Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Performance Test Selection: |
|





Description
ESLint has been updated to v9, and the related packages have been updated to versions that support it. Our configuration has been converted to the new "flat" config format introduced with ESLint v9.
This unblocks bulk error suppression, which is what the existing "remove these modifications after the ESLint v9 update" blocks in the config are waiting on. Actually adopting suppressions is a follow-up; this PR only gets us onto v9 with the linted file set and reported problems unchanged.
ESLint v9 migration guide: https://eslint.org/docs/latest/use/migrate-to-9.0.0
ESLint "flat config" migration guide: https://eslint.org/docs/latest/use/configure/migration-guide
Modelled on the equivalent extension change, MetaMask/metamask-extension#44734.
Config layout
.eslintrc.js→eslint.config.mjs, and.eslintignore→ the globalignoresentry. It has to be an ES module:eslint-plugin-import-xships separate CommonJS and ESM builds, so the plugin object registered here must be imported (not required) to stay object-identical with the copy@metamask/eslint-config-typescriptregisters. Otherwise ESLint fails withCannot redefine plugin "import-x".tests/framework/.eslintrc.js→tests/framework/eslint-config.js. Each glob is now rooted attests/, because reaching these entries through anextendsnested inside atests/**/*.{js,ts}override used to intersect the outer and inner file globs.@react-native/eslint-configis still published in the legacy eslintrc format, so it is translated withFlatCompat.metro.transform.jsreads the config directly for code-fence linting, so it was updated for the flat format and the currentESLintconstructor options.Behaviour kept the same
.github/scripts,.storybook, and.yarn/plugins. Those are not covered bytsconfig.json, so type-aware rules cannot run on them; they are ignored explicitly to keep the linted file set unchanged.reportUnusedDisableDirectivesdefaults towarnin v9. The repo has ~800 stale directives and already opted out of the equivalenteslint-comments/no-unused-disablerule, so reporting is turned off for now.@metamask/eslint-config-typescript@14newly pulls in jsdoc'srecommended-typescript-errorpreset (~18k pre-existing violations), so the 27 rules that preset turns on are disabled. The jsdoc rules the shared config sets directly (check-syntax,check-indentation) stay enforced.eslint-plugin-import-x@4addedno-rename-defaultto itswarningspreset, which fires on the widespread and intentionalimport styles from './Foo.styles'pattern, so it is off.no-named-as-default-memberis also off: it is redundant with TypeScript and false-positives onReact.forwardRef-style namespace access.eslint-plugin-ft-flow(pulled in by@react-native/eslint-config) is not v9 compatible and crashes on load. No source file here is Flow-annotated, so its two rules are off rather than addingeslint-plugin-ft-flow/hermes-eslintupgrades to this PR.Source change
One real new finding, from
no-constant-binary-expressionbeing added toeslint:recommendedin v9: the intentionally gated-off{false && ...}block inPerpsProOrderBookPanel. Since the constant is the gate, it gets an inline disable rather than a behaviour change.Every other change under
app/in this PR is a comment — four lines in total, all updating.eslintrc.jsreferences toeslint.config.mjs.Changelog
CHANGELOG entry: null
Related issues
Fixes: N/A
Manual testing steps
The lint CI job should suffice for ensuring this still works. Verified locally:
yarn lintexits 0 with 0 errors and 5935 warnings, against 0 errors and 5939 warnings on themaincommit this branched from, so the two are directly comparable. The four fewer are theimport-x/no-named-as-default-memberwarnings disabled above; every other rule's warning count is identical. After mergingmainthe branch is still at 0 errors.eslint --print-configfor nine representative files (TS, JS, tests/framework, page-objects, scripts, burndown and route-isolation paths) in this branch and in amainworktree. ~500 rules per file resolve identically. The only differences are ESLint v9's owneslint:recommendedchanges (no-new-symbol,no-extra-semi,no-inner-declarations,no-mixed-spaces-and-tabsout;no-constant-binary-expression,no-empty-static-block,no-new-native-nonconstructor,no-unused-private-class-membersin) plus the two deliberate ones listed above. Details in the review threads.yarn lint:fixexits 0 and leaves the working tree clean.yarn format:check,yarn test:depcheck,yarn constraints,yarn dedupe --check, andyarn audit:ciall pass.yarn lint:tscis unchanged.mainand this branch OOM at--max-old-space-size=6144and8192, and both complete at the12288thelintscript already sets.metro.transform.jscode-fence lint path was exercised against a real fenced file (app/lib/snaps/preinstalled-snaps.ts) and passes.@metamask/design-tokens/color-no-hex,tailwindcss/classnames-order,tailwindcss/no-custom-classname, and theimport-x/no-restricted-pathsBN.js fence all report as expected.Known CI notes
Two checks are red for reasons outside this change:
Check PR Max Linesfails at 2578/1000. 95% of that is the two config files moving to new filenames — a rename plus rewrite counts as a full delete and a full add (.eslintrc.js948 deletions +eslint.config.mjs1044 additions, and 235 + 232 for the framework config). Everything else in the PR totals ~90 lines. There is no way under the limit without splitting the rename and the conversion into separate PRs. The check is non-required and applies thesize-XLlabel.appium-wallet-platform-ios-smoke-2fails on three tests inwallet/connections/evm-provider-events.spec.ts, all needing the local test dapp. The job log shows the runner failing to load the dapp server's native websocket binary (Cannot find module '../binaries/uws_darwin_arm64_137.node') andxcrun simctl privacy … grant notificationsfailing with "Simulator device failed to complete the requested operation". Since everyapp/change here is a comment, and the same build artifact passes the other five iOS shards plus all Android shards, this is runner flake and needs a re-run.Everything else is green: all 10 unit test shards, all component-view and integration shards, both native builds, and every lint/format/depcheck/constraints/audit/dedupe job.
Screenshots/Recordings
N/A — tooling-only change with no user-facing surface.
Before
N/A
After
N/A
Pre-merge author checklist
Performance checks (if applicable)
Not applicable — this change only touches lint tooling, and every
app/edit is a comment, so nothing reaches the app bundle.trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist