Upgrade to ESLint v10 and modernize lint config#284
Conversation
Upgrade eslint from v9 to v10 and rewrite eslint.config.mjs to use native flat config, removing FlatCompat and several dead/incompatible dependencies. - Bump eslint 9→10, @eslint/js 9→10 - Replace @typescript-eslint/eslint-plugin + parser with unified typescript-eslint - Replace eslint-plugin-import with eslint-plugin-import-x (maintained fork) - Remove dead packages: eslint-config-airbnb-base, eslint-config-standard, eslint-plugin-standard, eslint-plugin-node, eslint-plugin-react, eslint-plugin-promise, eslint-plugin-n, @eslint/compat, @eslint/eslintrc - devDependencies reduced from 16 to 8 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThis change refactors the ESLint configuration to use the typescript-eslint configuration wrapper ( 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eslint.config.mjs`:
- Line 165: The base ESLint rule "indent" (currently set as indent: ["error",
2]) can mis-handle TypeScript syntax; replace or disable it by setting indent:
"off" and rely on your formatter (e.g., Prettier) to enforce indentation, or
alternatively swap to the TypeScript-aware rule (historically
`@typescript-eslint/indent`) if you choose to keep ESLint enforcing indent; update
the rule definition where indent: ["error", 2] is declared to either indent:
"off" or the TypeScript-specific rule.
- Around line 16-21: The globals configuration currently expands browser globals
by mapping each key to "off" using
Object.fromEntries(Object.entries(globals.browser).map(...)); simplify by either
removing that expansion entirely if you don't need to override browser globals,
or replace it with a static empty object to explicitly disable them when
overriding upstream (i.e., stop using Object.fromEntries/Object.entries mapping
and instead omit the browser spread or use an explicit {}). Update the globals
object where "globals" is declared to apply one of these simpler approaches.
- Around line 169-180: The base ESLint rule "no-use-before-define" conflicts
with the TypeScript-aware rule "@typescript-eslint/no-use-before-define"
(different `variables` settings), so disable the base rule to avoid false
positives: update the ESLint config to turn off "no-use-before-define" (or
remove it) and keep only "@typescript-eslint/no-use-before-define" with the
existing options; target the rule key "no-use-before-define" in the config for
the change.
In `@package.json`:
- Around line 20-27: The package.json references non-existent versions for
ESLint-related packages; update the dependency versions for "eslint",
"@eslint/js", "typescript-eslint" (package name likely "@typescript-eslint/*" or
"typescript-eslint" based on your lockfile) and "eslint-plugin-import-x" to the
corresponding published versions on npm (e.g., set "eslint" and "@eslint/js" to
a valid 9.x release, set "eslint-plugin-import-x" to 4.9.4, and use the correct
`@typescript-eslint` package names/versions such as the latest stable
`@typescript-eslint/parser` and `@typescript-eslint/eslint-plugin` matching
TypeScript 5.x), then run npm install to verify; ensure you update the exact
package keys in package.json (symbols: "eslint", "@eslint/js",
"eslint-plugin-import-x", and the typescript-eslint package entries) rather than
adding new keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c0f6861-f067-45e9-9571-389ccad8b375
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
eslint.config.mjspackage.json
| globals: { | ||
| ...Object.fromEntries(Object.entries(globals.browser).map(([key]) => [key, "off"])), | ||
| ...globals.node, | ||
| ...globals.mocha, | ||
| GLOBAL: true, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying globals configuration.
The pattern to disable browser globals by mapping each to "off" works, but is verbose. If the goal is simply to not include browser globals, you could omit them entirely:
globals: {
- ...Object.fromEntries(Object.entries(globals.browser).map(([key]) => [key, "off"])),
...globals.node,
...globals.mocha,
GLOBAL: true,
},If you need to explicitly disable browser globals to override upstream configs, the current approach is valid.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| globals: { | |
| ...Object.fromEntries(Object.entries(globals.browser).map(([key]) => [key, "off"])), | |
| ...globals.node, | |
| ...globals.mocha, | |
| GLOBAL: true, | |
| }, | |
| globals: { | |
| ...globals.node, | |
| ...globals.mocha, | |
| GLOBAL: true, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eslint.config.mjs` around lines 16 - 21, The globals configuration currently
expands browser globals by mapping each key to "off" using
Object.fromEntries(Object.entries(globals.browser).map(...)); simplify by either
removing that expansion entirely if you don't need to override browser globals,
or replace it with a static empty object to explicitly disable them when
overriding upstream (i.e., stop using Object.fromEntries/Object.entries mapping
and instead omit the browser spread or use an explicit {}). Update the globals
object where "globals" is declared to apply one of these simpler approaches.
|
|
||
| "import/no-named-as-default": 0, | ||
| "import/no-deprecated": 2, | ||
| indent: ["error", 2], |
There was a problem hiding this comment.
Base indent rule may misbehave with TypeScript syntax.
The base ESLint indent rule can struggle with TypeScript-specific syntax (generics, type assertions, decorators). The @typescript-eslint/indent rule existed to address this but was deprecated in favor of dedicated formatters like Prettier.
If you encounter indentation errors on valid TS code, consider either:
- Adding
indent: "off"and relying on a formatter - Using
@typescript-eslint/indent(deprecated but functional)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eslint.config.mjs` at line 165, The base ESLint rule "indent" (currently set
as indent: ["error", 2]) can mis-handle TypeScript syntax; replace or disable it
by setting indent: "off" and rely on your formatter (e.g., Prettier) to enforce
indentation, or alternatively swap to the TypeScript-aware rule (historically
`@typescript-eslint/indent`) if you choose to keep ESLint enforcing indent; update
the rule definition where indent: ["error", 2] is declared to either indent:
"off" or the TypeScript-specific rule.
| "@typescript-eslint/no-use-before-define": ["error", { | ||
| variables: false, | ||
| typedefs: false, | ||
| functions: false, | ||
| classes: false, | ||
| }], | ||
|
|
||
| "no-use-before-define": ["error", { | ||
| variables: true, | ||
| functions: false, | ||
| classes: false, | ||
| }], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
typescript-eslint no-use-before-define rule documentation - should base ESLint rule be disabled?
💡 Result:
Yes, the base ESLint "no-use-before-define" rule should be disabled when using the @typescript-eslint/no-use-before-define rule. The typescript-eslint version is an extension rule that replaces the core ESLint rule, adding support for TypeScript features like type, interface, and enum declarations. The base rule can report incorrect errors in TypeScript code, so it must be turned off. Official configuration example from typescript-eslint docs (Flat Config): rules: { "no-use-before-define": "off", "@typescript-eslint/no-use-before-define": "error" } This prevents conflicts and ensures correct TypeScript-aware linting.
Citations:
- 1: https://typescript-eslint.io/rules/no-use-before-define
- 2: https://chromium.googlesource.com/devtools/devtools-frontend/+/703e6ab4eb9c6fe3ab41bd5e72d54a08ea015996/node_modules/%40typescript-eslint/eslint-plugin/docs/rules/no-use-before-define.md
Disable the base no-use-before-define rule to prevent conflicts with the TypeScript extension rule.
The @typescript-eslint/no-use-before-define rule is a TypeScript-aware replacement for the base ESLint rule. Having both enabled with different variables settings (false for TS rule, true for base) can cause incorrect errors in TypeScript code. According to typescript-eslint documentation, the base rule must be disabled.
Suggested fix
+ "no-use-before-define": "off",
"@typescript-eslint/no-use-before-define": ["error", {
variables: false,
typedefs: false,
functions: false,
classes: false,
}],
-
- "no-use-before-define": ["error", {
- variables: true,
- functions: false,
- classes: false,
- }],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@typescript-eslint/no-use-before-define": ["error", { | |
| variables: false, | |
| typedefs: false, | |
| functions: false, | |
| classes: false, | |
| }], | |
| "no-use-before-define": ["error", { | |
| variables: true, | |
| functions: false, | |
| classes: false, | |
| }], | |
| "no-use-before-define": "off", | |
| "@typescript-eslint/no-use-before-define": ["error", { | |
| variables: false, | |
| typedefs: false, | |
| functions: false, | |
| classes: false, | |
| }], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eslint.config.mjs` around lines 169 - 180, The base ESLint rule
"no-use-before-define" conflicts with the TypeScript-aware rule
"@typescript-eslint/no-use-before-define" (different `variables` settings), so
disable the base rule to avoid false positives: update the ESLint config to turn
off "no-use-before-define" (or remove it) and keep only
"@typescript-eslint/no-use-before-define" with the existing options; target the
rule key "no-use-before-define" in the config for the change.
The base ESLint rule can produce false positives on TypeScript code. Disable it and rely solely on @typescript-eslint/no-use-before-define. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Overview
Upgrade eslint from v9 to v10 and rewrite
eslint.config.mjsto use native flat config, removing FlatCompat and several dead/incompatible dependencies.This replaces the piecemeal
@eslint/jsv10 bump from #280, which created an unsupported split (@eslint/jsv10 witheslintv9 runtime). This PR upgrades everything together.Key changes:
@typescript-eslint/eslint-plugin+@typescript-eslint/parserwith the unifiedtypescript-eslintpackageeslint-plugin-importwitheslint-plugin-import-x(actively maintained fork with eslint v10 support)eslint-config-airbnb-base,eslint-config-standard,eslint-plugin-standard,eslint-plugin-node,eslint-plugin-react,eslint-plugin-promise,eslint-plugin-n,@eslint/compat,@eslint/eslintrceslint.config.mjsto usetseslint.config()with native flat config — no moreFlatCompatorfixupConfigRules/fixupPluginRuleswrappersAll existing lint rules are preserved. The
import/extensionsrule was disabled (alongside the already-disabledimport/no-unresolved) since TypeScript handles module resolution, andeslint-plugin-import-xtreats.jsextensions in TS imports differently than the old plugin.Why remove airbnb-base and standard?
These presets are dead projects that don't support eslint v10:
eslint ^7.32.0 || ^8.2.0eslint ^8.0.1There's no v10-compatible version of either, and no maintained fork. Most of what they provided falls into three buckets:
eslint:recommended+tseslint.configs.recommended— correctness rules likeno-undef,no-unused-vars, etc.===, specific spacing preferences) — these are the ones we lost. If any are missed, they can be added back explicitly.Risks
dist/output changed. The action itself is identical.import/extensionsdisabled — previously enforced "never use .js/.ts extensions in imports." Now disabled becauseeslint-plugin-import-xhandles TypeScript ESM.jsextensions differently. No functional impact since TypeScript validates this at compile time.eslint:recommendedrules from v10 (no-unassigned-vars,no-useless-assignment,preserve-caught-error) are now active. They passed clean on the current code, but could flag issues in future code — which is a positive.Checklist
yarn buildand committed resulting changes..github/workflows/test.ymlor explained why it doesn't make sense to do so.lintCI job validates the config works correctly. No workflow test changes needed.Important
After merging, make sure to create a new GitHub release and associated tag for this release.
You can either create the tag locally and then create a corresponding GitHub release,
or just create both the tag and release using the GitHub Release UI.
Additionally, if this is not a breaking change, make sure to update the
v1tag:🤖 Generated with Claude Code