Build: Migrate formatter from Prettier to oxfmt#34171
Closed
kasperpeulen wants to merge 3 commits into
Closed
Conversation
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:
📝 WalkthroughWalkthroughReplace Prettier with oxfmt across tooling and scripts; add oxfmt/prettier configs and editor setting changes; normalize quoting/whitespace across many files; update CI/workflow job names and YAML quoting; refactor codegen/scripts to use oxfmt; adjust various templates, tests, and package manifests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
|
![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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.serena/memories/project_overview.md (1)
14-24:⚠️ Potential issue | 🟠 MajorUpdate formatter entry to reflect oxfmt migration.
Line 21 is stale for this PR: formatter workflow moved to oxfmt, while Prettier remains for programmatic use. This should be reflected to avoid incorrect contributor guidance.
Suggested doc fix
-- **Formatting**: Prettier 3.7+ +- **Formatting**: oxfmt (repository formatting workflows), Prettier 3.x (programmatic formatting only)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.serena/memories/project_overview.md around lines 14 - 24, Update the "Formatting" entry in project_overview.md to reflect the migration to oxfmt: replace or augment the current "Formatting: Prettier 3.7+" line so it reads that the repository's formatter workflow uses oxfmt (with Prettier retained only for programmatic formatting), ensuring the doc accurately references "oxfmt" and mentions Prettier's limited programmatic role.CONTRIBUTING.old.md (1)
1-437:⚠️ Potential issue | 🟡 MinorRemove
CONTRIBUTING.old.mdfrom this PR.This file is superseded by the active
CONTRIBUTING.md(242 lines), as evidenced by the.oldsuffix and the significantly longer content in the deprecated version (437 lines). Reformatting deprecated files adds unnecessary repository churn. If this file is no longer needed, it should be deleted entirely rather than reformatted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.old.md` around lines 1 - 437, The file CONTRIBUTING.old.md is deprecated and should be removed from the PR; delete CONTRIBUTING.old.md from the branch/commit so only the active CONTRIBUTING.md remains (remove the file from git, commit the deletion, and push the branch), ensuring you don't modify or reformat it—remove the file entry for CONTRIBUTING.old.md from the changeset so the PR no longer contains that deprecated file.test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts (1)
35-41:⚠️ Potential issue | 🟠 MajorFail fast when file mutation is a no-op.
If
modify(content)returns unchanged text, the test continues with stale state and can silently assert the wrong behavior. Add a guard so this fails immediately.Proposed fix
const modifyFile = async (filePath: string, modify: (content: string) => string) => { const content = (await fs.readFile(filePath)).toString(); const modifiedContent = modify(content); + if (modifiedContent === content) { + throw new Error(`modifyFile did not change content for ${filePath}`); + } + if (!modifiedFiles.has(filePath)) { + modifiedFiles.set(filePath, content); + } await fs.writeFile(filePath, modifiedContent); - if (!modifiedFiles.has(filePath)) { - modifiedFiles.set(filePath, content); - } // the file change causes a HMR event, which causes a browser reload,and that can take a few seconds await new Promise((resolve) => setTimeout(resolve, 2000)); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts` around lines 35 - 41, The modifyFile helper currently writes back files even when modify(content) returns an identical string, leaving tests with stale state; update modifyFile (the async function modifyFile and its use of modifiedFiles) to detect a no-op by comparing content and modifiedContent and immediately throw an error or fail the test if they are equal, so tests stop fast instead of continuing with unchanged files; ensure you still record the original content in modifiedFiles only when a real change was made.code/lib/eslint-plugin/docs/rules/no-stories-of.md (1)
28-30:⚠️ Potential issue | 🟡 MinorFix the default-export syntax in both "correct" examples.
Lines 28 and 38 use
export default = {, which is invalid JavaScript/TypeScript syntax. The correct ES-module syntax isexport default {(without the=). Readers copy-pasting these examples will get broken code.✏️ Proposed fix
-export default = { +export default { component: Button }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/eslint-plugin/docs/rules/no-stories-of.md` around lines 28 - 30, Two "correct" examples use the invalid syntax `export default = {`; locate both occurrences of the string `export default = {` in the markdown examples and remove the extra `=` so they read `export default {` (i.e., replace `export default = {` with `export default {` in the "correct" example blocks) to restore valid ES-module default-export syntax.
🧹 Nitpick comments (5)
test-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.stories.ts (1)
96-96: Remove debug logging from story setup
console.log('hello')looks like leftover debug output. It adds noise to test/CI logs and should be removed before merge.Proposed cleanup
setup() { - console.log('hello'); return { args }; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.stories.ts` at line 96, Remove the leftover debug logging in the story setup by deleting the console.log('hello') call found in Button.stories.ts (inside the story definition / setup block for the Button stories); ensure no other stray console.log calls remain in the file so test/CI logs stay clean.test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts (1)
43-45: Replace hardcoded 2s sleep with deterministic HMR readiness checks.Lines 44 and 53 use fixed timeouts to wait for HMR stabilization. This pattern is prone to CI flakiness and unnecessary latency. Wait for concrete UI state changes or event signals instead.
Locations
44: await new Promise((resolve) => setTimeout(resolve, 2000)); 53: await new Promise((resolve) => setTimeout(resolve, 2000));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts` around lines 43 - 45, Replace the hardcoded "await new Promise((resolve) => setTimeout(resolve, 2000));" sleeps with deterministic HMR readiness checks: detect HMR completion by polling for a concrete UI state or event (e.g., wait for a specific DOM change/text/element that updates after HMR or assert the webpack HMR hash/window flag has changed via window.__webpack_hash or a custom window.__hmr_ready boolean), and swap both occurrences so the test proceeds only when that condition is met instead of waiting a fixed 2s.code/.storybook/bench/bundle-analyzer/index.js (1)
62-2059: Consider excluding bundled artifacts from broad formatter rewrites.This file appears to be generated/bundled code, and formatting churn here creates very high review noise. Consider excluding this path from formatter sweeps so behavioral diffs remain easier to audit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/.storybook/bench/bundle-analyzer/index.js` around lines 62 - 2059, This diff is a generated/bundled JS file (large blob defining symbols like et (metafile parser), Me (legend element), and DOM ids chartPanel/startPanel/resultsPanel) and should be excluded from broad formatter runs to avoid churn; update the project's formatter config (e.g., add a rule to .prettierignore / formatter ignore list or the formatter CI step) to ignore generated bundle output (the file that declares et/Me and the chartPanel handlers) and revert the formatting-only changes in this file so only behavioral changes remain in the PR..serena/memories/style_and_conventions.md (1)
11-30: Update the Prettier Configuration section to reflect oxfmt as the primary formatter.The repo has migrated to oxfmt as the primary formatter (root
package.jsonsets"format": "oxfmt"), but this memory file still documents Prettier as the canonical formatter. The.oxfmtrc.jsonconfiguration mirrors most of the documented Prettier settings (print width: 100, tab width: 2, single quotes, trailing commas: es5, arrow parens: always).Update this section to document oxfmt as the primary formatter and note that import sorting (previously handled by
@trivago/prettier-plugin-sort-importsin Prettier) is no longer configured at the formatter level.Suggested update direction
-## Prettier Configuration +## Formatter Configuration -- Print width: 100 -- Tab width: 2 -- Single quotes: yes -- Trailing commas: es5 -- Arrow parens: always -- Brace style: 1tbs (one true brace style) -- Import order (via `@trivago/prettier-plugin-sort-imports`): - 1. `node:` builtins - 2. `vitest`, `@testing-library` - 3. `react`, `react-dom` - 4. `storybook/internal` - 5. `@storybook/[non-addon]` - 6. `@storybook/addon-*` - 7. Third-party modules - 8. Relative imports (`./`, `../`) -- Import order separation: yes (blank lines between groups) -- Import specifiers sorted: yes +- Primary formatter: oxfmt (configured via `.oxfmtrc.json`) +- Print width: 100 +- Tab width: 2 +- Single quotes: yes +- Trailing commas: es5 +- Arrow parens: always +- End of line: lf +- Note: Import sorting is no longer enforced at the formatter level🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.serena/memories/style_and_conventions.md around lines 11 - 30, Update the "Prettier Configuration" section in .serena/memories/style_and_conventions.md to state that oxfmt is the primary formatter (referencing the repo root package.json "format": "oxfmt") and that the .oxfmtrc.json mirrors the previously listed formatting rules (print width: 100, tab width: 2, single quotes, trailing commas: es5, arrow parens: always); remove or replace wording that presents Prettier as canonical and explicitly note that import sorting is no longer configured at the formatter level (previously handled by `@trivago/prettier-plugin-sort-imports`) so import ordering should be handled by CI/linter or developer tooling instead.package.json (1)
51-51: Pinoxfmtto a fixed version for deterministic formatting.Using
"latest"in devDependencies can introduce unexpected formatting changes and flaky format checks across different environments and CI/CD runs. Currently resolves to 0.41.0 in yarn.lock—pin to this or the appropriate fixed version.♻️ Suggested change
- "oxfmt": "latest" + "oxfmt": "0.41.0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 51, The devDependency "oxfmt" is pinned to "latest" which causes non-deterministic formatting; update the package.json entry for "oxfmt" (replace the value "latest") to the resolved fixed version (e.g., "0.41.0") and then regenerate the lockfile (run your package manager's install command such as yarn install or npm install) so yarn.lock / package-lock.json is updated accordingly; ensure the package.json change references the exact version string (not a range like ^) to guarantee deterministic installs.
🤖 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/DISCUSSION_TEMPLATE/help.yml:
- Line 3: The markdown element in the form schema currently includes an invalid
property `id: intro`; remove the `id` key from that markdown element in
.github/DISCUSSION_TEMPLATE/help.yml so the element only contains `type:
markdown` and allowed properties per GitHub's form schema (ensuring the markdown
block no longer includes `id` and the file validates against the form schema).
In @.serena/memories/suggested_commands.md:
- Line 77: Replace the problematic recommendation of the literal command "yarn
start" in suggested_commands.md with an explicit, safe alternative: remove the
generic "yarn start" suggestion and instead instruct users to run the specific
project/dev script for the intended target (e.g., the react-vite default) or
provide the exact workspace/dev command to start only that service, and add a
short note warning that "yarn start" can launch long-running dev servers and
should be avoided for routine workflows.
In `@CHANGELOG.md`:
- Line 140: Update the changelog entry to use imperative tense to match nearby
entries: change "Addon-Vitest: Added timeout for fetching localhost 6006 during
global setup." to "Addon-Vitest: Add timeout for fetching localhost 6006 during
global setup." so the verb tense is consistent with other lines in CHANGELOG.md.
In `@code/package.json`:
- Around line 30-34: The lint script "lint:fmt" currently runs oxfmt in default
(write) mode and should be split into a write-mode script and a check-mode
script: add "lint:fmt" (or "format") that runs oxfmt in write mode and add
"lint:fmt:check" (or "format:check") that runs oxfmt with the no-write/check
flag (e.g., --check or --no-write) so CI and pre-commit can fail on violations;
update the lint-staged entry that references "lint:fmt" to use the new
"lint:fmt:check" instead so pre-commit runs the check-only script. Ensure you
update the package.json scripts block where "lint:fmt", "lint:other", "lint:js",
"lint:js:cmd", and "lint:package" are defined and the lint-staged configuration
that currently calls "lint:fmt".
In `@code/renderers/web-components/template/cli/js/Header.js`:
- Around line 29-43: The current falsy-user branch in the Header template
creates an html`` template with two Button(...) calls separated by literal
whitespace, producing an unwanted text node; replace that html`` return with an
array containing the two Button(...) results so there is no intervening text
node — locate the conditional in Header.js where user ? Button(...) : html`...`
and change the falsy branch to return [Button({ size: 'small', onClick: onLogin,
label: 'Log in' }), Button({ primary: true, size: 'small', onClick:
onCreateAccount, label: 'Sign up' })] (keep the same props and handlers:
onLogin, onCreateAccount, Button).
In `@CONTRIBUTING.md`:
- Line 143: The file contains a duplicate H3 heading "Running the local
development environment" which triggers MD024; remove or rename one of the
duplicated headings so each section has a unique title—locate the two headings
matching "Running the local development environment" and either delete the
redundant one or replace it with a more specific heading (e.g., "Running the
local dev environment: Docker" or "Running the local dev environment: From
source") and update any surrounding section text or links to match the new
heading.
- Around line 165-179: The example block uses a four-backtick fence to open a
shell code block but then contains an inner triple-backtick fence, breaking
Markdown rendering; change the outer opening fence to a standard triple-backtick
"```shell" and ensure the inner closing fence is "```" (remove the extra
backtick) so both the top and bottom fences match and the inner example block is
correctly fenced; update the fences around the two shell examples (the opening
"```shell" and the closing "```") in the CONTRIBUTING.md example to fix the
nesting.
In `@scripts/package.json`:
- Line 12: The "docs:oxfmt:check" npm script currently masks failures by
appending "|| echo ...", so when oxfmt fails CI still sees success; update the
"docs:oxfmt:check" script (the package.json entry named docs:oxfmt:check) to
remove the "|| echo ..." fallback and let oxfmt return a non-zero exit code on
failure (or replace it with a short fail-friendly wrapper that echoes the
message and exits non-zero) so formatting errors surface in CI.
In `@test-storybooks/external-docs/package.json`:
- Around line 22-23: The package.json has mismatched React majors—"react" is 16,
"react-dom" is 18 and "@types/react" is 17—so update the dependency trio to
compatible React 18 versions: set "react" to a React 18 release (e.g., ^18.2.0),
ensure "react-dom" remains ^18.2.0, and bump "@types/react" to the corresponding
18.x types (e.g., ^18.0.0) so all three (react, react-dom, `@types/react`) share
the same major version and satisfy peer dependencies.
In `@test-storybooks/external-docs/styles/globals.css`:
- Around line 8-15: The CSS font-family list contains unquoted multi-word font
family names causing a Stylelint `font-family-name-quotes` failure; update the
font-family declaration in globals.css to wrap each multi-word family name in
quotes (e.g., "Segoe UI", "Fira Sans", "Droid Sans", "Helvetica Neue") while
leaving single-word names (Roboto, Oxygen, Ubuntu, Cantarell) unquoted so the
rule passes.
In `@test-storybooks/external-docs/styles/Home.module.css`:
- Around line 64-72: The font-family declaration uses multi-word family names
without quotes (the font-family rule block), which breaks the
font-family-name-quotes lint; update the font-family list in Home.module.css
(the font-family rule) by wrapping all multi-word names such as Lucida Console,
Liberation Mono, DejaVu Sans Mono, Bitstream Vera Sans Mono, and Courier New in
quotes so they comply with the linter.
In `@test-storybooks/portable-stories-kitchen-sink/nextjs/styles/globals.css`:
- Line 5: The Stylelint `value-keyword-case` linter flags capitalized font
family names in the CSS variable --font-mono; fix it by normalizing the font
names to lowercase (change "Menlo" and "Monaco" to menlo and monaco) or
alternatively wrap them in quotes to preserve case; update the declaration that
defines --font-mono so the font-family list uses menlo and monaco (or "Menlo"
and "Monaco" quoted) alongside the existing ui-monospace and other entries.
In
`@test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts`:
- Around line 137-143: The click that expands the test story is not awaited,
causing flakiness; modify the conditional branch where testStoryElement is
obtained (via page.getByRole) so that when (await
testStoryElement.getAttribute('aria-expanded')) !== 'true' you call and await
testStoryElement.click() to ensure the expansion completes before proceeding;
keep the rest of the logic the same and do not remove the aria-expanded check.
In
`@test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.ts`:
- Line 33: The test currently calls
sbPage.panelContent().locator('[data-short-label="Unsaved
changes"]').isVisible() (also at the second occurrence) but ignores its boolean
result; change both calls so they perform explicit assertions instead of
dropping the return value — either use the framework's locator assertion (e.g.,
expect(locator).toBeVisible()) or await the boolean and assert it (e.g.,
expect(await locator.isVisible()).toBe(true)); update both uses of
sbPage.panelContent().locator('[data-short-label="Unsaved changes"]') (lines
referencing isVisible()) to use one of these explicit assertions so the test
fails when the footer is missing.
In
`@test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/save-from-controls.spec.ts`:
- Line 33: Replace the bare visibility checks that call isVisible() / isHidden()
on the locator with Playwright assertions: wrap
sbPage.panelContent().locator('[data-short-label="Unsaved changes"]') in
expect(...) and use toBeVisible() where isVisible() was used and toBeHidden()
where isHidden() was used (these occur in the test spec functions referencing
that locator at the three sites); update the three lines to use
expect(locator).toBeVisible() or expect(locator).toBeHidden() to ensure failures
are asserted.
In `@test-storybooks/portable-stories-kitchen-sink/svelte/stories/Button.cy.tsx`:
- Around line 11-12: The test mounts an undefined variable CSF3Primary causing a
ReferenceError; fix by using the composed story instance or defining
CSF3Primary: either change cy.mount(CSF3Primary) to cy.mount(Primary) (Primary
is created by composeStory(stories.CSF3Primary, stories.default)), or restore
the earlier destructuring that defines CSF3Primary so the original identifier is
available; ensure the mounted argument matches the identifier returned from
composeStory or the destructured export.
In `@test-storybooks/standalone-preview/package.json`:
- Around line 15-16: Package.json currently lists "react": "16.14.0" and
"react-dom": "^18.2.0", causing a major version mismatch; update the
dependencies so both use the same major version by either upgrading "react" to a
React 18 release that matches "react-dom" (e.g., set "react" to "^18.2.0") or
downgrading "react-dom" to the React 16 line (e.g., set "react-dom" to
"16.14.0"), then run your package manager (npm/yarn/pnpm) to reinstall and
verify the app starts without runtime errors.
---
Outside diff comments:
In @.serena/memories/project_overview.md:
- Around line 14-24: Update the "Formatting" entry in project_overview.md to
reflect the migration to oxfmt: replace or augment the current "Formatting:
Prettier 3.7+" line so it reads that the repository's formatter workflow uses
oxfmt (with Prettier retained only for programmatic formatting), ensuring the
doc accurately references "oxfmt" and mentions Prettier's limited programmatic
role.
In `@code/lib/eslint-plugin/docs/rules/no-stories-of.md`:
- Around line 28-30: Two "correct" examples use the invalid syntax `export
default = {`; locate both occurrences of the string `export default = {` in the
markdown examples and remove the extra `=` so they read `export default {`
(i.e., replace `export default = {` with `export default {` in the "correct"
example blocks) to restore valid ES-module default-export syntax.
In `@CONTRIBUTING.old.md`:
- Around line 1-437: The file CONTRIBUTING.old.md is deprecated and should be
removed from the PR; delete CONTRIBUTING.old.md from the branch/commit so only
the active CONTRIBUTING.md remains (remove the file from git, commit the
deletion, and push the branch), ensuring you don't modify or reformat it—remove
the file entry for CONTRIBUTING.old.md from the changeset so the PR no longer
contains that deprecated file.
In
`@test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts`:
- Around line 35-41: The modifyFile helper currently writes back files even when
modify(content) returns an identical string, leaving tests with stale state;
update modifyFile (the async function modifyFile and its use of modifiedFiles)
to detect a no-op by comparing content and modifiedContent and immediately throw
an error or fail the test if they are equal, so tests stop fast instead of
continuing with unchanged files; ensure you still record the original content in
modifiedFiles only when a real change was made.
---
Nitpick comments:
In @.serena/memories/style_and_conventions.md:
- Around line 11-30: Update the "Prettier Configuration" section in
.serena/memories/style_and_conventions.md to state that oxfmt is the primary
formatter (referencing the repo root package.json "format": "oxfmt") and that
the .oxfmtrc.json mirrors the previously listed formatting rules (print width:
100, tab width: 2, single quotes, trailing commas: es5, arrow parens: always);
remove or replace wording that presents Prettier as canonical and explicitly
note that import sorting is no longer configured at the formatter level
(previously handled by `@trivago/prettier-plugin-sort-imports`) so import ordering
should be handled by CI/linter or developer tooling instead.
In `@code/.storybook/bench/bundle-analyzer/index.js`:
- Around line 62-2059: This diff is a generated/bundled JS file (large blob
defining symbols like et (metafile parser), Me (legend element), and DOM ids
chartPanel/startPanel/resultsPanel) and should be excluded from broad formatter
runs to avoid churn; update the project's formatter config (e.g., add a rule to
.prettierignore / formatter ignore list or the formatter CI step) to ignore
generated bundle output (the file that declares et/Me and the chartPanel
handlers) and revert the formatting-only changes in this file so only behavioral
changes remain in the PR.
In `@package.json`:
- Line 51: The devDependency "oxfmt" is pinned to "latest" which causes
non-deterministic formatting; update the package.json entry for "oxfmt" (replace
the value "latest") to the resolved fixed version (e.g., "0.41.0") and then
regenerate the lockfile (run your package manager's install command such as yarn
install or npm install) so yarn.lock / package-lock.json is updated accordingly;
ensure the package.json change references the exact version string (not a range
like ^) to guarantee deterministic installs.
In
`@test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts`:
- Around line 43-45: Replace the hardcoded "await new Promise((resolve) =>
setTimeout(resolve, 2000));" sleeps with deterministic HMR readiness checks:
detect HMR completion by polling for a concrete UI state or event (e.g., wait
for a specific DOM change/text/element that updates after HMR or assert the
webpack HMR hash/window flag has changed via window.__webpack_hash or a custom
window.__hmr_ready boolean), and swap both occurrences so the test proceeds only
when that condition is met instead of waiting a fixed 2s.
In
`@test-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.stories.ts`:
- Line 96: Remove the leftover debug logging in the story setup by deleting the
console.log('hello') call found in Button.stories.ts (inside the story
definition / setup block for the Button stories); ensure no other stray
console.log calls remain in the file so test/CI logs stay clean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fcfa0e77-74b1-4585-905b-6edc4a96964c
⛔ Files ignored due to path filters (2)
code/lib/codemod/src/transforms/__tests__/__snapshots__/upgrade-deprecated-types.test.ts.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (263)
.circleci/config.yml.github/DISCUSSION_TEMPLATE/help.yml.github/ISSUE_TEMPLATE/tracking_issue.yml.github/PULL_REQUEST_TEMPLATE.md.github/workflows/code-simplifier.lock.yml.github/workflows/cron-weekly.yml.github/workflows/danger-js.yml.github/workflows/duplicate-code-detector.lock.yml.github/workflows/fork-checks.yml.github/workflows/handle-release-branches.yml.github/workflows/nx.yml.github/workflows/publish.yml.github/workflows/shared/mood.md.github/workflows/shared/reporting.md.github/workflows/stale.yml.github/workflows/triage.yml.nx/workflows/agents.yaml.oxfmtrc.json.prettierrc.serena/memories/project_overview.md.serena/memories/style_and_conventions.md.serena/memories/suggested_commands.md.serena/memories/task_completion_checklist.md.serena/project.yml.vscode/settings.json.yarnrc.ymlCHANGELOG.mdCHANGELOG.v6.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdCONTRIBUTING.old.mdCONTRIBUTING/RELEASING.mdMIGRATION.mdcode/.prettierignorecode/.storybook/bench/bundle-analyzer/index.jscode/addons/a11y/package.jsoncode/addons/docs/docs/frameworks/WEB_COMPONENTS.mdcode/addons/docs/package.jsoncode/addons/links/package.jsoncode/addons/onboarding/package.jsoncode/addons/pseudo-states/package.jsoncode/addons/themes/package.jsoncode/addons/vitest/package.jsoncode/builders/builder-vite/package.jsoncode/builders/builder-webpack5/package.jsoncode/core/package.jsoncode/core/src/common/utils/__tests-formatter__/withPrettierConfig/.prettierrccode/core/src/csf/story.tscode/core/src/manager-api/modules/stories.tscode/frameworks/angular/package.jsoncode/frameworks/angular/src/client/angular-beta/ComputesTemplateFromComponent.test.tscode/frameworks/angular/src/client/angular-beta/StorybookModule.test.tscode/frameworks/angular/src/client/csf-factories.test.tscode/frameworks/angular/src/client/decorateStory.test.tscode/frameworks/angular/template/cli/button.component.tscode/frameworks/angular/template/cli/header.component.tscode/frameworks/angular/template/cli/page.component.tscode/frameworks/angular/template/components/button.component.tscode/frameworks/angular/template/components/form.component.tscode/frameworks/angular/template/components/html.component.tscode/frameworks/angular/template/components/pre.component.tscode/frameworks/angular/template/stories/basics/angular-forms/customControlValueAccessor/custom-cva.component.tscode/frameworks/angular/template/stories/basics/component-with-complex-selectors/attribute-selector.component.tscode/frameworks/angular/template/stories/basics/component-with-complex-selectors/class-selector.component.tscode/frameworks/angular/template/stories/basics/component-with-complex-selectors/multiple-selector.component.tscode/frameworks/angular/template/stories/basics/component-with-inheritance/base-button.component.tscode/frameworks/angular/template/stories/basics/component-with-inheritance/icon-button.component.tscode/frameworks/angular/template/stories/basics/component-with-ng-content/ng-content-about-parent.stories.tscode/frameworks/angular/template/stories/basics/component-with-ng-content/ng-content-simple.stories.tscode/frameworks/angular/template/stories/basics/component-with-ng-on-destroy/component-with-on-destroy.stories.tscode/frameworks/angular/template/stories/basics/component-with-on-push/on-push-box.component.tscode/frameworks/angular/template/stories/basics/component-with-pipe/with-pipe.component.tscode/frameworks/angular/template/stories/basics/component-with-template/template.component.tscode/frameworks/angular/template/stories/basics/component-without-selector/without-selector-ng-component-outlet.stories.tscode/frameworks/angular/template/stories/basics/component-without-selector/without-selector.component.tscode/frameworks/angular/template/stories/basics/ng-module/angular-src/chip.component.tscode/frameworks/angular/template/stories/basics/ng-module/angular-src/chips-group.component.tscode/frameworks/angular/template/stories/button.component.tscode/frameworks/angular/template/stories/core/decorators/componentWrapperDecorator/child.component.tscode/frameworks/angular/template/stories/core/decorators/componentWrapperDecorator/parent.component.tscode/frameworks/angular/template/stories/core/moduleMetadata/angular-src/service.component.tscode/frameworks/angular/template/stories/core/moduleMetadata/angular-src/token.component.tscode/frameworks/angular/template/stories/core/parameters/bootstrap-options.stories.tscode/frameworks/angular/template/stories_angular-cli-default-ts/signal/button.component.tscode/frameworks/angular/template/stories_angular-cli-prerelease/signal/button.component.tscode/frameworks/ember/package.jsoncode/frameworks/html-vite/package.jsoncode/frameworks/nextjs-vite/package.jsoncode/frameworks/nextjs/package.jsoncode/frameworks/preact-vite/package.jsoncode/frameworks/react-native-web-vite/package.jsoncode/frameworks/react-vite/package.jsoncode/frameworks/react-webpack5/package.jsoncode/frameworks/server-webpack5/package.jsoncode/frameworks/svelte-vite/package.jsoncode/frameworks/sveltekit/package.jsoncode/frameworks/vue3-vite/package.jsoncode/frameworks/web-components-vite/package.jsoncode/frameworks/web-components-vite/template/cli/js/Header.jscode/frameworks/web-components-vite/template/cli/ts/Header.tscode/lib/cli-sb/package.jsoncode/lib/cli-storybook/package.jsoncode/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.tscode/lib/cli-storybook/src/codemod/helpers/story-to-csf-factory.test.tscode/lib/codemod/src/transforms/__tests__/csf-2-to-3.test.tscode/lib/create-storybook/package.jsoncode/lib/eslint-plugin/docs/rules/no-stories-of.mdcode/lib/eslint-plugin/package.jsoncode/lib/eslint-plugin/scripts/update-lib-configs.tscode/lib/eslint-plugin/scripts/update-lib-flat-configs.tscode/lib/eslint-plugin/scripts/update-lib-index.tscode/lib/eslint-plugin/src/index.tscode/package.jsoncode/prettier.config.mjscode/renderers/html/package.jsoncode/renderers/preact/package.jsoncode/renderers/server/package.jsoncode/renderers/svelte/package.jsoncode/renderers/vue3/package.jsoncode/renderers/web-components/package.jsoncode/renderers/web-components/src/csf-factories.test.tscode/renderers/web-components/src/docs/__testfixtures__/lit-element-demo-card/input.jscode/renderers/web-components/template/cli/js/Header.jscode/renderers/web-components/template/cli/ts/Header.tscode/renderers/web-components/template/stories/demo-wc-card/DemoWcCard.jsdependabot.ymldocs/.prettierignoredocs/.prettierrcdocs/_snippets/individual-snapshot-tests-portable-stories.mddocs/_snippets/login-form-with-play-function.mddocs/_snippets/mount-advanced.mddocs/_snippets/portable-stories-jest-multi-snapshot-test.mddocs/_snippets/portable-stories-jest-override-globals.mddocs/_snippets/portable-stories-jest-snapshot-test.mddocs/_snippets/portable-stories-playwright-ct-override-globals.mddocs/_snippets/portable-stories-vitest-multi-snapshot-test.mddocs/_snippets/portable-stories-vitest-override-globals.mddocs/_snippets/portable-stories-vitest-snapshot-test.mddocs/_snippets/snapshot-tests-portable-stories.mddocs/_snippets/storybook-addon-use-global.mddocs/_snippets/storybook-addons-preset-viteFinal.mddocs/_snippets/storybook-preview-use-global-type.mddocs/_snippets/storybook-preview-with-styled-components-decorator.mddocs/_snippets/test-runner-custom-page-viewport.mddocs/_snippets/vitest-plugin-vitest-config.mddocs/configure/integration/eslint-plugin.mdxdocs/versions/latest.jsondocs/versions/next.jsonpackage.jsonprettier.config.mjsscripts/.eslintrc.cjsscripts/build/utils/dts-process.tsscripts/package.jsonscripts/prettier.config.jstest-storybooks/ember-cli/.storybook/main.jstest-storybooks/ember-cli/app/index.htmltest-storybooks/ember-cli/app/templates/application.hbstest-storybooks/ember-cli/app/templates/components/named-block.hbstest-storybooks/ember-cli/app/templates/components/welcome-banner.hbstest-storybooks/ember-cli/app/templates/components/welcome-page.hbstest-storybooks/ember-cli/package.jsontest-storybooks/ember-cli/stories/welcome-banner.stories.jstest-storybooks/external-docs/.babelrctest-storybooks/external-docs/package.jsontest-storybooks/external-docs/styles/Home.module.csstest-storybooks/external-docs/styles/globals.csstest-storybooks/portable-stories-kitchen-sink/nextjs/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/nextjs/jest.config.jstest-storybooks/portable-stories-kitchen-sink/nextjs/jest.setup.tstest-storybooks/portable-stories-kitchen-sink/nextjs/package.jsontest-storybooks/portable-stories-kitchen-sink/nextjs/pages/_app.tsxtest-storybooks/portable-stories-kitchen-sink/nextjs/pages/_document.tsxtest-storybooks/portable-stories-kitchen-sink/nextjs/pages/api/hello.tstest-storybooks/portable-stories-kitchen-sink/nextjs/pages/index.tsxtest-storybooks/portable-stories-kitchen-sink/nextjs/stories/Image.stories.tsxtest-storybooks/portable-stories-kitchen-sink/nextjs/stories/NextHeader.tsxtest-storybooks/portable-stories-kitchen-sink/nextjs/styles/Home.module.csstest-storybooks/portable-stories-kitchen-sink/nextjs/styles/globals.csstest-storybooks/portable-stories-kitchen-sink/nextjs/typings.d.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/.eslintrc.cjstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/get-decorator-string.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/setup-file-dependency.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vite.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.tstest-storybooks/portable-stories-kitchen-sink/react/.eslintrc.cjstest-storybooks/portable-stories-kitchen-sink/react/.storybook/get-decorator-string.tstest-storybooks/portable-stories-kitchen-sink/react/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/react/.storybook/setup-file-dependency.tstest-storybooks/portable-stories-kitchen-sink/react/cypress.config.tstest-storybooks/portable-stories-kitchen-sink/react/cypress/support/commands.tstest-storybooks/portable-stories-kitchen-sink/react/cypress/support/component-index.htmltest-storybooks/portable-stories-kitchen-sink/react/cypress/support/component.tstest-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react/e2e-tests/save-from-controls.spec.tstest-storybooks/portable-stories-kitchen-sink/react/package.jsontest-storybooks/portable-stories-kitchen-sink/react/playwright-e2e.config.tstest-storybooks/portable-stories-kitchen-sink/react/playwright/index.htmltest-storybooks/portable-stories-kitchen-sink/react/playwright/index.tstest-storybooks/portable-stories-kitchen-sink/react/pre-e2e.jstest-storybooks/portable-stories-kitchen-sink/react/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react/stories/Button.playwright.tsxtest-storybooks/portable-stories-kitchen-sink/react/stories/Button.stories.playwright.tstest-storybooks/portable-stories-kitchen-sink/react/stories/Button.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react/stories/Button.tsxtest-storybooks/portable-stories-kitchen-sink/react/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react/stories/get-button-string.tstest-storybooks/portable-stories-kitchen-sink/react/vite.config.mtstest-storybooks/portable-stories-kitchen-sink/svelte/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/svelte/.storybook/preview.tstest-storybooks/portable-stories-kitchen-sink/svelte/cypress.config.tstest-storybooks/portable-stories-kitchen-sink/svelte/cypress/support/commands.tstest-storybooks/portable-stories-kitchen-sink/svelte/cypress/support/component-index.htmltest-storybooks/portable-stories-kitchen-sink/svelte/cypress/support/component.tstest-storybooks/portable-stories-kitchen-sink/svelte/package.jsontest-storybooks/portable-stories-kitchen-sink/svelte/playwright.config.tstest-storybooks/portable-stories-kitchen-sink/svelte/playwright/index.htmltest-storybooks/portable-stories-kitchen-sink/svelte/playwright/index.tstest-storybooks/portable-stories-kitchen-sink/svelte/stories/Button.cy.tsxtest-storybooks/portable-stories-kitchen-sink/svelte/stories/Button.playwright.tstest-storybooks/portable-stories-kitchen-sink/svelte/stories/Button.stories.playwright.tsxtest-storybooks/portable-stories-kitchen-sink/svelte/svelte.config.jstest-storybooks/portable-stories-kitchen-sink/svelte/vite.config.tstest-storybooks/portable-stories-kitchen-sink/vue3/.eslintrc.cjstest-storybooks/portable-stories-kitchen-sink/vue3/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/vue3/.storybook/preview.tstest-storybooks/portable-stories-kitchen-sink/vue3/cypress.config.tstest-storybooks/portable-stories-kitchen-sink/vue3/cypress/support/commands.tstest-storybooks/portable-stories-kitchen-sink/vue3/cypress/support/component-index.htmltest-storybooks/portable-stories-kitchen-sink/vue3/cypress/support/component.tstest-storybooks/portable-stories-kitchen-sink/vue3/package.jsontest-storybooks/portable-stories-kitchen-sink/vue3/playwright.config.tstest-storybooks/portable-stories-kitchen-sink/vue3/playwright/index.htmltest-storybooks/portable-stories-kitchen-sink/vue3/playwright/index.tstest-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.cy.tsxtest-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.playwright.tsxtest-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.stories.portable.tstest-storybooks/portable-stories-kitchen-sink/vue3/stories/Button.stories.tstest-storybooks/portable-stories-kitchen-sink/vue3/vite.config.tstest-storybooks/server-kitchen-sink/.storybook/main.tstest-storybooks/server-kitchen-sink/package.jsontest-storybooks/standalone-preview/package.jsontest-storybooks/standalone-preview/storybook.htmltest-storybooks/yarn-pnp/.storybook/main.tstest-storybooks/yarn-pnp/.storybook/preview.tstest-storybooks/yarn-pnp/eslint.config.jstest-storybooks/yarn-pnp/index.htmltest-storybooks/yarn-pnp/package.jsontest-storybooks/yarn-pnp/src/App.vuetest-storybooks/yarn-pnp/src/components/HelloWorld.vuetest-storybooks/yarn-pnp/src/main.tstest-storybooks/yarn-pnp/tsconfig.jsontest-storybooks/yarn-pnp/vite.config.ts
💤 Files with no reviewable changes (8)
- docs/.prettierrc
- CHANGELOG.v6.md
- code/prettier.config.mjs
- .github/PULL_REQUEST_TEMPLATE.md
- code/.prettierignore
- prettier.config.mjs
- docs/.prettierignore
- scripts/prettier.config.js
| ### When you're ready to post | ||
| Add labels to your discussion (e.g. React, Vue, Vite) to make it clearer for other users. | ||
| - type: markdown | ||
| id: intro |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub discussion category form schema: is idallowed on a body element wheretype: markdown?
💡 Result:
No. In GitHub’s form schema, id is not allowed when an element’s type is markdown (“id … except when type is set to markdown”). Use attributes.value only for markdown blocks. [1]
Discussion category forms use this same form schema, so the same restriction applies there as well. [2]
Sources: [1] [2]
🏁 Script executed:
# Check if the file exists and view the relevant section
if [ -f ".github/DISCUSSION_TEMPLATE/help.yml" ]; then
echo "=== File exists, showing lines 1-10 ==="
head -10 ".github/DISCUSSION_TEMPLATE/help.yml" | cat -n
else
echo "File not found at .github/DISCUSSION_TEMPLATE/help.yml"
fiRepository: storybookjs/storybook
Length of output: 428
Remove id from the markdown element to keep the form schema valid.
GitHub's form schema does not allow id for elements where type: markdown. This will cause template validation to fail.
Suggested fix
- type: markdown
- id: intro
attributes:
value: |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| id: intro | |
| - type: markdown | |
| attributes: | |
| value: | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/DISCUSSION_TEMPLATE/help.yml at line 3, The markdown element in the
form schema currently includes an invalid property `id: intro`; remove the `id`
key from that markdown element in .github/DISCUSSION_TEMPLATE/help.yml so the
element only contains `type: markdown` and allowed properties per GitHub's form
schema (ensuring the markdown block no longer includes `id` and the file
validates against the form schema).
|
|
||
| ```bash | ||
| # Start a sandbox with a specific template | ||
| yarn start # defaults to react-vite/default-ts |
Contributor
There was a problem hiding this comment.
Avoid recommending yarn start in this guide.
Line 77 suggests yarn start, but this command is discouraged for routine workflows in this repo.
✏️ Suggested doc update
-# Start a sandbox with a specific template
-yarn start # defaults to react-vite/default-ts
+# Start a sandbox with an explicit template
+yarn task --task dev --template react-vite/default-ts --start-from=installBased on learnings: Do not run yarn start as it usually starts unwanted long-running development servers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.serena/memories/suggested_commands.md at line 77, Replace the problematic
recommendation of the literal command "yarn start" in suggested_commands.md with
an explicit, safe alternative: remove the generic "yarn start" suggestion and
instead instruct users to run the specific project/dev script for the intended
target (e.g., the react-vite default) or provide the exact workspace/dev command
to start only that service, and add a short note warning that "yarn start" can
launch long-running dev servers and should be avoided for routine workflows.
| - Addon-Docs: Skip `!autodocs` stories when computing primary story - [#32712](https://github.com/storybookjs/storybook/pull/32712), thanks @ia319! | ||
| - Addon-Pseudo States: Fix stylesheet rewrite for `:not()` with parenthesis in inner selector - [#33491](https://github.com/storybookjs/storybook/pull/33491), thanks @ghengeveld! | ||
| - Addon-Vitest: Added timeout for fetching localhost 6006 during global setup. - [#33232](https://github.com/storybookjs/storybook/pull/33232), thanks @snippy4! | ||
| - Addon-Vitest: Added timeout for fetching localhost 6006 during global setup. - [#33232](https://github.com/storybookjs/storybook/pull/33232), thanks @snippy4! |
Contributor
There was a problem hiding this comment.
Use consistent verb tense in the changelog entry.
Line 140 uses “Added” while nearby entries mostly use imperative tense (“Add”, “Fix”, “Improve”). Consider normalizing for consistency.
Suggested wording
-- Addon-Vitest: Added timeout for fetching localhost 6006 during global setup. - [`#33232`](https://github.com/storybookjs/storybook/pull/33232), thanks `@snippy4`!
+- Addon-Vitest: Add timeout for fetching localhost 6006 during global setup. - [`#33232`](https://github.com/storybookjs/storybook/pull/33232), thanks `@snippy4`!📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - Addon-Vitest: Added timeout for fetching localhost 6006 during global setup. - [#33232](https://github.com/storybookjs/storybook/pull/33232), thanks @snippy4! | |
| - Addon-Vitest: Add timeout for fetching localhost 6006 during global setup. - [`#33232`](https://github.com/storybookjs/storybook/pull/33232), thanks `@snippy4`! |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 140, Update the changelog entry to use imperative tense
to match nearby entries: change "Addon-Vitest: Added timeout for fetching
localhost 6006 during global setup." to "Addon-Vitest: Add timeout for fetching
localhost 6006 during global setup." so the verb tense is consistent with other
lines in CHANGELOG.md.
Comment on lines
+30
to
34
| "lint:fmt": "oxfmt --no-error-on-unmatched-pattern", | ||
| "lint:js": "yarn lint:js:cmd . --quiet", | ||
| "lint:js:cmd": "cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives", | ||
| "lint:other": "yarn lint:prettier '**/*.{css,html,json,md,yml}'", | ||
| "lint:other": "yarn lint:fmt '**/*.{css,html,json,md,yml}'", | ||
| "lint:package": "yarn --cwd ../scripts lint:package", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
According to the official oxfmt CLI documentation, what is the default mode when running oxfmtwithout--checkor--write, and does it modify files in place?
💡 Result:
The official oxfmt CLI docs state that the default mode (when you run oxfmt without --check or --write) is --write: “Format and write files in place (default),” meaning it does modify files in place. [1]
Sources:
[1] (oxc.rs)
Citations:
🏁 Script executed:
# Check the current state of code/package.json around the relevant lines
head -60 code/package.json | tail -35Repository: storybookjs/storybook
Length of output: 1818
🏁 Script executed:
# Also check if there's a git diff to understand what changed
git diff code/package.json 2>/dev/null | head -100 || echo "No git diff available"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Search for oxfmt references in the codebase to understand if it's new or existing
rg "oxfmt" --type json --type yaml --type mdRepository: storybookjs/storybook
Length of output: 688
🏁 Script executed:
# Verify all references to lint:fmt to ensure complete picture
rg "lint:fmt" code/ --type json -A 2 -B 2Repository: storybookjs/storybook
Length of output: 940
🏁 Script executed:
# Check if there are any lint:fmt:check scripts already defined
rg "lint:fmt:check" --type jsonRepository: storybookjs/storybook
Length of output: 47
Add check-mode script to prevent write-mode mutations in CI and pre-commit linting
oxfmt defaults to --write mode, so lint:fmt mutates files in place. This causes lint:other (line 33) and the lint-staged pre-commit hook (line 54) to silently fix formatting instead of failing on violations, weakening CI signal. The root-level package.json already defines this pattern correctly with separate format and format:check scripts—apply the same approach here.
Suggested fix
- "lint:fmt": "oxfmt --no-error-on-unmatched-pattern",
+ "lint:fmt": "oxfmt --no-error-on-unmatched-pattern --write",
+ "lint:fmt:check": "oxfmt --no-error-on-unmatched-pattern --check",
"lint:js": "yarn lint:js:cmd . --quiet",
"lint:js:cmd": "cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives",
- "lint:other": "yarn lint:fmt '**/*.{css,html,json,md,yml}'",
+ "lint:other": "yarn lint:fmt:check '**/*.{css,html,json,md,yml}'",Also update lint-staged (line 54) to use lint:fmt:check instead of lint:fmt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/package.json` around lines 30 - 34, The lint script "lint:fmt" currently
runs oxfmt in default (write) mode and should be split into a write-mode script
and a check-mode script: add "lint:fmt" (or "format") that runs oxfmt in write
mode and add "lint:fmt:check" (or "format:check") that runs oxfmt with the
no-write/check flag (e.g., --check or --no-write) so CI and pre-commit can fail
on violations; update the lint-staged entry that references "lint:fmt" to use
the new "lint:fmt:check" instead so pre-commit runs the check-only script.
Ensure you update the package.json scripts block where "lint:fmt", "lint:other",
"lint:js", "lint:js:cmd", and "lint:package" are defined and the lint-staged
configuration that currently calls "lint:fmt".
Comment on lines
+29
to
+43
| ${ | ||
| user | ||
| ? Button({ size: 'small', onClick: onLogout, label: 'Log out' }) | ||
| : html`${Button({ | ||
| size: 'small', | ||
| onClick: onLogin, | ||
| label: 'Log in', | ||
| })} | ||
| ${Button({ | ||
| primary: true, | ||
| size: 'small', | ||
| onClick: onCreateAccount, | ||
| label: 'Sign up', | ||
| })}`} | ||
| })}` | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find code/renderers/web-components -name "Header.js" -type fRepository: storybookjs/storybook
Length of output: 120
🏁 Script executed:
git ls-files "code/renderers/web-components/template/cli/js/Header.js"Repository: storybookjs/storybook
Length of output: 120
🏁 Script executed:
cat -n code/renderers/web-components/template/cli/js/Header.jsRepository: storybookjs/storybook
Length of output: 1746
🏁 Script executed:
cat -n code/renderers/web-components/template/cli/js/Button.js | head -50Repository: storybookjs/storybook
Length of output: 788
🌐 Web query:
Lit html template whitespace between interpolations text nodes
💡 Result:
In Lit, anything that’s not inside ${...} is literal template text. That includes spaces/newlines you add for readability, and when the template is parsed as HTML those whitespace characters can become text nodes (and may render as a single collapsed space). (developer.mozilla.org)
Example (creates a whitespace text node between parts)
html`${a}
${b}`Ways to control it
-
No space at all: put expressions adjacent (and don’t leave whitespace between them)
html`${a}${b}`
-
Exactly one space: include it explicitly (recommended)
html`${a} ${b}`
-
Keep your template nicely formatted without rendering the gap (between elements):
html`<span>${a}</span><!-- --><span>${b}</span>`
(HTML comments prevent the “whitespace-between-tags” text node. Don’t put Lit expressions inside comments—Lit won’t update them.) (lit.dev)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Document_Object_Model/Whitespace
- 2: https://lit.dev/docs/v2/templates/expressions/
🌐 Web query:
Lit html template return array of TemplateResult
💡 Result:
In Lit, you can return an array/iterable of TemplateResult—Lit will render arrays/iterables the same way it renders them inside an expression. (lit.dev)
Example (returning TemplateResult[] from render())
import {LitElement, html} from 'lit';
class MyEl extends LitElement {
items = ['a', 'b', 'c'];
render() {
return this.items.map((x) => html`<div>${x}</div>`);
}
}
customElements.define('my-el', MyEl);Lit supports arrays or iterables of renderable values (including TemplateResult). (lit.dev)
Example (array inside a template)
render() {
return html`
<section>
${this.items.map((x) => html`<div>${x}</div>`)}
</section>
`;
}This pattern is explicitly documented: map() returns an array of TemplateResults and Lit renders it. (lit.dev)
Citations:
- 1: https://lit.dev/docs/components/rendering/?utm_source=openai
- 2: https://lit.dev/docs/components/rendering/?utm_source=openai
- 3: https://lit.dev/docs/v1/lit-html/writing-templates/?utm_source=openai
Avoid whitespace between the two auth buttons in the template.
Lines 32–42 contain literal whitespace (newline and indentation) between the two Button() calls inside html\``. In Lit templates, whitespace outside interpolations becomes a text node and will render as a space, changing the button layout. Since this is a formatting-only PR, this becomes a visual regression.
Fix by returning an array instead of an html\`` template:
Suggested fix
- : html`${Button({
- size: 'small',
- onClick: onLogin,
- label: 'Log in',
- })}
- ${Button({
- primary: true,
- size: 'small',
- onClick: onCreateAccount,
- label: 'Sign up',
- })}`
+ : [
+ Button({
+ size: 'small',
+ onClick: onLogin,
+ label: 'Log in',
+ }),
+ Button({
+ primary: true,
+ size: 'small',
+ onClick: onCreateAccount,
+ label: 'Sign up',
+ }),
+ ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/renderers/web-components/template/cli/js/Header.js` around lines 29 -
43, The current falsy-user branch in the Header template creates an html``
template with two Button(...) calls separated by literal whitespace, producing
an unwanted text node; replace that html`` return with an array containing the
two Button(...) results so there is no intervening text node — locate the
conditional in Header.js where user ? Button(...) : html`...` and change the
falsy branch to return [Button({ size: 'small', onClick: onLogin, label: 'Log
in' }), Button({ primary: true, size: 'small', onClick: onCreateAccount, label:
'Sign up' })] (keep the same props and handlers: onLogin, onCreateAccount,
Button).
Comment on lines
137
to
143
| const testStoryElement = await page.getByRole('button', { | ||
| name: 'Test', | ||
| exact: true, | ||
| }); | ||
| if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { | ||
| if ((await testStoryElement.getAttribute('aria-expanded')) !== 'true') { | ||
| testStoryElement.click(); | ||
| } |
Contributor
There was a problem hiding this comment.
Await the tree expansion click to avoid flake
Line 142 triggers testStoryElement.click() without await, so later steps can run before expansion completes.
Proposed fix
- if ((await testStoryElement.getAttribute('aria-expanded')) !== 'true') {
- testStoryElement.click();
- }
+ if ((await testStoryElement.getAttribute('aria-expanded')) !== 'true') {
+ await testStoryElement.click();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const testStoryElement = await page.getByRole('button', { | |
| name: 'Test', | |
| exact: true, | |
| }); | |
| if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { | |
| if ((await testStoryElement.getAttribute('aria-expanded')) !== 'true') { | |
| testStoryElement.click(); | |
| } | |
| const testStoryElement = await page.getByRole('button', { | |
| name: 'Test', | |
| exact: true, | |
| }); | |
| if ((await testStoryElement.getAttribute('aria-expanded')) !== 'true') { | |
| await testStoryElement.click(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts`
around lines 137 - 143, The click that expands the test story is not awaited,
causing flakiness; modify the conditional branch where testStoryElement is
obtained (via page.getByRole) so that when (await
testStoryElement.getAttribute('aria-expanded')) !== 'true' you call and await
testStoryElement.click() to ensure the expansion completes before proceeding;
keep the rest of the logic the same and do not remove the aria-expanded check.
| .panelContent() | ||
| .locator('[data-short-label="Unsaved changes"]') | ||
| .isVisible(); | ||
| await sbPage.panelContent().locator('[data-short-label="Unsaved changes"]').isVisible(); |
Contributor
There was a problem hiding this comment.
Use explicit assertions for visibility checks
Line 33 and Line 52 call isVisible() but ignore the boolean result, so these “Assert” steps won’t fail when the footer is missing.
Proposed fix
- await sbPage.panelContent().locator('[data-short-label="Unsaved changes"]').isVisible();
+ await expect(
+ sbPage.panelContent().locator('[data-short-label="Unsaved changes"]')
+ ).toBeVisible();
@@
- await sbPage.panelContent().locator('[data-short-label="Unsaved changes"]').isVisible();
+ await expect(
+ sbPage.panelContent().locator('[data-short-label="Unsaved changes"]')
+ ).toBeVisible();Also applies to: 52-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.ts`
at line 33, The test currently calls
sbPage.panelContent().locator('[data-short-label="Unsaved
changes"]').isVisible() (also at the second occurrence) but ignores its boolean
result; change both calls so they perform explicit assertions instead of
dropping the return value — either use the framework's locator assertion (e.g.,
expect(locator).toBeVisible()) or await the boolean and assert it (e.g.,
expect(await locator.isVisible()).toBe(true)); update both uses of
sbPage.panelContent().locator('[data-short-label="Unsaved changes"]') (lines
referencing isVisible()) to use one of these explicit assertions so the test
fails when the footer is missing.
| .panelContent() | ||
| .locator('[data-short-label="Unsaved changes"]') | ||
| .isVisible(); | ||
| await sbPage.panelContent().locator('[data-short-label="Unsaved changes"]').isVisible(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify non-assertive visibility checks in this spec
rg -nP --type=ts 'await\s+.+\.is(Visible|Hidden)\(\);' \
test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/save-from-controls.spec.ts -C2Repository: storybookjs/storybook
Length of output: 530
🏁 Script executed:
cat -n test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/save-from-controls.spec.tsRepository: storybookjs/storybook
Length of output: 3230
🏁 Script executed:
# Search for Playwright assertion patterns used elsewhere in e2e tests
fd --type f '\.(spec|test)\.ts$' test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/ | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && grep -n "expect.*toBeVisible\|expect.*toBeHidden" {}'Repository: storybookjs/storybook
Length of output: 2577
🏁 Script executed:
# Check general Playwright assertion patterns in the e2e-tests directory
rg -n 'expect\(' test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/ --type=ts -A1 | head -20Repository: storybookjs/storybook
Length of output: 2000
Replace bare visibility checks with Playwright assertions.
Lines 33, 44, and 52 use isVisible() and isHidden() without assertion wrappers, meaning the test can silently pass even when these conditions fail. Use expect(...).toBeVisible() and expect(...).toBeHidden() to properly assert visibility state, consistent with the assertions already used at lines 40 and 66.
💡 Suggested fix
- await sbPage.panelContent().locator('[data-short-label="Unsaved changes"]').isVisible();
+ await expect(
+ sbPage.panelContent().locator('[data-short-label="Unsaved changes"]')
+ ).toBeVisible();
@@
- await notification1.isHidden();
+ await expect(notification1).toBeHidden();
@@
- await sbPage.panelContent().locator('[data-short-label="Unsaved changes"]').isVisible();
+ await expect(
+ sbPage.panelContent().locator('[data-short-label="Unsaved changes"]')
+ ).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/save-from-controls.spec.ts`
at line 33, Replace the bare visibility checks that call isVisible() /
isHidden() on the locator with Playwright assertions: wrap
sbPage.panelContent().locator('[data-short-label="Unsaved changes"]') in
expect(...) and use toBeVisible() where isVisible() was used and toBeHidden()
where isHidden() was used (these occur in the test spec functions referencing
that locator at the three sites); update the three lines to use
expect(locator).toBeVisible() or expect(locator).toBeHidden() to ensure failures
are asserted.
Comment on lines
+11
to
+12
| const Primary = composeStory(stories.CSF3Primary, stories.default); | ||
| cy.mount(CSF3Primary) | ||
| cy.mount(CSF3Primary); |
Contributor
There was a problem hiding this comment.
Undefined variable CSF3Primary — the test will fail at runtime.
Line 11 defines Primary via composeStory(), but line 12 mounts CSF3Primary, which is not defined (the destructuring on line 5 is commented out). This will throw a ReferenceError.
🐛 Proposed fix
const Primary = composeStory(stories.CSF3Primary, stories.default);
- cy.mount(CSF3Primary);
+ cy.mount(Primary);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const Primary = composeStory(stories.CSF3Primary, stories.default); | |
| cy.mount(CSF3Primary) | |
| cy.mount(CSF3Primary); | |
| const Primary = composeStory(stories.CSF3Primary, stories.default); | |
| cy.mount(Primary); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test-storybooks/portable-stories-kitchen-sink/svelte/stories/Button.cy.tsx`
around lines 11 - 12, The test mounts an undefined variable CSF3Primary causing
a ReferenceError; fix by using the composed story instance or defining
CSF3Primary: either change cy.mount(CSF3Primary) to cy.mount(Primary) (Primary
is created by composeStory(stories.CSF3Primary, stories.default)), or restore
the earlier destructuring that defines CSF3Primary so the original identifier is
available; ensure the mounted argument matches the identifier returned from
composeStory or the destructured export.
Comment on lines
+15
to
+16
| "react": "16.14.0", | ||
| "react-dom": "^18.2.0", |
Contributor
There was a problem hiding this comment.
React and React DOM major version mismatch.
react is pinned to 16.14.0 while react-dom allows ^18.2.0. These packages must use the same major version to work correctly together. This mismatch can cause runtime errors.
Proposed fix
Either upgrade both to React 18:
- "react": "16.14.0",
- "react-dom": "^18.2.0",
+ "react": "^18.2.0",
+ "react-dom": "^18.2.0",Or downgrade react-dom to match React 16:
- "react": "16.14.0",
- "react-dom": "^18.2.0",
+ "react": "^16.14.0",
+ "react-dom": "^16.14.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "react": "16.14.0", | |
| "react-dom": "^18.2.0", | |
| "react": "^18.2.0", | |
| "react-dom": "^18.2.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test-storybooks/standalone-preview/package.json` around lines 15 - 16,
Package.json currently lists "react": "16.14.0" and "react-dom": "^18.2.0",
causing a major version mismatch; update the dependencies so both use the same
major version by either upgrading "react" to a React 18 release that matches
"react-dom" (e.g., set "react" to "^18.2.0") or downgrading "react-dom" to the
React 16 line (e.g., set "react-dom" to "16.14.0"), then run your package
manager (npm/yarn/pnpm) to reinstall and verify the app starts without runtime
errors.
kasperpeulen
changed the title
|
View your CI Pipeline Execution ↗ for commit 1ab360c ☁️ Nx Cloud last updated this comment at |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/create-nx-sandbox-projects.ts (1)
102-105: Pass the real output path toformat()for proper config override matching.oxfmt's
format()function uses its first argument for language inference and applying file-pattern-based config overrides. Passing the real destination path (full) instead of the constant'project.json'ensures overrides are correctly matched to the file being generated.Suggested change
- const { code: data } = await format( - 'project.json', - JSON.stringify(projectJson(key, project, tags, value)) - ); + const { code: data } = await format( + full, + JSON.stringify(projectJson(key, project, tags, value)) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/create-nx-sandbox-projects.ts` around lines 102 - 105, The call to format() uses a fixed filename string which prevents oxfmt from matching file-pattern config overrides; change the first argument of format(...) from the literal 'project.json' to the actual destination path variable (full) used for the generated file so format(full, JSON.stringify(projectJson(key, project, tags, value))) is called; update the call site where format is invoked and ensure the same 'full' path variable is passed to preserve language inference and proper config override matching.
🤖 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/core/scripts/generate-source-files.ts`:
- Around line 49-54: The fmtOptions object includes a Prettier-only key
embeddedLanguageFormatting which is not supported by oxfmt; remove the
embeddedLanguageFormatting property from the fmtOptions constant (located where
fmtOptions is declared) so the options only include supported keys (printWidth,
singleQuote, trailingComma), or if you intended an oxfmt-specific behavior,
replace it with the correct oxfmt option after verifying the oxfmt docs; update
any code that references embeddedLanguageFormatting to avoid referencing the
removed key.
In `@scripts/sandbox/utils/template.ts`:
- Around line 12-16: The format call currently passes a hardcoded
'template.html' which prevents oxfmt from selecting the correct parser; update
the code in the renderTemplate (or the function containing format, render,
template, templateData, templatePath) to derive the filename from templatePath
(e.g., using path.basename(templatePath)) and pass that filename to format
instead of the literal 'template.html', and add the necessary import for path if
missing so the proper parser is chosen based on the template's real extension.
---
Nitpick comments:
In `@scripts/create-nx-sandbox-projects.ts`:
- Around line 102-105: The call to format() uses a fixed filename string which
prevents oxfmt from matching file-pattern config overrides; change the first
argument of format(...) from the literal 'project.json' to the actual
destination path variable (full) used for the generated file so format(full,
JSON.stringify(projectJson(key, project, tags, value))) is called; update the
call site where format is invoked and ensure the same 'full' path variable is
passed to preserve language inference and proper config override matching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98622c81-9184-42b7-bc3d-b88302c0042f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
code/core/scripts/generate-source-files.tscode/lib/eslint-plugin/scripts/update-lib-configs.tscode/lib/eslint-plugin/scripts/update-lib-flat-configs.tscode/lib/eslint-plugin/scripts/update-lib-index.tscode/lib/eslint-plugin/scripts/utils/docs.tsscripts/create-nx-sandbox-projects.tsscripts/package.jsonscripts/sandbox/utils/template.tsscripts/utils/tools-esm.tsscripts/utils/tools.ts
💤 Files with no reviewable changes (1)
- scripts/utils/tools.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- code/lib/eslint-plugin/scripts/update-lib-configs.ts
- code/lib/eslint-plugin/scripts/update-lib-flat-configs.ts
- scripts/package.json
Comment on lines
+49
to
+54
| const fmtOptions = { | ||
| printWidth: 100, | ||
| singleQuote: true, | ||
| trailingComma: 'es5' as const, | ||
| embeddedLanguageFormatting: 'off' as const, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
oxfmt configuration options trailingComma printWidth singleQuote
💡 Result:
In oxfmt (Oxc formatter), these config options are:
trailingComma:"all" | "es5" | "none"(default:"all") — adds/removes trailing commas in multi-line comma-separated structures. [1]printWidth:integer(default:100) — the line length at which the printer wraps. [1]singleQuote:boolean(default:false) — use single quotes instead of double quotes (JSX has a separatejsxSingleQuote). [1]
Example .oxfmtrc.json:
{
"printWidth": 80,
"singleQuote": true,
"trailingComma": "es5"
}[1]
Sources:
[1] Oxc docs – “Configuration options for the Oxfmt” https://oxc.rs/docs/guide/usage/formatter/config-file-reference.html
Remove or verify embeddedLanguageFormatting: 'off' — it is not a supported oxfmt configuration option.
The options trailingComma: 'es5', printWidth: 100, and singleQuote: true are confirmed compatible with oxfmt. However, embeddedLanguageFormatting is a Prettier-specific option not documented in the oxfmt configuration reference and should be removed or verified to ensure it does not cause issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/scripts/generate-source-files.ts` around lines 49 - 54, The
fmtOptions object includes a Prettier-only key embeddedLanguageFormatting which
is not supported by oxfmt; remove the embeddedLanguageFormatting property from
the fmtOptions constant (located where fmtOptions is declared) so the options
only include supported keys (printWidth, singleQuote, trailingComma), or if you
intended an oxfmt-specific behavior, replace it with the correct oxfmt option
after verifying the oxfmt docs; update any code that references
embeddedLanguageFormatting to avoid referencing the removed key.
Comment on lines
+12
to
+16
| const { code } = await format('template.html', render(template, templateData), { | ||
| printWidth: 100, | ||
| singleQuote: true, | ||
| }); | ||
| return code; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
❓ Verification inconclusive
oxfmt npm package API format function signature
In the oxfmt Node.js API, format is an async function called like:
await format(filepath, input, options)So the signature is:
function format(
filepath: string,
input: string,
options?: FormatOptions
): Promise<FormatResult>FormatResult includes at least:
interface FormatResult {
code: string
errors: Array<OxcError>
}[1][2]
Script executed:
# Find template files in the repository to see what formats exist
find . -type f -name "*.ejs" | head -20Repository: storybookjs/storybook
Repository: storybookjs/storybook
Exit code: 0
stdout:
./code/builders/builder-webpack5/templates/preview.ejs
./code/core/assets/server/template.ejs
./scripts/sandbox/templates/item.ejs
./scripts/sandbox/templates/root.ejs
Script executed:
# Look for where renderTemplate is called to understand what templates are passed
rg "renderTemplate" -A 2 -B 2Repository: storybookjs/storybook
Repository: storybookjs/storybook
Exit code: 0
stdout:
scripts/sandbox/publish.ts-import { REPROS_DIRECTORY } from '../utils/constants';
scripts/sandbox/publish.ts-import { commitAllToGit } from './utils/git';
scripts/sandbox/publish.ts:import { getTemplatesData, renderTemplate } from './utils/template';
scripts/sandbox/publish.ts-
scripts/sandbox/publish.ts-export const logger = console;
--
scripts/sandbox/publish.ts- const templateData = { data: templatesData, version: branch === 'main' ? 'latest' : 'next' };
scripts/sandbox/publish.ts-
scripts/sandbox/publish.ts: const output = await renderTemplate(templatePath, templateData);
scripts/sandbox/publish.ts-
scripts/sandbox/publish.ts- await writeFile(join(tmpFolder, 'README.md'), output);
--
scripts/sandbox/utils/template.ts-import type { GeneratorConfig } from './types';
scripts/sandbox/utils/template.ts-
scripts/sandbox/utils/template.ts:export async function renderTemplate(templatePath: string, templateData: Record<string, any>) {
scripts/sandbox/utils/template.ts- const template = await readFile(templatePath, 'utf8');
scripts/sandbox/utils/template.ts-
--
scripts/sandbox/generate.ts-import type { OptionValues } from '../utils/options';
scripts/sandbox/generate.ts-import { createOptions } from '../utils/options';
scripts/sandbox/generate.ts:import { getStackblitzUrl, renderTemplate } from './utils/template';
scripts/sandbox/generate.ts-import { localizeYarnConfigFiles, setupYarn } from './utils/yarn';
scripts/sandbox/generate.ts-
--
scripts/sandbox/generate.ts-
scripts/sandbox/generate.ts- const stackblitzUrl = getStackblitzUrl(dirName);
scripts/sandbox/generate.ts: const contents = await renderTemplate(readmePath, {
scripts/sandbox/generate.ts- name,
scripts/sandbox/generate.ts- stackblitzUrl,
Script executed:
# Check the actual template.ts file to understand the full context
cat -n scripts/sandbox/utils/template.tsRepository: storybookjs/storybook
Repository: storybookjs/storybook
Exit code: 0
stdout:
1 import { readFile } from 'node:fs/promises';
2
3 import { render } from 'ejs';
4 import { format } from 'oxfmt';
5
6 import { allTemplates as sandboxTemplates } from '../../../code/lib/cli-storybook/src/sandbox-templates';
7 import type { GeneratorConfig } from './types';
8
9 export async function renderTemplate(templatePath: string, templateData: Record<string, any>) {
10 const template = await readFile(templatePath, 'utf8');
11
12 const { code } = await format('template.html', render(template, templateData), {
13 printWidth: 100,
14 singleQuote: true,
15 });
16 return code;
17 }
18
19 export const getStackblitzUrl = (path: string, branch = 'next') => {
20 return `https://stackblitz.com/github/storybookjs/sandboxes/tree/${branch}/${path}/after-storybook?preset=node`;
21 };
22
23 export async function getTemplatesData(branch: string) {
24 type TemplatesData = Record<
25 string,
26 Record<
27 string,
28 GeneratorConfig & {
29 stackblitzUrl: string;
30 }
31 >
32 >;
33
34 const templatesData = Object.keys(sandboxTemplates).reduce<TemplatesData>((acc, curr) => {
35 const [dirName, templateName] = curr.split('/');
36 const groupName =
37 dirName === 'cra' ? 'CRA' : dirName.slice(0, 1).toUpperCase() + dirName.slice(1);
38 const generatorData = sandboxTemplates[curr as keyof typeof sandboxTemplates];
39 acc[groupName] = acc[groupName] || {};
40 acc[groupName][templateName] = {
41 ...generatorData,
42 stackblitzUrl: getStackblitzUrl(curr, branch),
43 };
44 return acc;
45 }, {});
46 return templatesData;
47 }
Script executed:
# Check what the template files actually contain to understand their output format
cat -n scripts/sandbox/templates/item.ejsRepository: storybookjs/storybook
Repository: storybookjs/storybook
Exit code: 0
stdout:
1 <h1>
2 <%= name %>
3 </h1>
4
5 <p>This is project generated to serve as a reproduction starter for Storybook.</p>
6
7 <a href="<%= stackblitzUrl %>">
8 View it in Stackblitz
9 </a>
10
11 <h3>Testing instructions</h3>
12
13 <p>Install dependencies:</p>
14 <pre>
15 yarn
16 </pre>
17
18 <p>Run Storybook:</p>
19 <pre>
20 yarn storybook
21 </pre>
Script executed:
# Check the other template file
cat -n scripts/sandbox/templates/root.ejsRepository: storybookjs/storybook
Repository: storybookjs/storybook
Exit code: 0
stdout:
1 <h1>Storybook Reproduction Templates</h1>
2
3 <img alt="Storybook <%= version %> Badge" src="https://img.shields.io/npm/v/@storybook/react/<%= version %>" />
4
5 <p>The following repros have been generated with the `<%= version %>` version of Storybook.</p>
6
7 <% if (typeof data !== 'undefined' && Object.keys(data).length) { %>
8 <p>Preview any repro live on <a href="http://stackblitz.com/">StackBlitz:</a></p>
9 <% for (var groupName in data) { %>
10 <% if (data[groupName] !== undefined) { %>
11 <details>
12 <summary><b><%- (groupName) %></b></summary>
13 <ul>
14 <% for (var exampleName in data[groupName]) { %>
15 <% if (data[groupName][exampleName] !== undefined) { %>
16 <li>
17 <a href="<%=data[groupName][exampleName]['stackblitzUrl']%>">
18 <%=data[groupName][exampleName]['name']%>
19 </a>
20 </li>
21 <% } %>
22 <% } %>
23 </ul>
24 </details>
25 <% } %>
26 <% } %>
27 <% } %>
Script executed:
# Check where templates are read from in publish.ts to understand what's being formatted
sed -n '1,50p' scripts/sandbox/publish.tsRepository: storybookjs/storybook
Repository: storybookjs/storybook
Exit code: 0
stdout:
import { cp, mkdir, readdir, rm, writeFile } from 'node:fs/promises';
import { program } from 'commander';
// eslint-disable-next-line depend/ban-dependencies
import { execaCommand } from 'execa';
import { existsSync } from 'fs';
// eslint-disable-next-line depend/ban-dependencies
import { glob } from 'glob';
import { dirname, join, relative } from 'path';
import { temporaryDirectory } from '../../code/core/src/common/utils/cli';
import { REPROS_DIRECTORY } from '../utils/constants';
import { commitAllToGit } from './utils/git';
import { getTemplatesData, renderTemplate } from './utils/template';
export const logger = console;
const emptyDir = async (dir: string): Promise<void> => {
await mkdir(dir, { recursive: true });
const names = await readdir(dir);
await Promise.all(names.map((name) => rm(join(dir, name), { recursive: true, force: true })));
};
interface PublishOptions {
remote?: string;
push?: boolean;
branch?: string;
}
const publish = async (options: PublishOptions & { tmpFolder: string }) => {
const { branch: inputBranch, remote, push, tmpFolder } = options;
const scriptPath = __dirname;
const branch = inputBranch || 'next';
const templatesData = await getTemplatesData(branch === 'main' ? 'main' : 'next');
logger.log(`👯♂️ Cloning the repository ${remote} in branch ${branch}`);
await execaCommand(`git clone ${remote} .`, { cwd: tmpFolder, cleanup: true });
await execaCommand(`git checkout ${branch}`, { cwd: tmpFolder, cleanup: true });
// otherwise old files will stick around and result inconsistent states
logger.log(`🗑 Delete existing template dirs from clone`);
// empty all existing directories for sandboxes that have a successful after-storybook directory
await Promise.all(
// find all successfully generated after-storybook/README.md files
// eg. /home/repros/react-vite/default-ts/after-storybook/README.md
// README.md being the last file generated, thus representing a successful generation
(await glob(join(REPROS_DIRECTORY, '**', 'after-storybook/README.md'))).map((readmePath) => {
Hardcoded 'template.html' filename should use the actual template file path to ensure correct parser selection.
The filename passed to format() determines which parser oxfmt uses. While all current templates render HTML, renderTemplate is a generic function that accepts any template file path. Using a hardcoded filename makes the function fragile and prevents it from correctly handling templates that render to JavaScript, TypeScript, JSON, or other formats if used in the future.
Extract the filename from templatePath to let oxfmt select the appropriate parser:
Proposed fix
+import { basename } from 'node:path';
+
export async function renderTemplate(templatePath: string, templateData: Record<string, any>) {
const template = await readFile(templatePath, 'utf8');
+ const filename = basename(templatePath).replace(/\.ejs$/, '');
- const { code } = await format('template.html', render(template, templateData), {
+ const { code } = await format(filename, render(template, templateData), {
printWidth: 100,
singleQuote: true,
});
return code;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sandbox/utils/template.ts` around lines 12 - 16, The format call
currently passes a hardcoded 'template.html' which prevents oxfmt from selecting
the correct parser; update the code in the renderTemplate (or the function
containing format, render, template, templateData, templatePath) to derive the
filename from templatePath (e.g., using path.basename(templatePath)) and pass
that filename to format instead of the literal 'template.html', and add the
necessary import for path if missing so the proper parser is chosen based on the
template's real extension.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
code/package.json (1)
30-33:⚠️ Potential issue | 🟠 MajorMake formatter mode explicit and use check-mode for
lint:other.At Line 30,
lint:fmtrelies on oxfmt’s default mode. At Line 33,lint:otherthen inherits that behavior, which can turn linting into in-place mutation instead of validation. Keep autofix behavior for lint-staged (Line 54), but use a dedicated check script forlint:other.Suggested update
- "lint:fmt": "oxfmt --no-error-on-unmatched-pattern", + "lint:fmt": "oxfmt --no-error-on-unmatched-pattern --write", + "lint:fmt:check": "oxfmt --no-error-on-unmatched-pattern --check", @@ - "lint:other": "yarn lint:fmt '**/*.{css,html,json,md,yml}'", + "lint:other": "yarn lint:fmt:check '**/*.{css,html,json,md,yml}'",According to the official oxfmt CLI docs, what is the default behavior when no mode flag is provided (for example, without --check/--write), and does it modify files in place?Also applies to: 54-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/package.json` around lines 30 - 33, The package.json scripts are ambiguous about oxfmt’s mode: make formatter mode explicit by changing the existing "lint:fmt" to an explicit autofix invocation (e.g., add --write and keep --no-error-on-unmatched-pattern) and introduce a new check-only script (e.g., "lint:fmt:check" using --check --no-error-on-unmatched-pattern); then update "lint:other" to call the check-only script (e.g., "yarn lint:fmt:check '**/*.{css,html,json,md,yml}'") so linting is validation-only in CI, and ensure "lint-staged" continues to call the autofix "lint:fmt" for staged-file autoformatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@code/package.json`:
- Around line 30-33: The package.json scripts are ambiguous about oxfmt’s mode:
make formatter mode explicit by changing the existing "lint:fmt" to an explicit
autofix invocation (e.g., add --write and keep --no-error-on-unmatched-pattern)
and introduce a new check-only script (e.g., "lint:fmt:check" using --check
--no-error-on-unmatched-pattern); then update "lint:other" to call the
check-only script (e.g., "yarn lint:fmt:check '**/*.{css,html,json,md,yml}'") so
linting is validation-only in CI, and ensure "lint-staged" continues to call the
autofix "lint:fmt" for staged-file autoformatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3304c652-3587-472f-8c85-7fad4d6df8b6
📒 Files selected for processing (6)
.github/workflows/nx.yml.nx/workflows/distribution-config.yamlcode/package.jsonscripts/ci/common-jobs.tsscripts/ci/main.tsscripts/project.json
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/nx.yml
Replace Prettier CLI with oxfmt for the formatting workflow while keeping Prettier as a dependency for user-facing features (codemods, formatter.ts). Key changes: - Add .oxfmtrc.json at root with project formatting settings - Add .prettierrc at root for programmatic prettier use only - Remove prettier config files and .prettierignore files - Replace prettier with oxfmt in root devDependencies - Remove prettier plugins and eslint-plugin-prettier from scripts - Replace internal prettier API usage with oxfmt API in build scripts - Rename pretty-docs CI task to format (runs oxfmt --check) - Update ESLint config, lint-staged, CI workflows, and VS Code settings - Reformat codebase with oxfmt
kasperpeulen
force-pushed
the
kasper/migrate-prettier-to-oxfmt
branch
from
4e29c56 to
ed62128
Compare
…rmat options, scope NX cache inputs
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.
Closes #
What I did
Replace Prettier CLI with oxfmt (Rust-based, ~30x faster) for the formatting workflow, moving the config to the repository root.
Key changes:
.oxfmtrc.jsonat root with project formatting settings.prettierrcat root for programmatic prettier use only (codemods, story generation)eslint-plugin-prettier, keepeslint-config-prettierfor rule disablingprettier.config.mjs→ delegates toscripts/prettier.config.js.oxfmtrc.jsonat rootcode/.prettierignore+docs/.prettierignoreignorePatternsin.oxfmtrc.json+.gitignoreyarn lint:prettier/prettier --writeyarn format/oxfmtprettier --checkin CIyarn format:check/oxfmt --checkChecklist 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 format:checkpasses on all 4555 filesyarn nx run-many -t compilesucceedsDocumentation
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
Bug Fixes
Chores
Documentation
Tests