feat(devops): add Oxlint alongside ESLint for incremental migration#3866
feat(devops): add Oxlint alongside ESLint for incremental migration#3866wraith4081 wants to merge 2 commits intoopenfrontio:mainfrom
Conversation
WalkthroughOxlint is integrated as a primary linter alongside ESLint through configuration, package scripts, CI workflow updates, and pre-commit hook adjustments. Documentation is updated to reflect the linting infrastructure changes. ChangesLinting Infrastructure Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
23-23: ⚡ Quick winRun both linters in
lint:github, even if one fails.Using
&&stops at the first failing linter, so ESLint results can be hidden in the same CI run. That weakens the migration safety-net feedback loop.Proposed change
- "lint:github": "oxlint --format=github && eslint --format gha", + "lint:github": "concurrently --success=all \"oxlint --format=github\" \"eslint --format gha\"",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 23, The lint:github npm script currently uses `&&` which stops at the first failure; update the "lint:github" script so it runs both linters regardless of the first exit code and then returns a non-zero exit code if either failed (for example, run oxlint then eslint sequentially capturing each exit status and exit with a non-zero code if either failed). Modify the "lint:github" script entry in package.json (the script key "lint:github") to implement this behavior so both linters always run but the CI still fails when any linter fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@package.json`:
- Line 23: The lint:github npm script currently uses `&&` which stops at the
first failure; update the "lint:github" script so it runs both linters
regardless of the first exit code and then returns a non-zero exit code if
either failed (for example, run oxlint then eslint sequentially capturing each
exit status and exit with a non-zero code if either failed). Modify the
"lint:github" script entry in package.json (the script key "lint:github") to
implement this behavior so both linters always run but the CI still fails when
any linter fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a943748-8fe8-4c21-b3d3-8ad1e98c5cdc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.github/workflows/ci.yml.husky/pre-commit.oxlintrc.jsonCLAUDE.mdCONTRIBUTING.mdREADME.mdpackage.json
Description:
Adds Oxlint to the project without removing ESLint yet.
This is intended as a first migration step: Oxlint now runs in local lint scripts, CI, lint-staged, and the pre-commit hook, while ESLint stays in place to keep the existing lint coverage unchanged.
Changes
oxlintandoxlint-tsgolint..oxlintrc.json.npm run lintandnpm run lint:fixto run Oxlint first, then ESLint.lint-stagedso Oxlint and ESLint only run on JS/TS files.Why not replace ESLint yet?
Oxlint looks promising and is much faster, but it is not a clean drop-in replacement for this repo yet.
When type-aware Oxlint defaults were enabled, it reported a lot of issues that our current ESLint setup does not enforce, including:
typescript/no-floating-promisestypescript/restrict-template-expressionstypescript/unbound-methodtypescript/no-base-to-stringThere were also a few behavior differences around
no-unused-vars. (where Oxlint reported issues that the current ESLint setup does not fail on)Because of that, this PR keeps ESLint as the safety net and only enables a small Oxlint rule set for now.
Current Oxlint rules
Oxlint’s default categories are disabled for now so this PR does not turn into a large unrelated lint cleanup.
Enabled rules:
typescript/prefer-nullish-coalescingeqeqeqno-case-declarationsESLint still handles the existing recommended JS/TypeScript checks and current unused-var behavior.
Notes
oxlint-tsgolintis needed becausetypescript/prefer-nullish-coalescingis type-aware.lint-stagedonly passes JS/TS files to Oxlint because Oxlint exits with an error when given unsupported files like Markdown.Local timing
Initial local timings:
8.967s0.775swall time (350msreported by Oxlint over 474 files)This is not a full parity benchmark yet, since Oxlint is currently running a smaller rule set, but it gives us a good reason to keep migrating rules over. (Ryzen 9 7940HS, 8x2 Thread, 40GB 4800MT/s DDR5, 990 Pro, Node v25.8.1)
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
wraith4081