chore: prepare for ESLint 10 more - #458
Conversation
🦋 Changeset detectedLatest commit: 26bee1e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ESLint 10 to CI and dependencies, updates Node in CI, upgrades many Changes
Sequence Diagram(s)(Skipped — changes are dependency, test, and typing adjustments without new multi-component control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 5138a48 in 15 seconds. Click for details.
- Reviewed
174lines of code in7files - Skipped
1files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_LiVZ1hoaBxGC3B33
You can customize 
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
There was a problem hiding this comment.
Pull request overview
Prepares the project for upcoming ESLint 10 compatibility by expanding CI coverage, updating lint-related dependencies (notably @typescript-eslint/* and @babel/eslint-parser), and adjusting a handful of tests to accommodate new parser behaviors/APIs.
Changes:
- Add ESLint 10 to the CI version matrix and introduce an
eslint10devDependency alias. - Bump TypeScript ESLint and Babel ESLint parser dependencies/lockfile to versions compatible with newer ESLint/TypeScript parser behavior.
- Update/guard tests that rely on removed/changed ESLint APIs and parser restrictions.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Adds ESLint 10 to the CI matrix. |
package.json |
Updates peer/dev deps for @typescript-eslint/*, adds eslint10 alias. |
yarn.lock |
Lockfile updates reflecting dependency bumps and new ESLint 10 graph. |
src/utils/apply-default.ts |
Adjusts default-options handling to allow undefined. |
test/cli.spec.ts |
Pins “use-at-your-own-risk” import to the eslint9 alias. |
test/rules/no-unused-modules.spec.ts |
Adds ESLint version constraints to rule-tester setup. |
test/rules/no-empty-named-blocks.spec.ts |
Switches specific TS import-type cases to Babel parser. |
test/rules/order.spec.ts |
Switches a specific TS import-type case to Babel parser. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/apply-default.ts (1)
15-25:⚠️ Potential issue | 🟠 MajorPreserve user options when defaults are undefined.
With
defaultOptionsundefined,optionsbecomes[]and the merge loop never appliesuserOptions, so caller-provided options are dropped. This changes behavior for rules without defaults.🛠️ Suggested fix
- const options = defaultOptions - ? (structuredClone(defaultOptions) as AsMutable<Default>) - : ([] as AsMutable<Default>) - - if (userOptions == null) { - return options - } + if (defaultOptions == null) { + return structuredClone((userOptions ?? []) as Default) + } + const options = structuredClone(defaultOptions) as AsMutable<Default> + if (userOptions == null) { + return options + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/apply-default.ts` around lines 15 - 25, The current logic initializes options to [] when defaultOptions is undefined, which drops caller-provided userOptions; update the initialization and return path so userOptions are preserved: if defaultOptions is undefined and userOptions is non-null, set options to a mutable structured clone of userOptions (or return that clone directly) instead of [] so the subsequent merge/return yields the caller-supplied values; update code referencing defaultOptions, userOptions, options, structuredClone, and AsMutable<Default> accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/utils/apply-default.ts`:
- Around line 15-25: The current logic initializes options to [] when
defaultOptions is undefined, which drops caller-provided userOptions; update the
initialization and return path so userOptions are preserved: if defaultOptions
is undefined and userOptions is non-null, set options to a mutable structured
clone of userOptions (or return that clone directly) instead of [] so the
subsequent merge/return yields the caller-supplied values; update code
referencing defaultOptions, userOptions, options, structuredClone, and
AsMutable<Default> accordingly.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/rules/no-unused-modules.ts (1)
44-45: Stale commented-out@ts-expect-errordirective — consider removing the line entirely.The
// //@ts-expect-error`` is effectively dead code now thatshouldUseFlatConfigis typed as `any` (from the cast on line 26). Rather than leaving a double-commented directive, it would be cleaner to either remove it or replace it with a plain TODO comment.Suggested cleanup
- // // `@ts-expect-error` -- only available in ESLint v9 -- TODO: fix this with ESLint 10 types + // TODO: fix this with ESLint 10 types -- shouldUseFlatConfig is only available in ESLint v9🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rules/no-unused-modules.ts` around lines 44 - 45, Remove the stale double-commented "@ts-expect-error" directive next to the expression using shouldUseFlatConfig and ESLINT_USE_FLAT_CONFIG; update the line to be clean (either delete the commented directive entirely or replace it with a short TODO comment) because shouldUseFlatConfig is already cast to any on the earlier line (line with the cast) so the ts-expect-error is no longer needed — locate the expression referencing shouldUseFlatConfig and ESLINT_USE_FLAT_CONFIG to make this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/rules/no-unused-modules.ts`:
- Around line 44-45: Remove the stale double-commented "@ts-expect-error"
directive next to the expression using shouldUseFlatConfig and
ESLINT_USE_FLAT_CONFIG; update the line to be clean (either delete the commented
directive entirely or replace it with a short TODO comment) because
shouldUseFlatConfig is already cast to any on the earlier line (line with the
cast) so the ts-expect-error is no longer needed — locate the expression
referencing shouldUseFlatConfig and ESLINT_USE_FLAT_CONFIG to make this change.
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 @.changeset/hungry-owls-start.md:
- Line 2: The changeset currently labels "eslint-plugin-import-x" as a patch but
also tightens the peer dependency floor for "@typescript-eslint/utils" from
^8.0.0 to ^8.56.0, which is a breaking-of-compatibility within the same major
and should bump the package at least a minor; update the changeset
classification from "patch" to "minor" and expand the changeset description to
mention the tightened "@typescript-eslint/utils" peerDep floor so release notes
accurately reflect the compatibility contraction.
In `@package.json`:
- Line 65: The peer dependency floor for "@typescript-eslint/utils" was
tightened from ^8.0.0 to ^8.56.0 which can break consumers on 8.0.x–8.55.x, so
update the release type in the changeset to reflect this behavioral change: open
the .changeset/hungry-owls-start.md changeset referenced in the PR and change
its version bump from "patch" to "minor" (or alternatively relax the peerDep
back to the previous range if you intend it to be non-breaking); make sure the
changeset summary mentions the peerDep range narrowing and why it's a minor
semver change.
- Line 66: The peerDependencies range for "eslint" currently allows ^10.0.0 but
CI's eslint10 alias targets 10.0.1 only, so either narrow the peerDependencies
to ^10.0.1 or broaden CI to actually test 10.0.0; update package.json by
changing the "eslint" entry in peerDependencies (symbol: "eslint") to ^10.0.1 if
you want to require CI's tested version, or change the eslint10 alias (symbol:
"eslint10") to "npm:eslint@^10.0.0" so CI also exercises 10.0.0.
- Around line 97-98: The package versions are mixed:
`@babel/eslint-parser`@^8.0.0-rc.2 requires `@babel/core`@^8.0.0-rc.2 while the
project pins `@babel/core` and other `@babel/`* packages at ^7.x; resolve this by
either (A) upgrading `@babel/core` and all `@babel/`* packages (e.g., `@babel/core`,
`@babel/preset-env`, `@babel/preset-react`, `@babel/preset-typescript`,
`@babel/plugin-proposal-decorators`) to the compatible ^8.0.0 range, or (B)
downgrade `@babel/eslint-parser` to the latest stable ^7.x release so it matches
the existing `@babel/core`@^7.27.4; pick one approach and make the corresponding
package.json change to keep all `@babel/`* packages on the same major version.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
69-73: Heads-up:@babel/eslint-parser@8.0.0-rc.2is pinned to a release candidate.The step is correctly gated to
matrix.eslint == 10. Since this is a pre-release version, remember to update the pin when a newer RC or stable release of@babel/eslint-parserv8 ships — otherwise the ESLint 10 CI leg may silently fall behind.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 69 - 73, The CI step named "Install Babel RC Parser for ESLint 10" pins `@babel/eslint-parser`@8.0.0-rc.2 (a release candidate), which can become stale; update the workflow to remove or revise this hard pin when a newer RC or the stable v8 is released—locate the step with name "Install Babel RC Parser for ESLint 10" and replace the fixed `@babel/eslint-parser`@8.0.0-rc.2 reference with the new stable/RC version (or a variable like matrix.babel_parser_version) and add a short comment noting to bump it when v8 stabilizes so the ESLint 10 CI leg stays current.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 63-67: The workflow contains a duplicated job step named "Install
ESLint ${{ matrix.eslint }}" with the same if condition (`${{ matrix.eslint != 9
}}`) and identical run commands (`yarn add -D eslint@${{ matrix.eslint }}
eslint-plugin-unicorn@56` and `yarn --no-immutable`); remove the duplicate step
so the "Install ESLint ${{ matrix.eslint }}" block appears only once to prevent
running yarn add and yarn --no-immutable twice per job.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 69-73: The CI step named "Install Babel RC Parser for ESLint 10"
pins `@babel/eslint-parser`@8.0.0-rc.2 (a release candidate), which can become
stale; update the workflow to remove or revise this hard pin when a newer RC or
the stable v8 is released—locate the step with name "Install Babel RC Parser for
ESLint 10" and replace the fixed `@babel/eslint-parser`@8.0.0-rc.2 reference with
the new stable/RC version (or a variable like matrix.babel_parser_version) and
add a short comment noting to bump it when v8 stabilizes so the ESLint 10 CI leg
stays current.
0b6c663 to
f63d340
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sukka <github@skk.moe>
Pull request was closed
Another step toward #438.
@typescript-eslint/utilspeerDeps range to8.56.0, which also ensures our end-users won't face issues with ESLint 10.@typescript-eslint/parser,@typescript-eslint/rule-tester, and@typescript-eslint/utils: fixes ESLint 10@typescript-eslint/parser, we are facingA type-only import can specify a default import or named bindings, but not both.error. I fixed it by using the NBabel ESLint parser in those tests.LegacyESLinthas been removed, only testscli.spec.tswith ESLint 9 for nowno-unused-modulestests for ESLint 10 for now, proper fix is in refactor: makeno-unused-modulesno-op on ESLint 10 or later #457Important
Prepare for ESLint 10 by updating dependencies and adjusting tests for compatibility.
ci.yml.@typescript-eslint/utilspeer dependency to^8.56.0inpackage.json.@babel/eslint-parser,@typescript-eslint/parser,@typescript-eslint/rule-tester,@typescript-eslint/utilsto^8.56.0.no-empty-named-blocks.spec.tsandorder.spec.tsby using Babel parser for type-only import errors.no-unused-modulestests for ESLint 10 inno-unused-modules.spec.ts.LegacyESLintusage incli.spec.tsand testcli.spec.tswith ESLint 9 only.This description was created by
for 5138a48. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Chores
Tests