Cleanup: Unify oxlint at root and add more file to format#34245
Merged
Conversation
huang-julien
added
ci:normal
cleanup
|
View your CI Pipeline Execution ↗ for commit c9ad52a
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
huang-julien
changed the title
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:
📝 WalkthroughWalkthroughRepository-wide toolchain migration: Prettier replaced by oxfmt/oxc; root formatter config and VS Code defaults added; per-package formatter cleaned/removed; lint-staged consolidated at root; Husky pre-commit simplified to a single yarn lint-staged; package scripts and CI updated to use oxfmt and workspace-aware lint commands. (44 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
scripts/ci/common-jobs.ts (1)
74-78:prettyDocsis no longer docs-scoped.Lines 74-78 now install from the workspace root and run the root
yarn fmt:check, so this lane can fail on unrelated repo files instead of justdocs/_snippets. If the intent is still a dedicated docs check, keep this job pointed at thescriptspackage formatter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ci/common-jobs.ts` around lines 74 - 78, The prettyDocs job currently runs npm.install('.') and executes the root 'yarn fmt:check', which causes formatting failures across the whole repo; change the job to target the docs/scripts workspace instead (e.g., replace npm.install('.') with npm.install('packages/scripts') or install the workspace containing the docs) and/or run the package-scoped formatter command instead of the root formatter (replace the 'yarn fmt:check' run command with the scripts package's formatter such as a workspace or package-specific script) so prettyDocs only checks docs/_snippets as intended; update references in the prettyDocs job where npm.install('.') and the run.command 'yarn fmt:check' appear.package.json (1)
44-62: The lint-staged scope is intentionally narrower than CI formatter coverage.Lines 45–55 only run
oxfmt --checkforcode/**/*files, while the rootfmt:checkscript covers the entire repository (except ignored paths). This design means root-level files likepackage.jsonandscripts/**/*are formatted on CI (npm run fmt:check) but not validated at the pre-commit hook stage. If stricter pre-commit validation is desired, consider addingoxfmt --checktasks for non-code files to the lint-staged config. However,.vscodeis intentionally excluded by.oxfmtrc.jsonignore patterns and does not need formatter coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 44 - 62, The lint-staged config in package.json currently runs "oxfmt --check" only for the "code/**/*.{js,jsx,mjs,ts,tsx}" and "code/**/*.{html,json}" globs, leaving root-level and scripts files un-checked pre-commit; update the "lint-staged" entries to include "oxfmt --check" for the non-code globs (e.g., add "oxfmt --check" to the "scripts/**/*.{html,js,json,jsx,mjs,ts,tsx}" and "**/package.json" tasks or create a new glob covering root-level files) so that the pre-commit hook mirrors the repository-wide formatting enforced by the "fmt:check" script; keep existing exclusions handled by .oxfmtrc.json (such as .vscode)..oxfmtrc.json (1)
38-51: Dead configuration: MD/MDX override will never apply.MD and MDX files are listed in
ignorePatterns(lines 38-39), so the override block for*.mdand*.mdx(lines 42-51) is unreachable—ignored files won't be processed by the formatter at all.If the intent is to exclude MD/MDX for now (as stated in the PR objectives), consider removing the unused override block to avoid confusion. If you plan to re-enable MD/MDX formatting later, you could add a comment explaining why the override is kept.
🧹 Suggested cleanup: remove dead override
"*.md", "*.mdx" ], - "overrides": [ - { - "files": [ - "*.md", - "*.mdx" - ], - "options": { - "importOrderSeparation": false, - "importOrderSortSpecifiers": false - } - }, + "overrides": [ { "files": [ "*.component.html"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.oxfmtrc.json around lines 38 - 51, The overrides block for "*.md" and "*.mdx" is dead because those patterns are already present in ignorePatterns, so remove the unused overrides entry (the object containing "files": ["*.md","*.mdx"] and its "options") to avoid confusion; alternatively, if you intend to keep it for future re-enabling, add a clear inline comment above that overrides object explaining why MD/MDX are currently ignored and when to restore the override, referencing the ignorePatterns and overrides keys so reviewers can find the related configuration quickly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/builders/builder-vite/input/iframe.html`:
- Around line 84-88: The injected fallback HTML assigned to
document.getElementById('storybook-root').innerHTML contains a mismatched tag:
the list opens with <ul> twice; update the second opening <ul> at the end of the
template string to a closing </ul> so the markup produced by
message.replaceAll(...) and docs.map(...) results in a valid <ul>...</ul> list.
In `@scripts/project.json`:
- Around line 7-8: The pretty-docs target's caching is too narrow: it declares
inputs like docs/**/* but its command runs "yarn fmt:check" with cwd
"{workspaceRoot}" (which runs the root formatter over the whole repo), causing
stale cache hits; fix by either changing the command to use the docs-scoped
formatter (use the fmt:check script defined in scripts/package.json instead of
the root formatter) so the command only checks docs, or widen the target inputs
to include all files checked by the root formatter (e.g.,
"{workspaceRoot}/**/*") so the cache reflects the full command scope; update the
pretty-docs target's "command" and/or its inputs accordingly.
---
Nitpick comments:
In @.oxfmtrc.json:
- Around line 38-51: The overrides block for "*.md" and "*.mdx" is dead because
those patterns are already present in ignorePatterns, so remove the unused
overrides entry (the object containing "files": ["*.md","*.mdx"] and its
"options") to avoid confusion; alternatively, if you intend to keep it for
future re-enabling, add a clear inline comment above that overrides object
explaining why MD/MDX are currently ignored and when to restore the override,
referencing the ignorePatterns and overrides keys so reviewers can find the
related configuration quickly.
In `@package.json`:
- Around line 44-62: The lint-staged config in package.json currently runs
"oxfmt --check" only for the "code/**/*.{js,jsx,mjs,ts,tsx}" and
"code/**/*.{html,json}" globs, leaving root-level and scripts files un-checked
pre-commit; update the "lint-staged" entries to include "oxfmt --check" for the
non-code globs (e.g., add "oxfmt --check" to the
"scripts/**/*.{html,js,json,jsx,mjs,ts,tsx}" and "**/package.json" tasks or
create a new glob covering root-level files) so that the pre-commit hook mirrors
the repository-wide formatting enforced by the "fmt:check" script; keep existing
exclusions handled by .oxfmtrc.json (such as .vscode).
In `@scripts/ci/common-jobs.ts`:
- Around line 74-78: The prettyDocs job currently runs npm.install('.') and
executes the root 'yarn fmt:check', which causes formatting failures across the
whole repo; change the job to target the docs/scripts workspace instead (e.g.,
replace npm.install('.') with npm.install('packages/scripts') or install the
workspace containing the docs) and/or run the package-scoped formatter command
instead of the root formatter (replace the 'yarn fmt:check' run command with the
scripts package's formatter such as a workspace or package-specific script) so
prettyDocs only checks docs/_snippets as intended; update references in the
prettyDocs job where npm.install('.') and the run.command 'yarn fmt:check'
appear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f790951e-159c-4906-96f9-1c9ce25a7547
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
.husky/pre-commit.oxfmtrc.json.vscode/extensions.json.vscode/settings.jsoncode/.oxfmtrc.jsoncode/builders/builder-vite/input/iframe.htmlcode/core/src/manager/components/sidebar/Menu.tsxcode/package.jsondocs/.oxfmtrc.jsonpackage.jsonscripts/ci/common-jobs.tsscripts/package.jsonscripts/project.json
💤 Files with no reviewable changes (3)
- code/.oxfmtrc.json
- .husky/pre-commit
- docs/.oxfmtrc.json
AriPerkkio
reviewed
Mar 23, 2026
Contributor
valentinpalkovic
added
the
patch:yes
Merged
19 tasks
valentinpalkovic
pushed a commit
that referenced
this pull request
Mar 31, 2026
Cleanup: Unify oxlint at root and add more file to format (cherry picked from commit b31240c)
Closed
19 tasks
github-actions
Bot
added
the
patch:done
This was referenced Apr 16, 2026
8 tasks
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
6 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
.*dirstodo in follow-up PR:
Checklist 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!
Documentation
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