Build: Migrate ESLint to oxlint for 50-100x faster linting#34170
Closed
kasperpeulen wants to merge 6 commits into
Closed
Build: Migrate ESLint to oxlint for 50-100x faster linting#34170kasperpeulen wants to merge 6 commits into
kasperpeulen wants to merge 6 commits into
Conversation
Replace ESLint with oxlint for ~50-100x faster linting (2935 files in ~1.4s). Move lint configuration from code/ and scripts/ to root level. - Add .oxlintrc.json at root with rules matching the old ESLint config - Add oxlint and lint-staged to root package.json devDependencies - Move lint scripts to root: `yarn lint`, `yarn lint:fix`, `yarn lint:prettier` - Move lint-staged config and pre-commit hook to root level - Remove ESLint config files (.eslintrc.js, .eslintrc.cjs, .eslintrc.json, .eslintignore) - Remove ESLint and all ESLint plugin dependencies from scripts/package.json - Remove ESLint dependency from code/package.json - Update fork-checks CI workflow - Update AGENTS.md with new lint commands - Merge depend/ban-dependencies rules into no-restricted-imports - Keep eslint-plugin-storybook package (user-facing, not for internal linting)
Contributor
|
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:
📝 WalkthroughWalkthroughReplaces many per-package ESLint configs/ignore files with a new root OxLint config, adds root lint/prettier scripts and lint-staged, removes nested lint configs/ignore files, simplifies CI and husky lint invocations, and removes many inline ESLint suppression comments; two small destructuring tweaks remain. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.oxlintrc.json (1)
22-22: Re-enableno-consolefor normal code paths.Line 22 turns off
no-consoleglobally, which removes lint enforcement for the logger policy. Consider enabling it globally and keeping relaxed behavior only in test/template overrides.Suggested tightening
- "no-console": "off", + "no-console": "warn",As per coding guidelines: "Use
storybook/internal/node-loggerfor server-side logging instead of rawconsole.*calls", "Usestorybook/internal/client-loggerfor client-side logging instead of rawconsole.*calls", and "Avoidconsole.log,console.warn, andconsole.errorin normal code paths unless the file is isolated enough that importing the logger is not reasonable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.oxlintrc.json at line 22, The global lint rule "no-console" in .oxlintrc.json is currently disabled; change its value from "off" to "error" (or "warn") to re-enable prohibition of console.* in normal code paths and add/retain specific overrides for test/template files where console usage is allowed; ensure the rule key "no-console" is set at the root config and that any existing "overrides" array contains entries (e.g., matching test/**/*.js or templates/**) that explicitly set "no-console": "off" so only those paths remain relaxed while all other code must use the prescribed logger modules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 239: Update AGENTS.md to restore and align the per-file linting guidance
with the project's required command: replace or augment the current single-line
instruction referencing `yarn lint:fix` by adding the per-file JS/TS lint
command `yarn --cwd code lint:js:cmd <file-relative-to-code-folder> --fix` (you
may keep `yarn lint:fix` as a global option) so the document explicitly
instructs developers to run `yarn --cwd code lint:js:cmd
<file-relative-to-code-folder> --fix` before submitting changes; ensure the file
mentions both commands and clearly labels the per-file command as the required
workflow.
---
Nitpick comments:
In @.oxlintrc.json:
- Line 22: The global lint rule "no-console" in .oxlintrc.json is currently
disabled; change its value from "off" to "error" (or "warn") to re-enable
prohibition of console.* in normal code paths and add/retain specific overrides
for test/template files where console usage is allowed; ensure the rule key
"no-console" is set at the root config and that any existing "overrides" array
contains entries (e.g., matching test/**/*.js or templates/**) that explicitly
set "no-console": "off" so only those paths remain relaxed while all other code
must use the prescribed logger modules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0762d574-abd6-4125-adc6-8aff1fcabd9c
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
.github/workflows/fork-checks.yml.husky/pre-commit.oxlintrc.jsonAGENTS.mdcode/.eslintignorecode/.eslintrc.jscode/core/.eslintrc.jsoncode/core/src/components/components/Card/Card.tsxcode/frameworks/nextjs-vite/.eslintrc.jsoncode/frameworks/nextjs/.eslintrc.jsoncode/lib/cli-storybook/.eslintrc.cjscode/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.tscode/lib/create-storybook/.eslintrc.cjscode/package.jsonpackage.jsonscripts/.eslintignorescripts/.eslintrc.cjsscripts/package.json
💤 Files with no reviewable changes (11)
- code/.eslintrc.js
- code/lib/create-storybook/.eslintrc.cjs
- scripts/package.json
- scripts/.eslintrc.cjs
- scripts/.eslintignore
- code/frameworks/nextjs-vite/.eslintrc.json
- code/lib/cli-storybook/.eslintrc.cjs
- .husky/pre-commit
- code/.eslintignore
- code/core/.eslintrc.json
- code/frameworks/nextjs/.eslintrc.json
|
|
||
| 1. Format with `yarn prettier --write <file>` | ||
| 2. Lint with `yarn --cwd code lint:js:cmd <file-relative-to-code-folder> --fix` or `cd code && yarn lint:js:cmd <file-relative-to-code-folder>` | ||
| 2. Lint with `yarn lint:fix` (runs oxlint with auto-fix on code/ and scripts/) |
Contributor
There was a problem hiding this comment.
Keep AGENTS lint guidance aligned with the required per-file JS/TS lint workflow.
Line 239 now documents only yarn lint:fix, which diverges from the required per-file command and can make single-file linting guidance inconsistent.
As per coding guidelines: "Lint JavaScript/TypeScript files with yarn --cwd code lint:js:cmd <file-relative-to-code-folder> --fix before submitting changes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 239, Update AGENTS.md to restore and align the per-file
linting guidance with the project's required command: replace or augment the
current single-line instruction referencing `yarn lint:fix` by adding the
per-file JS/TS lint command `yarn --cwd code lint:js:cmd
<file-relative-to-code-folder> --fix` (you may keep `yarn lint:fix` as a global
option) so the document explicitly instructs developers to run `yarn --cwd code
lint:js:cmd <file-relative-to-code-folder> --fix` before submitting changes;
ensure the file mentions both commands and clearly labels the per-file command
as the required workflow.
kasperpeulen
changed the title
|
View your CI Pipeline Execution ↗ for commit ae93a73
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 0c0ab4e ☁️ Nx Cloud last updated this comment at |
- Add allowTypeImports to no-restricted-imports for react-aria-components and es-toolkit (matching old ESLint behavior) - Add typescript/no-wrapper-object-types and no-unsafe-function-type rules (were "warn" in old config, missing from migration) - Restore EJS linting in root lint-staged config
Use oxlint's jsPlugins feature to load ESLint plugins that were lost in migration: - eslint-plugin-storybook: all recommended rules for stories - eslint-plugin-playwright: e2e test best practices (19 spec files) - local-rules: 3 custom rules (no-uncategorized-errors, storybook-monorepo-imports, no-duplicated-error-codes) This recovers all previously "lost" plugins without needing ESLint installed.
Clean up disable comments for rules that no longer exist: - 86 depend/ban-dependencies comments (plugin removed, replaced by no-restricted-imports) - 4 compat/compat comments (plugin removed) - 1 eslint-comments/no-unlimited-disable comment (plugin removed)
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.oxlintrc.json (1)
75-125: Consider hardeninglodashandlodash-esrestrictions to block subpath imports.Lines 100-105 restrict only exact roots (
lodash,lodash-es). Subpath imports likelodash-es/addandlodash-es/sumcurrently bypass these restrictions. Addingpatternsfield closes this gap.Note: Test files (
code/core/template/stories/test/NodeModuleMocking.stories.js) intentionally demonstrate mocking oflodash-es/addandlodash-es/sumfor documentation purposes. If patterns are added, these test examples may require exclusion or the overrides section may need updating.Suggested change
"no-restricted-imports": [ "error", { "paths": [ { "name": "lodash", "message": "lodash is banned. Use es-toolkit or native alternatives." }, { "name": "lodash-es", "message": "lodash-es is banned. Use es-toolkit or native alternatives." } - ] + ], + "patterns": ["lodash/*", "lodash-es/*"] } ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.oxlintrc.json around lines 75 - 125, The no-restricted-imports rule currently bans only the exact module roots "lodash" and "lodash-es" but allows subpath imports like "lodash-es/add"; update the "no-restricted-imports" configuration for the "lodash" and "lodash-es" entries to include a "patterns" field that blocks subpath imports (e.g., "lodash/*" and "lodash-es/*") so submodule imports are prohibited as well, and ensure you update or add an override to exempt the documented test file (code/core/template/stories/test/NodeModuleMocking.stories.js) if those examples must remain permitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.oxlintrc.json:
- Around line 75-125: The no-restricted-imports rule currently bans only the
exact module roots "lodash" and "lodash-es" but allows subpath imports like
"lodash-es/add"; update the "no-restricted-imports" configuration for the
"lodash" and "lodash-es" entries to include a "patterns" field that blocks
subpath imports (e.g., "lodash/*" and "lodash-es/*") so submodule imports are
prohibited as well, and ensure you update or add an override to exempt the
documented test file
(code/core/template/stories/test/NodeModuleMocking.stories.js) if those examples
must remain permitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72e032ef-7b63-4a29-ba53-a976d62b4a64
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
.oxlintrc.jsonpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
- CRITICAL: Fix scripts/ci/common-jobs.ts lint job referencing deleted scripts - Fix react-hooks/ rule prefix to react/ (oxlint uses react/ for all react rules) - Add eslint-plugin-storybook and sort-package-json to root devDependencies - Add storybook/use-storybook-expect: off override for core stories - Add storybook/no-renderer-packages: off override for renderer stories - Add *.compat.* files to local-rules exclusion - Remove orphaned eslint-enable comment in preview.tsx
Remove 20 unnecessary "off" rules that aren't default-on, 4 unnecessary overrides (angular no-useless-constructor, test no-empty-function, stories no-console, .d.ts no-var), 2 redundant "warn" rules that match defaults (no-wrapper-object-types, jsx-a11y/no-autofocus), the env block, duplicate ignorePattern, and the lint:prettier:fix script. 262 lines → 183 lines, same 115 rules, same 0 errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Replace ESLint with oxlint for ~50-100x faster linting. Lints 2935 files in ~1.5 seconds (ESLint took 30s+). Moves all lint configuration from
code/andscripts/to a single.oxlintrc.jsonat root.Performance
How plugins are handled
oxlint has two plugin mechanisms:
typescript,react,import,jsx-a11y, and others are built into oxlintjsPlugins(alpha, ESLint v9+ compatible) — loads standard ESLint plugins as JavaScript, runs them alongside native rulesWe use both:
typescriptreact/react-hooksimportjsx-a11yeslint-plugin-storybookeslint-plugin-playwrightlocal-rules(custom)Dependency ban approach
The old
depend/ban-dependenciesrule was investigated but not re-added via jsPlugins because:eslint-disablecomments just to suppress the preset — pure noiseno-restricted-importswithallowTypeImportssupportno-restricted-importsactually provides better coverage for our custom ban list: it includes ban messages explaining what to use instead, andallowTypeImports: trueforreact-aria-componentsandes-toolkit(matching the old@typescript-eslint/no-restricted-importsconfig)Deliberate rule decisions
react-hooks/exhaustive-deps: off — oxlint's auto-fix for this rule is dangerous: it silently removes dependencies from hook arrays (e.g.[store]→[]), which can introduce subtle bugs. Disabled to prevent accidental breakage viaoxlint --fix.consistent-type-imports: off for.vue/.svelte— framework SFC files define types in<script>blocks where the rule produces false positivesno-restricted-imports: off for**/template/**— template files intentionally import banned packages for testing (e.g.NodeModuleMocking.stories.jsimportslodash-es)Warning count
The lint output shows ~3500 warnings and 0 errors. This is intentional — the warnings match the old ESLint severity levels:
typescript/no-explicit-any: ~2600 warnings (waswarnin ESLint too)typescript/no-unused-vars: ~300 warnings (waswarnin ESLint too)local-rules/no-uncategorized-errors: ~255 warnings (waswarnin ESLint too)warn-level rulesThese were never blocking in the old ESLint setup either. The lint command exits 0 (success) as long as there are no errors.
What changed
Added:
.oxlintrc.jsonat root — single config with native plugins + jsPluginsoxlint,lint-staged,eslint-plugin-playwrightin rootpackage.jsonyarn lint,yarn lint:fix,yarn lint:prettierRemoved:
code/.eslintrc.js,code/.eslintignorescripts/.eslintrc.cjs,scripts/.eslintignore.eslintrc.jsonfilesscripts/package.jsoneslint,cross-env,lint-stagedfromcode/package.jsoneslint-disablecomments (86depend/ban-dependencies, 4compat/compat, 1eslint-comments)Changed:
cd code && yarn lint-staged+cd scripts && yarn lint-staged) to 1 run at root (yarn lint-staged)cd code && yarn lint:prettier --check .→yarn lint:prettier(now checks both code/ and scripts/)code/package.jsonci-tests script:yarn lint→yarn --cwd .. lint(delegates to root)Kept:
code/lib/eslint-plugin/(eslint-plugin-storybook) — user-facing npm package, untouched.eslintrc.jsonfiles — these ship to userseslint-disablecomments — oxlint respects them for compatibilityRemaining true losses (no oxlint equivalent)
eslint-plugin-compateslint-comments/no-unused-disable@typescript-eslint/dot-notationobj.propoverobj['prop']; TypeScript catches the important casesreact/destructuring-assignmentconst { x } = propsChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
yarn installyarn lint— should complete in ~1-2 seconds with 0 errors (~3500 warnings expected)yarn lint:fix— should auto-fix safe issues.tsfile and commit — pre-commit hook should run oxlint via lint-stagedDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Chores