ci: add oxlint as fast pre-lint pass before ESLint#10355
ci: add oxlint as fast pre-lint pass before ESLint#10355benpsnyder wants to merge 3 commits intoTanStack:mainfrom
Conversation
…nfiguration - Added `eslint-plugin-oxlint` and `jiti` as dependencies. - Created `oxlint.config.ts` for custom linting rules and configurations. - Updated `eslint.config.js` to include oxlint settings and disable overlapping ESLint rules. - Modified package scripts to include a new `test:oxlint` command for running oxlint checks. - Adjusted test files to comply with new linting rules.
📝 WalkthroughWalkthroughIntegrates oxlint into the repo: adds Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as "ESLint Runner"
participant RootConfig as "root eslint.config.js"
participant Jiti as "jiti loader"
participant OxlintConfig as "oxlint.config.ts"
participant Oxlint as "oxlint.buildFromOxlintConfig"
participant Final as "Final ESLint config"
Runner->>RootConfig: require/import config
RootConfig->>Jiti: load './oxlint.config.ts' (runtime)
Jiti->>OxlintConfig: compile & return default export
RootConfig->>Oxlint: call buildFromOxlintConfig(oxlintConfig)
Oxlint-->>RootConfig: oxlint-generated config entries
RootConfig->>Final: append oxlint entries & plugin settings
Final-->>Runner: run lint with merged config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
- Added `eslint-plugin-unused-imports` to manage unused imports. - Updated `@tanstack/eslint-config` to version 0.4.0 for improved linting rules. - Modified ESLint configurations across multiple packages to include type annotations and ensure consistent exports. - Adjusted rules in `oxlint.config.ts` to allow shadowing and refined other linting rules. - Updated package dependencies in `pnpm-lock.yaml` to reflect the changes.
TkDodo
left a comment
There was a problem hiding this comment.
generally in favour of this, but what’s stopping us from replacing eslint completely with oxlint? I think it would even be fine if there isn’t a 1:1 mapping of the rules (dependent on which rule are missing of course). But I’d rather just have one linter than two ...
| "eslint": "^9.36.0", | ||
| "eslint-plugin-oxlint": "^1.57.0", | ||
| "eslint-plugin-react-hooks": "^6.1.1", | ||
| "eslint-plugin-unused-imports": "^4.4.1", |
There was a problem hiding this comment.
why do we need eslint-plugin-unused-imports? As far as I know, TypeScript will flag unused imports just fine
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 1m 24s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
❌ Failed | 35s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-30 12:39:32 UTC
There was a problem hiding this comment.
🧹 Nitpick comments (1)
eslint.config.js (1)
10-12: Consider adding error context for config loading failure.The dynamic import of
oxlint.config.tswill throw if the file is missing or has syntax errors. While fail-fast behavior is reasonable here, the error message may not clearly indicate the source of the problem during ESLint initialization.This is optional—the current approach works and keeps the code concise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 10 - 12, Wrap the dynamic import of './oxlint.config.ts' performed via createJiti(import.meta.url) and jiti.import(...) in a try/catch so failures include clear context; catch the error around the jiti.import call that assigns to oxlintConfig, augment or rethrow a new Error that mentions it failed to load or parse the oxlint.config.ts (including the original error.message or error), and ensure the process exits or bubbles the error so ESLint initialization reports the config filename and underlying error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eslint.config.js`:
- Around line 10-12: Wrap the dynamic import of './oxlint.config.ts' performed
via createJiti(import.meta.url) and jiti.import(...) in a try/catch so failures
include clear context; catch the error around the jiti.import call that assigns
to oxlintConfig, augment or rethrow a new Error that mentions it failed to load
or parse the oxlint.config.ts (including the original error.message or error),
and ensure the process exits or bubbles the error so ESLint initialization
reports the config filename and underlying error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a14ceddf-3071-442b-b19c-bf6435ac366f
📒 Files selected for processing (2)
eslint.config.jspackages/vue-query/src/__tests__/useMutation.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue-query/src/tests/useMutation.test.ts

🎯 Changes
Add oxlint (Rust-based linter, ~50-100x faster than ESLint) as a first-pass linter that runs before ESLint in CI. Oxlint handles the bulk of correctness rules in ~250ms across 691 files, letting ESLint focus only on rules it exclusively covers (cspell, type-aware typescript-eslint rules, import ordering, react-hooks).
This pattern is already used by Preact, Sentry, Renovate, and Kibana. The implementation follows the same approach used in the AnalogJS repo (v3-alpha branch).
What changed:
oxlint.config.ts— typed config withdefineConfig, plugins: typescript, vitest, import, reacteslint.config.js— imports oxlint config via jiti, appendsbuildFromOxlintConfig()as last entry to auto-disable overlapping rules, setsreportUnusedDisableDirectives: 'off'package.json— addstest:oxlintscript, adds it totest:pr/test:ciNx targets andincludedScriptspackages/vue-query/src/__tests__/useMutation.test.ts— fix pre-existing unsafe optional chaining with a defensive checkpackages/react-query/src/__tests__/useQuery.promise.test.tsx— suppress false positive on vitest 3-argdescribe()with options objectoxlint,eslint-plugin-oxlint,jitiAlso incorporated from other open PRs:
@tanstack/eslint-configbump to 0.4.0,eslint-plugin-unused-imports, JSDoc type annotations +const configexport style on all 31 eslint configs,no-shadow/pnpm/enforce-catalog/pnpm/json-enforce-catalogrule overridesvitest/expect-expectis now handled by oxlint at warn level, so the eslint-disable comments that PR adds are no longer neededRules that stay ESLint-only (no oxlint equivalent)
@cspell/eslint-plugin(spell checking)@typescript-eslint/consistent-type-imports,array-type,naming-convention,method-signature-style@typescript-eslint/no-unnecessary-condition,no-unnecessary-type-assertion,no-for-in-array,require-await(type-aware)import/*rules (ordering, no-commonjs, no-duplicates)node/prefer-node-protocol@stylistic/spaced-commentsort-importseslint-plugin-react-hooksRelated issues & PRs
Superseded by this PR (can be closed):
vitest/expect-expect, making the eslint-disable comments unnecessaryRelated but independent (not affected by this PR):
This is PR 1 of 6 in the Oxc/Rolldown/tsdown modernization series.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Chores
test:oxlintscript to validation pipeline