Addon-Vitest: Extra changes for Vitest 4 #32600
Conversation
📝 WalkthroughWalkthroughThe pull request adds comprehensive Vitest 3.x and 4.x version compatibility to the vitest addon by introducing version-aware branching, dynamic context imports, and new internal export paths. This prevents import-time failures and handles breaking changes between Vitest versions. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as vitest-plugin/index.ts
participant CheckHelper as checkIsVitest4orHigher()
participant PkgJson as vitest/package.json
participant Vite as Vite Config
Plugin->>CheckHelper: read Vitest version
CheckHelper->>PkgJson: dynamically import + semver check
alt Vitest >= 4
CheckHelper-->>Plugin: true
Plugin->>Vite: alias '@storybook/addon-vitest/internal/vitest-context'<br/>→ vitest-context.js (modern)
else Vitest < 4
CheckHelper-->>Plugin: false
Plugin->>Vite: alias '@storybook/addon-vitest/internal/vitest-context'<br/>→ vitest-context-legacy.js
end
Note over Plugin,Vite: vitest-context.ts handles<br/>dynamic resolution at runtime
sequenceDiagram
participant Module as test-utils.ts
participant Runtime as Runtime (test execution)
participant ViContext as vitest-context.ts
participant Vitest as @vitest/browser or vitest/browser
Module->>Runtime: testStory() called
Runtime->>Module: need initialGlobals
Module->>ViContext: dynamically import vitest-context
ViContext->>Vitest: check Vitest version
alt Vitest >= 4
Vitest-->>ViContext: vitest/browser context
else Vitest < 4
Vitest-->>ViContext: @vitest/browser/context
end
ViContext-->>Module: getInitialGlobals function
Module->>Module: extract initialGlobals via server.commands
Module-->>Runtime: pass to composeStory with fallback {}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
View your CI Pipeline Execution ↗ for commit 10e8b3e
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
code/addons/vitest/src/vitest-plugin/index.ts (1)
281-304: Replace@ts-expect-errorwith explicitinstancessupport for Vitest 4.
Vitest 4 removes the automatic fallback frombrowser.nameand makesbrowser.instancesrequired. Rather than suppressing the type error, either cast the config to Vitest’s expected type or includeinstances: [{ browser: name }]to satisfy the types.
code/core/src/test/preview.ts (1)
21-24: Switch default mock cleanup toresetAllMocks().The default branch currently calls
restoreAllMocks(), which only restores spies and leaves automock state intact. To fully reset both spies and automocks between stories, replacerestoreAllMocks()withresetAllMocks(), or useclearAllMocks()if only call-history reset is needed.code/addons/vitest/templates/vitest.config.4.template.ts (1)
10-12: Minor: simplify dirname resolution (optional).You can drop the __dirname guard and always use fileURLToPath(import.meta.url); Node ESM is standard for config files now. If you keep dual CJS/ESM support, current code is fine.
code/addons/vitest/src/postinstall.ts (2)
541-551: CLI hint: always suggest --project=storybook for consistency.New installs also create a projects config; recommending
vitest --project=storybookavoids accidentally running other projects in mixed setups.- const runCommand = rootConfig ? `npx vitest --project=storybook` : `npx vitest`; + const runCommand = `npx vitest --project=storybook`;
373-384: Type shim placement OK; consider idempotent write (optional).You already check existence, but writing at repo root can surprise monorepos. Consider writing next to the config file or guarding with a header comment to detect prior content.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
code/addons/vitest/package.json(2 hunks)code/addons/vitest/src/node/vitest-manager.ts(2 hunks)code/addons/vitest/src/postinstall.ts(5 hunks)code/addons/vitest/src/vitest-plugin/index.ts(2 hunks)code/addons/vitest/src/vitest-plugin/test-utils.ts(1 hunks)code/addons/vitest/src/vitest-plugin/viewports.ts(1 hunks)code/addons/vitest/templates/vitest.config.3.2.template.ts(1 hunks)code/addons/vitest/templates/vitest.config.4.template.ts(1 hunks)code/addons/vitest/templates/vitest.config.template.ts(1 hunks)code/addons/vitest/templates/vitest.workspace.template.ts(1 hunks)code/core/src/actions/loaders.ts(1 hunks)code/core/src/test/preview.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
code/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/postinstall.tscode/core/src/test/preview.tscode/core/src/actions/loaders.tscode/addons/vitest/templates/vitest.config.3.2.template.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/templates/vitest.config.template.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/addons/vitest/templates/vitest.config.4.template.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode
Files:
code/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/postinstall.tscode/core/src/test/preview.tscode/core/src/actions/loaders.tscode/addons/vitest/templates/vitest.config.3.2.template.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/templates/vitest.config.template.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/addons/vitest/templates/vitest.config.4.template.ts
🧠 Learnings (9)
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/postinstall.ts
📚 Learning: 2025-09-25T09:21:27.284Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
Applied to files:
code/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/templates/vitest.config.3.2.template.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Applied to files:
code/core/src/test/preview.tscode/core/src/actions/loaders.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without vi.mocked()
Applied to files:
code/core/src/actions/loaders.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the spy: true option
Applied to files:
code/core/src/actions/loaders.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Applied to files:
code/core/src/actions/loaders.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/core/src/actions/loaders.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Applied to files:
code/core/src/actions/loaders.ts
📚 Learning: 2025-09-25T09:21:27.284Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to test-storybooks/** : Maintain test configurations and assets under test-storybooks/ for Storybook testing
Applied to files:
code/addons/vitest/templates/vitest.config.4.template.ts
🧬 Code graph analysis (2)
code/addons/vitest/src/postinstall.ts (1)
code/addons/vitest/src/updateVitestFile.ts (1)
loadTemplate(9-16)
code/addons/vitest/templates/vitest.config.4.template.ts (3)
scripts/ecosystem-ci/before-test.js (1)
__dirname(15-15)code/lib/cli-storybook/test/helpers.cjs (1)
path(2-2)code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-450)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (13)
code/addons/vitest/templates/vitest.config.template.ts (1)
24-29: LGTM: Explicit browser configuration replaces placeholder.The replacement of
BROWSER_CONFIGwith an explicit browser configuration object is clear and appropriate for Vitest 4 support. The Playwright provider with Chromium instance is a sensible default.code/addons/vitest/src/vitest-plugin/viewports.ts (1)
5-5: LGTM: Removed unused import.The
ViewportStylesimport has been removed while retainingViewportMap. This appears to be appropriate cleanup asViewportStylesis not referenced in the file.code/addons/vitest/src/vitest-plugin/test-utils.ts (1)
3-3: LGTM: Removed unused import.The
toTestIdfunction has been removed from the import statement. This is appropriate cleanup as the function is not used in the file.code/addons/vitest/package.json (2)
107-110: LGTM: Peer dependency ranges support both Vitest 3 and 4.The broadened peer dependency ranges (
^3.0.0 || ^4.0.0) provide excellent backward compatibility, allowing users to adopt Vitest 4 at their own pace while maintaining support for Vitest 3.
85-86: Verify Vitest beta stability before production release.Vitest 4.0 remains in pre-release (latest: v4.0.0-beta.13, published Sep 24, 2025) with no stable 4.0.0 release date announced. Confirm:
- Is this addon intended for beta/canary distribution only?
- Should production releases wait for the stable 4.0.0?
- Has the beta been fully tested with the Storybook integration?
code/addons/vitest/templates/vitest.workspace.template.ts (1)
23-28: LGTM: Explicit browser configuration replaces placeholder.The replacement of
BROWSER_CONFIGwith an explicit browser configuration is consistent with the changes invitest.config.template.ts. The Playwright provider with Chromium is appropriate for Vitest browser testing.code/core/src/actions/loaders.ts (1)
21-25: LGTM! Clear migration note.The TODO comment appropriately flags the Vitest 3-specific check for future removal and provides a helpful link to the migration guide.
code/addons/vitest/src/vitest-plugin/index.ts (1)
269-279: LGTM! Clear Vitest 3 compatibility handling.The conditional inlining of
@storybook/addon-vitestis correctly guarded and includes a clear TODO for removal once Vitest 3 support is dropped.code/addons/vitest/src/node/vitest-manager.ts (2)
167-170: LGTM! Unified module invalidation approach.The refactored
updateLastChangedmethod now iterates over all Vitest projects to invalidate modules, replacing the previous per-project approach. This is cleaner and aligns with Vitest 4's project handling.
502-504: Ensure TestProject‐only signature – confirm that bothglobTestSpecifications()’sworkspaceSpec.projectand entries inthis.vitest.projectsare actuallyTestProject(notWorkspaceProject) under both Vitest 3 and 4.code/addons/vitest/templates/vitest.config.3.2.template.ts (1)
24-29: Approve explicit browser configuration
Thebrowser.instancesoption is supported since Vitest v3, so this setup is valid for Vitest 3.2.code/addons/vitest/templates/vitest.config.4.template.ts (1)
14-37: Template shape matches Vitest 4 browser config (provider + instances).Config uses projects, extends root, and sets Playwright with instances — consistent with Vitest 4 docs. Looks good.
code/addons/vitest/src/postinstall.ts (1)
435-440: Remove unnecessary v4‐template preference
The v3.2 template already includes thebrowser.instancesfield, so switching to a v4 template solely for that field isn’t needed.Likely an incorrect or invalid review comment.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 6 | 6 | 0 |
| Self size | 509 KB | 578 KB | 🚨 +69 KB 🚨 |
| Dependency size | 1.53 MB | 1.53 MB | 🎉 -251 B 🎉 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/vitest/package.json (1)
80-105: Align Vitest peerDependencies with beta & remove duplicate ts-dedent
- Update all Vitest peer ranges from
^4.0.0to^4.0.0-beta.16so they match your devDependencies.- Remove the redundant
ts-dedententry (keep it only underdevDependencies).
🧹 Nitpick comments (2)
code/addons/vitest/package.json (2)
74-105: Remove duplicate dependency: ts-dedent is listed twice.It’s in both dependencies (Line 78) and devDependencies (Line 102). Keep one (likely runtime dependency), remove the other.
- "ts-dedent": "^2.2.0",
107-112: Add Playwright as an optional peer and pin its version in templates
Declare “playwright” (or “playwright-core”) as an optional peer dependency in code/addons/vitest/package.json (e.g. ^1.52.0) or document it in your docs/postinstall, and update code/addons/vitest/templates to include the pinned Playwright version per repo policy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
code/addons/vitest/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/addons/vitest/package.json (1)
117-119: Optional peer meta for @vitest/browser-playwright: LGTM.Prevents noisy peer warnings for users not using browser mode.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/addons/vitest/package.json (2)
117-119: Optional peer for @vitest/browser-playwright is correct — add a migration note.Good call marking it optional. Please document that Vitest 4 browser mode requires installing @vitest/browser-playwright (and any Playwright prerequisites), while Vitest 3 users can keep @vitest/browser.
Happy to draft a short “Vitest 3 -> 4” migration note for the addon README.
107-112: Simplify 4.x prerelease-inclusive ranges
Replace each^4.0.0 || ^4.0.0-betain code/addons/vitest/package.json (lines 107–112) with>=4.0.0-0 <5.0.0to cover all 4.x prereleases and stable releases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/vitest/package.json(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/addons/vitest/package.json (1)
85-86: Vitest 4 beta versions verified on npm
All 4.0.0-beta.16 packages exist. Note that^4.0.0-beta.16will not resolve to the stable 4.0.0—plan an explicit bump to GA when it’s released.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/addons/vitest/package.json (1)
100-105: Remove duplicate dependencyts-dedentfrom devDependencies
ts-dedentexists in both dependencies (Line 78) and devDependencies (Line 102). Keep it only once (dependencies).Apply this diff:
"tinyglobby": "^0.2.10", "tree-kill": "^1.2.2", - "ts-dedent": "^2.2.0", "typescript": "^5.8.3", "vitest": "^4.0.0-beta.16"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
code/addons/vitest/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/addons/vitest/package.json (4)
85-86: Dev deps update to Vitest 4 path looks goodSwitch to
@vitest/browser-playwrightand@vitest/runnerv4 beta aligns with the new provider.
107-112: Confirm support expectations for@vitest/browserin v4If v4 usage is exclusively via
@vitest/browser-playwright, consider narrowing@vitest/browserto^3.0.0to avoid conflicting installs with two browser providers. If you truly support@vitest/browser@4, please confirm the runtime selection logic.Option if not supported in v4:
- "@vitest/browser": "^3.0.0 || ^4.0.0 || ^4.0.0-beta", + "@vitest/browser": "^3.0.0",
117-119: Optional peer meta for@vitest/browser-playwrightlooks rightMarking it optional avoids noisy warnings while postinstall handles guidance.
107-109: Postinstall and templates already enforce one-of peer dependency The postinstall script auto-installs the correct provider (@vitest/browseron v3,@vitest/browser-playwrighton v4), and all templates default to the appropriate provider.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/addons/vitest/src/node/vitest-manager.ts (1)
167-182: LGTM! Cross-version compatibility handled well.The iteration over
projectswith conditional checks forbrowser,vite, andserverfields correctly handles both Vitest 2 and 3+ structures. The@ts-expect-errordirective appropriately suppresses TypeScript errors for the version-specificserverfield.Minor consistency note: Line 171 uses explicit
(mod: any)typing while line 175 infers the type. Consider adding a brief comment on line 171 explaining whyanyis necessary (e.g., TypeScript cannot properly typeserver.moduleGraphdue to its version-specific nature):if (server) { const serverMods = server.moduleGraph.getModulesByFile(filepath); - serverMods?.forEach((mod: any) => server.moduleGraph.invalidateModule(mod)); + // Type is `any` because TypeScript cannot infer the exact type from version-specific `server.moduleGraph` + serverMods?.forEach((mod: any) => server.moduleGraph.invalidateModule(mod)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/vitest/src/node/vitest-manager.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
code/addons/vitest/src/node/vitest-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode
Files:
code/addons/vitest/src/node/vitest-manager.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/addons/vitest/src/node/vitest-manager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/addons/vitest/src/node/vitest-manager.ts (1)
513-515: LGTM! Signature correctly narrowed.The method signature change from
TestProject | WorkspaceProjecttoTestProjectaligns with the Vitest 3+ API structure. All call sites (lines 347, 358, 374) remain type-safe with this change.
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (16)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/vitest.setup.ts (1)
1-8: Consider whether the console.log is needed.The setup correctly wires Storybook annotations with Vitest. The
console.log(getString())on Line 8 may be intentional for verifying that setup file dependencies are loaded correctly, or it could be a debug artifact. If it's for verification purposes, consider adding a comment explaining its purpose.If the console.log is for debugging only, apply this diff:
setProjectAnnotations([addonA11yAnnotations, projectAnnotations]); - -console.log(getString());If it's intentional for verification, add a comment:
setProjectAnnotations([addonA11yAnnotations, projectAnnotations]); +// Log to verify setup file dependencies are loaded correctly console.log(getString());test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview.tsx (1)
4-4: Consider removing or explaining the console log.This debug statement will execute every time the preview configuration loads. If this is intentional for testing purposes, consider adding a comment explaining why. Otherwise, remove it.
-console.log('preview file is called!');test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/main.ts (2)
15-20: Consider documenting the red border CSS.The injected CSS adds a red border to the body element, which appears to be a testing artifact. Add a comment explaining its purpose to avoid confusion.
+ // Red border for visual testing verification previewHead: (head = "") => `${head} <style> body { border: 1px solid red; } </style>`,
22-37: Simplify redundant optimizeDeps spread.Line 27 spreads
config.optimizeDeps?.includeinto a new array without modification, which is redundant. You can simplify this.viteFinal: (config) => { return { ...config, - optimizeDeps: { - ...config.optimizeDeps, - include: [...(config.optimizeDeps?.include || [])], - }, resolve: { ...config.resolve, alias: { ...config.resolve?.alias, "test-alias": join(import.meta.dirname, "aliased.ts"), }, }, }; },test-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.node.json (1)
1-11: Tighten tsconfig.node.json for config-only use
- Add top-level "$schema": "https://json.schemastore.org/tsconfig"
- Set compilerOptions.noEmit to true
- Add compilerOptions.types = ["node"]
- Optionally add compilerOptions.esModuleInterop = true
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.json (2)
23-24: Split browser vs Node test configs to avoid type pollution.Including
e2e-testshere (with DOM libs) can blur Node vs browser types. Prefer: keep this tsconfig for stories/browser; move e2e to tsconfig.node.json with Node/Playwright/Vitest types.Proposed change:
- "include": ["stories", "e2e-tests"], + "include": ["stories"],And ensure
tsconfig.node.jsoncontains:{ "compilerOptions": { "composite": true, "module": "ESNext", "moduleResolution": "bundler", "strict": true, "noEmit": true, "types": ["node", "playwright", "vitest/globals"] }, "include": ["e2e-tests", "vitest.config.*", "vite.config.*", "playwright.config.*"] }Please confirm
tsconfig.node.jsonalready sets these types; if not, we can amend it. Based on learnings.
12-16: Add Vite client types for better DX.Include
vite/clientto getimport.meta.envand asset module typings."isolatedModules": true, "noEmit": true, "jsx": "react-jsx", + "types": ["vite/client"]test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsx (1)
66-66: Inconsistent quote style.Line 66 uses double quotes while the rest of the file uses single quotes (lines 57, 75, etc.). Maintain consistency.
Apply this diff:
- children: "foo", + children: 'foo',.circleci/src/jobs/e2e-ui-vitest-3.yml (2)
11-16: Drop redundant Yarn immutability togglesYou’re using both --no-immutable and YARN_ENABLE_IMMUTABLE_INSTALLS=false. One is enough. Prefer the CLI flag for clarity.
- command: yarn install --no-immutable - working_directory: test-storybooks/portable-stories-kitchen-sink/react-vitest-3 - environment: - YARN_ENABLE_IMMUTABLE_INSTALLS: false + command: yarn install --no-immutable + working_directory: test-storybooks/portable-stories-kitchen-sink/react-vitest-3
20-24: Optional: upload Playwright traces/videos for debuggingIf generated, also store playwright-report or traces/videos to aid triage.
- store_artifacts: path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/ destination: playwright + - store_artifacts: + path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-report + destination: playwright-report + - store_artifacts: + path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/traces + destination: playwright-tracestest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.ts (1)
6-6: Unify host with Playwright webServer to avoid cross-origin quirksUse 127.0.0.1 (matches config) to keep cookies/storage on the same origin as the server.
-const STORYBOOK_URL = "http://localhost:6006"; +const STORYBOOK_URL = "http://127.0.0.1:6006";test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.ts (1)
5-5: Unify host with Playwright webServerMatch 127.0.0.1 to avoid localhost vs 127.0.0.1 origin differences.
-const STORYBOOK_URL = "http://localhost:6006"; +const STORYBOOK_URL = "http://127.0.0.1:6006";test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts (2)
8-8: Unify host with Playwright webServerSwitch to 127.0.0.1 to match the config URL.
-const STORYBOOK_URL = "http://localhost:6006"; +const STORYBOOK_URL = "http://127.0.0.1:6006";
141-143: Drop redundantawaitonpage.locator(...)
locator()is synchronous; removeawaitfor clarity.- const testingModuleDescription = await page.locator( + const testingModuleDescription = page.locator( "#testing-module-description" );test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.js (1)
6-10: Make path resolution robust and strengthen replacementUse
__dirnameto avoid CWD sensitivity and a regex for safer replacement.-const testStoryPath = path.resolve("stories/AddonTest.stories.tsx"); +const testStoryPath = path.resolve(__dirname, "stories", "AddonTest.stories.tsx"); @@ -fs.writeFileSync(testStoryPath, storyContent.replace("forceFailure: true", "forceFailure: false")); +fs.writeFileSync( + testStoryPath, + storyContent.replace(/forceFailure:\s*true/g, "forceFailure: false") +);test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsx (1)
3-5: Set button type to avoid unintended submit in form contextsNot required here, but safer generally.
-export const Component = () => { - return <button>{getButtonString()}</button>; -}; +export const Component = () => { + return <button type="button">{getButtonString()}</button>; +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/yarn.lockis excluded by!**/yarn.lock,!**/*.locktest-storybooks/portable-stories-kitchen-sink/react/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (38)
.circleci/README.md(1 hunks).circleci/src/jobs/e2e-ui-vitest-3.yml(1 hunks).circleci/src/workflows/daily.yml(1 hunks).circleci/src/workflows/merged.yml(1 hunks).circleci/src/workflows/normal.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.eslintrc.cjs(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.gitignore(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/aliased.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/get-decorator-string.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/main.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview-head.html(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/setup-file-dependency.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/test-static-dirs/static.js(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/vitest.setup.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.json(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.js(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/button.css(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.json(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.node.json(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/vite.config.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/package.json(2 hunks)test-storybooks/portable-stories-kitchen-sink/react/vite.config.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/svelte/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- test-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.ts
- test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.yarnrc.yml
- test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview-head.html
- test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.gitignore
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
test-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/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.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/stories/MyButton.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react/vite.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode
Files:
test-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/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.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/stories/MyButton.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react/vite.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts
test-storybooks/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain test configurations and assets under test-storybooks/ for Storybook testing
Files:
test-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/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jstest-storybooks/portable-stories-kitchen-sink/svelte/package.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.node.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/button.csstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react/vite.config.tstest-storybooks/portable-stories-kitchen-sink/react/package.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
📚 Learning: 2025-09-25T09:21:27.284Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to test-storybooks/** : Maintain test configurations and assets under test-storybooks/ for Storybook testing
Applied to files:
.circleci/README.mdtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.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/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react/vite.config.ts
📚 Learning: 2025-09-25T09:21:27.284Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
Applied to files:
.circleci/src/jobs/e2e-ui-vitest-3.ymltest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
PR: storybookjs/storybook#32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview.tsx
🧬 Code graph analysis (13)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsx (1)
code/core/src/csf/csf-factories.ts (1)
Meta(77-96)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsx (1)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.ts (1)
getButtonString(1-3)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.ts (1)
code/e2e-tests/util.ts (1)
SbPage(8-255)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/vitest.setup.ts (1)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/setup-file-dependency.ts (1)
getString(1-3)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.ts (1)
code/e2e-tests/util.ts (1)
SbPage(8-255)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsx (1)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsx (2)
Button(30-44)ButtonProps(4-25)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsx (1)
code/core/src/csf/csf-factories.ts (1)
Meta(77-96)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx (4)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsx (1)
Component(3-5)code/core/src/test/index.ts (1)
instrument(24-44)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/test-static-dirs/static.js (2)
staticFunction(3-3)staticFunction(3-3)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/aliased.ts (1)
aliasedFunction(4-4)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts (1)
code/e2e-tests/util.ts (1)
SbPage(8-255)
test-storybooks/portable-stories-kitchen-sink/react/vite.config.ts (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-448)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview.tsx (1)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/get-decorator-string.ts (1)
getDecoratorString(1-3)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsx (1)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsx (2)
Button(30-44)ButtonProps(4-25)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts (2)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.js (1)
path(4-4)scripts/ecosystem-ci/before-test.js (1)
__dirname(15-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (31)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.eslintrc.cjs (1)
1-14: LGTM! Standard ESLint configuration for React + TypeScript + Storybook.The configuration is appropriate for a modern React + TypeScript + Storybook project with Vitest. All settings—including the react-refresh rule for HMR, the parser, plugins, and extended configs—are standard and correct.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/test-static-dirs/static.js (1)
1-3: LGTM! Minimal test helper for staticDirs verification.The implementation is appropriate for its purpose—a simple smoke test to verify that files from staticDirs can be loaded in Vitest tests.
test-storybooks/portable-stories-kitchen-sink/react/package.json (1)
78-81: LGTM! Vitest 4 beta upgrade is consistent.The upgrade to Vitest 4.0.0-beta.17 across all related packages (@vitest/browser, @vitest/browser-playwright, @vitest/coverage-v8, @vitest/ui, and vitest core) maintains version consistency. The addition of @vitest/browser-playwright aligns with the PR's objective to support Vitest 4 with Playwright browser integration.
Also applies to: 93-93
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/get-decorator-string.ts (1)
1-3: LGTM!Simple test helper function with clear purpose.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsx (1)
1-22: LGTM!Test story with intentional failure is properly tagged with 'fail-on-purpose' for test validation purposes.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/vite.config.ts (1)
1-7: LGTM!Standard Vite configuration with React plugin, appropriate for the test storybook setup.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/setup-file-dependency.ts (1)
1-3: LGTM!Test helper function to verify setup file dependency resolution.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/aliased.ts (1)
1-4: LGTM!Well-documented test helper for verifying Vite alias resolution in the Vitest environment.
test-storybooks/portable-stories-kitchen-sink/svelte/package.json (1)
69-69: Confirm need for @vitest/browser-playwright in Svelte tests
No vitest.config or test scripts reference browser/Playwright in the Svelte subpackage. If you don’t require browser-mode or E2E testing, adding @vitest/browser-playwright isn’t necessary. Otherwise, install it and update your vitest.config accordingly.test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview.tsx (1)
6-29: LGTM!The preview configuration is well-structured with appropriate decorators, initial globals, and global types for locale switching. The decorator implementation correctly wraps stories with test-friendly attributes.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.json (3)
15-15: LGTM! Playwright version correctly pinned.The Playwright version is correctly pinned to 1.52.0 in resolutions and devDependencies, aligning with the project's E2E testing standards.
Based on learnings.
Also applies to: 51-51, 58-58
71-73: Confirm Vitest 3 usage in a Vitest 4 support PR.This test setup uses Vitest 3.x (
vitest: "^3.2.4"and related@vitest/*packages), while the PR title indicates "Vitest 4 Support". Verify whether this is intentional (e.g., to maintain backward compatibility testing alongside Vitest 4) or if these versions should be updated.Also applies to: 82-82
9-9: playwright-e2e.config.ts presence verified Verified that the referenced configuration file exists; no further action needed.test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/main.ts (2)
1-11: LGTM!The configuration structure is appropriate for a test Storybook setup, with proper story patterns, addon registration, framework configuration, and external refs.
Also applies to: 38-50
33-33: Engines.node specifies ≥22.0.0, satisfying import.meta.dirname support—no changes required.test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsx (4)
1-1: LGTM!Import statement is correct and follows Storybook conventions.
3-13: LGTM!The error-throwing helpers are correctly implemented:
unhandledRejectionis intentionallyasyncto create a rejected promise when it throwsunhandledErrorthrows synchronously for testing unhandled errorsthrowErroris a simple helper for consistency
31-46: LGTM!Story definitions follow Storybook conventions and cover the three test scenarios:
UnhandledErrorfor testing unhandled synchronous errorsUnhandledRejectionfor testing unhandled promise rejectionsSuccessas a control case
14-30: Tests dynamically overrideforceFailure; default stories are correct
E2E and Vitest scripts setforceFailuretotrueat runtime, so having it default tofalsein the story is intentional.test-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.json (1)
10-11: moduleResolution "bundler" requirement satisfied: this project pins TypeScript to ^5.8.3 (≥5.0.0).test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/button.css (1)
1-30: LGTM!The CSS follows a clear BEM-like naming convention with well-defined modifiers for button variants and sizes.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsx (1)
1-19: LGTM!The story file correctly uses CSF3 format with TypeScript types. The
'!test'tag on line 9 appropriately excludes this story from test runs, which appears intentional for testing the Vitest integration's filtering behavior.test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsx (1)
1-1: Import path is correct'storybook/test'is the configured internal alias for the test utilities; no change required.test-storybooks/portable-stories-kitchen-sink/react/vite.config.ts (3)
1-4: LGTM! Imports are correctly aligned with Vitest 4.The imports correctly use
vitest/configfordefineConfig(Vitest 4 pattern) and include the necessary plugins for Storybook test integration with Playwright browser support.
8-19: LGTM! Clean conditional plugin configuration.The conditional logic for excluding the "fail-on-purpose" tag based on the environment variable is well-structured and allows for flexible test execution.
23-29: Verify if disabling the web optimizer is necessary.Setting
deps.optimizer.web.enabled: falsedisables Vite's dependency optimization for web targets. While this may be intentional for Storybook test compatibility, it could impact performance. Please confirm whether this configuration is required or if it can be removed.If this is a known requirement for Storybook tests with Vitest 4, consider adding a brief comment explaining why it's disabled.
.circleci/README.md (1)
56-56: Docs addition looks goodThe new e2e-ui-vitest3 job is documented and matches workflow keys.
.circleci/src/workflows/normal.yml (1)
65-67: LGTM: job added with correct dependencye2e-ui-vitest3 is correctly gated on build, mirroring e2e-ui.
.circleci/src/workflows/merged.yml (1)
66-68: LGTM: merged workflow paritye2e-ui-vitest3 added with the proper build dependency.
.circleci/src/workflows/daily.yml (1)
66-68: LGTM: daily workflow paritye2e-ui-vitest3 added and depends on build; consistent across workflows.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.ts (1)
1-3: LGTMSimple, correct utility. No issues.
| executor: | ||
| name: sb_playwright | ||
| class: medium |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Playwright version = 1.52.0 per policy
Ensure the test project uses Playwright 1.52.0 and config is aligned with our standard.
Based on learnings
Also applies to: 17-19
🏁 Script executed:
#!/bin/bash
# Check declared Playwright deps in the Vitest 3 test project
set -euo pipefail
proj="test-storybooks/portable-stories-kitchen-sink/react-vitest-3"
if [ -f "$proj/package.json" ]; then
echo "Deps in $proj/package.json:"
jq -r '.dependencies, .devDependencies | to_entries[]? | select(.key|test("playwright|@playwright/test")) | "\(.key): \(.value)"' "$proj/package.json" || true
else
echo "Missing $proj/package.json" >&2
fi
# Show any playwright config files present for a quick audit
echo
echo "Playwright config files:"
fd -a --max-results 50 -t f -E node_modules -S 'playwright*.config.*' "$proj" || trueLength of output: 305
🏁 Script executed:
#!/bin/bash
proj="test-storybooks/portable-stories-kitchen-sink/react-vitest-3"
echo "Searching for Playwright config files in $proj:"
find "$proj" -type f -iregex '.*playwright\.config\..*' || trueLength of output: 102
Add standard Playwright config for Vitest 3 E2E
- @playwright/test is pinned to v1.52.0 in
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.json– OK. - No
playwright.config.*file intest-storybooks/portable-stories-kitchen-sink/react-vitest-3; add the standardplaywright.config.ts(or reference the root config) to align with policy.
| - store_test_results: | ||
| path: test-results | ||
| - store_artifacts: | ||
| path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/ | ||
| destination: playwright |
There was a problem hiding this comment.
Fix test results path: CircleCI won’t find them at repo root
Tests run in test-storybooks/.../react-vitest-3, but store_test_results points to test-results at repo root. Align paths so JUnit results upload correctly.
Apply:
- - store_test_results:
- path: test-results
+ - store_test_results:
+ path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results📝 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.
| - store_test_results: | |
| path: test-results | |
| - store_artifacts: | |
| path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/ | |
| destination: playwright | |
| - store_test_results: | |
| path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results | |
| - store_artifacts: | |
| path: test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/ | |
| destination: playwright |
🤖 Prompt for AI Agents
In .circleci/src/jobs/e2e-ui-vitest-3.yml around lines 20 to 24, the
store_test_results step points to test-results at the repo root but the Vitest
JUnit output is written under
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results;
update the store_test_results path to the same directory used by store_artifacts
(test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results) so
CircleCI can find and upload the JUnit results.
| if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { | ||
| testStoryElement.click(); | ||
| } |
There was a problem hiding this comment.
Missing await on click may cause flake
Ensure the expand action completes before proceeding.
- 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.
| if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { | |
| testStoryElement.click(); | |
| } | |
| if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { | |
| await testStoryElement.click(); | |
| } |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts
around lines 137 to 139, the click call is missing an await which can cause
flaky tests; change the code to await the click so the expand action completes
before proceeding and optionally await a short assertion/wait until
testStoryElement.getAttribute("aria-expanded") === "true" to ensure the element
finished expanding.
| import { expect, test } from "@playwright/test"; | ||
|
|
||
| import { SbPage } from "../../../../code/e2e-tests/util"; | ||
|
|
||
| const STORYBOOK_URL = "http://localhost:6006"; | ||
|
|
||
| test.describe("composition", () => { | ||
| // the composed storybook can be slow to load, so we need to increase the timeout | ||
| test.describe.configure({ mode: "serial", timeout: 60000, retries: 2 }); | ||
| test("should filter and render composed stories", async ({ page }) => { | ||
| await page.goto(STORYBOOK_URL); | ||
| await new SbPage(page, expect).waitUntilLoaded(); | ||
|
|
||
| // Expect that composed Storybooks are visible | ||
| await expect(page.getByTitle("Storybook 8.0.0")).toBeVisible(); | ||
| await expect(page.getByTitle("Storybook 7.6.18")).toBeVisible(); | ||
|
|
||
| // Expect composed stories to be available in the sidebar | ||
| await page.locator('[id="storybook\\@8\\.0\\.0_components-badge"]').click(); | ||
| await expect( | ||
| page.locator('[id="storybook\\@8\\.0\\.0_components-badge--default"]') | ||
| ).toBeVisible(); | ||
|
|
||
| await page | ||
| .locator('[id="storybook\\@7\\.6\\.18_components-badge"]') | ||
| .click(); | ||
| await expect( | ||
| page | ||
| .locator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]') | ||
| .contentFrame() | ||
| .locator("#storybook-root") | ||
| .getByText("Default") | ||
| ).toBeVisible({ timeout: 15000 }); | ||
|
|
||
| // Expect composed stories `to be available in the search | ||
| await page.getByPlaceholder("Find components").fill("Button primary"); | ||
| await expect( | ||
| page.getByRole("option", { | ||
| name: "Primary Storybook 7.6.18 / @components / Button", | ||
| }) | ||
| ).toBeVisible(); | ||
|
|
||
| const buttonStory = page.getByRole("option", { | ||
| name: "Primary Storybook 8.0.0 / @blocks / examples / Button", | ||
| }); | ||
| await expect(buttonStory).toBeVisible(); | ||
| await buttonStory.click(); | ||
|
|
||
| // Note: this could potentially be flaky due to it accessing a hosted Storybook | ||
| await expect( | ||
| page | ||
| .locator('iframe[title="storybook-ref-storybook\\@8\\.0\\.0"]') | ||
| .contentFrame() | ||
| .getByRole("button") | ||
| ).toBeVisible({ timeout: 15000 }); | ||
| }); | ||
|
|
||
| test("should filter and render composed stories on mobile", async ({ | ||
| page, | ||
| }) => { | ||
| page.setViewportSize({ width: 320, height: 800 }); | ||
| await page.goto(STORYBOOK_URL); | ||
| await new SbPage(page, expect).waitUntilLoaded(); | ||
|
|
||
| await page.click('button[aria-label="Open navigation menu"]'); | ||
|
|
||
| // scroll down to the bottom of the element getByText('Skip to canvasStorybookSearch') | ||
|
|
||
| await page.getByTitle("Storybook 7.6.18").scrollIntoViewIfNeeded(); | ||
|
|
||
| // Expect that composed Storybooks are visible | ||
| await expect(page.getByTitle("Storybook 8.0.0")).toBeVisible(); | ||
| await expect(page.getByTitle("Storybook 7.6.18")).toBeVisible(); | ||
|
|
||
| // Expect composed stories to be available in the sidebar | ||
| await expect( | ||
| page.locator('[id="storybook\\@8\\.0\\.0_components-badge"]') | ||
| ).toBeVisible(); | ||
| await page.locator('[id="storybook\\@8\\.0\\.0_components-badge"]').click(); | ||
| await expect( | ||
| page.locator('[id="storybook\\@8\\.0\\.0_components-badge--default"]') | ||
| ).toBeVisible(); | ||
|
|
||
| await page | ||
| .locator('[id="storybook\\@7\\.6\\.18_components-badge"]') | ||
| .click(); | ||
| await page | ||
| .locator('[id="storybook\\@7\\.6\\.18_components-badge--default"]') | ||
| .click(); | ||
| await expect( | ||
| page | ||
| .locator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]') | ||
| .contentFrame() | ||
| .locator("#storybook-root") | ||
| .getByText("Default") | ||
| ).toBeVisible({ timeout: 15000 }); | ||
|
|
||
| await page.click('button[aria-label="Open navigation menu"]'); | ||
|
|
||
| // Expect composed stories `to be available in the search | ||
| await page.getByPlaceholder("Find components").fill("Button primary"); | ||
| await expect( | ||
| page.getByRole("option", { | ||
| name: "Primary Storybook 7.6.18 / @components / Button", | ||
| }) | ||
| ).toBeVisible(); | ||
|
|
||
| const buttonStory = page.getByRole("option", { | ||
| name: "Primary Storybook 8.0.0 / @blocks / examples / Button", | ||
| }); | ||
| await expect(buttonStory).toBeVisible(); | ||
| await buttonStory.click(); | ||
|
|
||
| // Note: this could potentially be flaky due to it accessing a hosted Storybook | ||
| await expect( | ||
| page | ||
| .locator('iframe[title="storybook-ref-storybook\\@8\\.0\\.0"]') | ||
| .contentFrame() | ||
| .getByRole("button") | ||
| ).toBeVisible({ timeout: 15000 }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Repo-wide check for contentFrame() misuse
Scan for other places chaining contentFrame() without awaiting and replace with frameLocator().
🏁 Script executed:
#!/bin/bash
# Find chained contentFrame() usages that likely need frameLocator()
rg -nP -C2 --type=ts 'contentFrame\(\)\s*\.\s*(locator|getBy\w+)\s*\('Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for any contentFrame() calls across the repository
rg -nP 'contentFrame\(\)' -nLength of output: 909
Use frameLocator() instead of chaining contentFrame()
Replace all .contentFrame()… chains in test-storybooks/portable-stories-kitchen-sink/{react,react-vitest-3}/e2e-tests/composition.spec.ts with page.frameLocator(iframeSelector).locator(…) to ensure proper waiting.
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.ts
lines 1-122, multiple places chain
.locator('iframe[...]').contentFrame().locator(...) or
.contentFrame().getByRole(...); replace each chain with
page.frameLocator('iframe[...]').locator(...) (or
page.frameLocator('iframe[...]').getByRole(...)) so Playwright waits for the
frame and its elements correctly; keep the existing awaits and timeout options
when calling toBeVisible() and click() so behavior and timeouts remain
unchanged.
| await page | ||
| .locator('[id="storybook\\@7\\.6\\.18_components-badge"]') | ||
| .click(); | ||
| await expect( | ||
| page | ||
| .locator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]') | ||
| .contentFrame() | ||
| .locator("#storybook-root") | ||
| .getByText("Default") | ||
| ).toBeVisible({ timeout: 15000 }); |
There was a problem hiding this comment.
Incorrect chaining of contentFrame(); use frameLocator()
contentFrame() returns a Promise<Frame|null>; chaining .locator() on it is invalid and will fail TS/type-checks. Use frameLocator() instead.
- await expect(
- page
- .locator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]')
- .contentFrame()
- .locator("#storybook-root")
- .getByText("Default")
- ).toBeVisible({ timeout: 15000 });
+ await expect(
+ page
+ .frameLocator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]')
+ .locator("#storybook-root")
+ .getByText("Default")
+ ).toBeVisible({ timeout: 15000 });📝 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.
| await page | |
| .locator('[id="storybook\\@7\\.6\\.18_components-badge"]') | |
| .click(); | |
| await expect( | |
| page | |
| .locator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]') | |
| .contentFrame() | |
| .locator("#storybook-root") | |
| .getByText("Default") | |
| ).toBeVisible({ timeout: 15000 }); | |
| await page | |
| .locator('[id="storybook\\@7\\.6\\.18_components-badge"]') | |
| .click(); | |
| await expect( | |
| page | |
| .frameLocator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]') | |
| .locator("#storybook-root") | |
| .getByText("Default") | |
| ).toBeVisible({ timeout: 15000 }); |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.ts
around lines 24 to 33, the test incorrectly chains .locator() off contentFrame()
(contentFrame() returns Promise<Frame|null> and cannot be chained); replace the
contentFrame() usage with a frameLocator targeting the iframe (e.g., use
page.frameLocator('iframe[title="storybook-ref-storybook\\@7\\.6\\.18"]').locator("#storybook-root").getByText("Default"))
so locators are used on the FrameLocator API and the TypeScript/type errors are
resolved.
| reporter: [ | ||
| ["line"], | ||
| [ | ||
| "junit", | ||
| { | ||
| embedAnnotationsAsProperties: true, | ||
| outputFile: path.join(__dirname, "..", "..", "..", "test-results"), | ||
| }, | ||
| ], | ||
| ], |
There was a problem hiding this comment.
JUnit reporter outputFile should be a file path (and avoid __dirname in TS/ESM)
Currently points to a directory and relies on __dirname. Use a concrete file name relative to the project.
reporter: [
["line"],
[
"junit",
{
embedAnnotationsAsProperties: true,
- outputFile: path.join(__dirname, "..", "..", "..", "test-results"),
+ outputFile: path.join("test-results", "junit.xml"),
},
],
],📝 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.
| reporter: [ | |
| ["line"], | |
| [ | |
| "junit", | |
| { | |
| embedAnnotationsAsProperties: true, | |
| outputFile: path.join(__dirname, "..", "..", "..", "test-results"), | |
| }, | |
| ], | |
| ], | |
| reporter: [ | |
| ["line"], | |
| [ | |
| "junit", | |
| { | |
| embedAnnotationsAsProperties: true, | |
| outputFile: path.join("test-results", "junit.xml"), | |
| }, | |
| ], | |
| ], |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts
around lines 21 to 30, the JUnit reporter's outputFile is currently pointing to
a directory and uses __dirname (which is problematic in TS/ESM); change it to a
concrete file path (e.g., "test-results/junit.xml" or use
path.join(process.cwd(), "test-results", "junit.xml")) so the reporter writes to
a file, and remove usage of __dirname in favor of process.cwd() or a relative
string.
| const meta = { | ||
| title: 'Addons/Group/Test', | ||
| component: Component, | ||
| } as Meta<typeof Component>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Fix story args typing; avoid as cast with satisfies and declare forceFailure
args.forceFailure is used in plays, but the component has no props, so args type is {}. This will trip strict TS. Declare the arg in meta and use satisfies for safer typing.
As per coding guidelines (prefer precise typings, avoid casts), apply:
-const meta = {
- title: 'Addons/Group/Test',
- component: Component,
-} as Meta<typeof Component>;
+const meta = {
+ title: 'Addons/Group/Test',
+ component: Component,
+ // make `forceFailure` a known arg for typed access in play functions
+ args: { forceFailure: false },
+} satisfies Meta<typeof Component>;Optional: you can then drop the redundant args in ExpectedFailure, or keep it to be explicit—both are fine.
#!/usr/bin/env bash
file="test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx"
echo "=== Story File Head (1–200) ==="
sed -n '1,200p' "$file"
echo
echo "=== Imports ==="
rg -n '^import' -C0 "$file"
echo
echo "=== play Usage ==="
rg -n 'play:' -C3 "$file"
echo
echo "=== args Usage ==="
rg -n 'forceFailure' -C3 "$file"#!/usr/bin/env bash
file="test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx"
echo "=== Meta Definition ==="
rg -n -C2 'Meta<.*Component>' "$file"
echo
echo "=== args in Meta ==="
rg -n 'args\s*:' -C2 "$file"
echo
echo "=== play functions ==="
rg -n 'play\s*:' -C3 "$file"
echo
echo "=== forceFailure usage ==="
rg -n 'forceFailure' -C3 "$file"🏁 Script executed:
#!/usr/bin/env bash
set -e
file="test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx"
echo "=== Meta Definition and Casting ==="
rg -n -C1 'Meta<.*Component>' "$file" || echo "No Meta<> matches"
rg -n 'as Meta<' "$file" || echo "No `as Meta` cast found"
echo
echo "=== args in Meta ==="
rg -n 'args\s*:' -C2 "$file" || echo "No args declaration in meta"
echo
echo "=== play functions ==="
rg -n -C2 'play\s*:' "$file" || echo "No play functions found"
echo
echo "=== forceFailure Usage ==="
rg -n -C2 'forceFailure' "$file" || echo "No forceFailure references"
echo
echo "=== Component Import Line ==="
import_line=$(rg -m1 '^import .*Component' "$file" || echo "")
echo "${import_line:-No import of Component found}"
component_path=$(echo "$import_line" | sed -E "s/.*from ['\"](.+)['\"].*/\1/")
if [ -n "$component_path" ]; then
for ext in tsx js; do
if [ -f "${component_path}.${ext}" ]; then
echo
echo "=== Component File: ${component_path}.${ext} (head) ==="
sed -n '1,50p' "${component_path}.${ext}"
break
fi
done
fiLength of output: 1974
#!/usr/bin/env bash
set -e
dir="test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories"
echo "=== Locate Component File ==="
find "$dir" -maxdepth 1 -type f -name 'AddonTest.tsx' -print || echo "Component file not found"
echo
file="$dir/AddonTest.tsx"
if [ -f "$file" ]; then
echo "=== Component File Head (1–200) ==="
sed -n '1,200p' "$file"
else
echo "Skipping inspection; file missing"
fi🏁 Script executed:
#!/usr/bin/env bash
file="test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsx"
if [ -f "$file" ]; then
echo "=== AddonTest.tsx (1–200) ==="
sed -n '1,200p' "$file"
else
echo "AddonTest.tsx not found at expected path"
fiLength of output: 168
#!/usr/bin/env bash
file="test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx"
echo "=== AddonTest.stories.tsx (1–20) ==="
sed -n '1,20p' "$file"Use satisfies and declare forceFailure in meta
Switch from an as cast to satisfies and add a default args entry so args.forceFailure is correctly typed:
-const meta = {
- title: 'Addons/Group/Test',
- component: Component,
-} as Meta<typeof Component>;
+const meta = {
+ title: 'Addons/Group/Test',
+ component: Component,
+ args: { forceFailure: false },
+} satisfies Meta<typeof Component>;📝 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.
| const meta = { | |
| title: 'Addons/Group/Test', | |
| component: Component, | |
| } as Meta<typeof Component>; | |
| const meta = { | |
| title: 'Addons/Group/Test', | |
| component: Component, | |
| args: { forceFailure: false }, | |
| } satisfies Meta<typeof Component>; |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx
around lines 11 to 14, replace the TypeScript "as Meta<typeof Component>" cast
with the "satisfies Meta<typeof Component>" form and add a default args object
that includes forceFailure (e.g. args: { forceFailure: false }) so
args.forceFailure is correctly typed; ensure the meta object still exports
default and any existing references to meta.args remain consistent with the new
property.
| const mockFn = fn(); | ||
| export const WithLoader: CSF3Story<{ mockFn: (val: string) => string }> = { | ||
| args: { | ||
| mockFn, | ||
| }, | ||
| loaders: [ | ||
| async () => { | ||
| mockFn.mockReturnValueOnce('mockFn return value'); | ||
| return { | ||
| value: 'loaded data', | ||
| }; | ||
| }, | ||
| ], | ||
| render: (args, { loaded }) => { | ||
| const data = args.mockFn('render'); | ||
| return ( | ||
| <div> | ||
| <div data-testid="loaded-data">{loaded.value}</div> | ||
| <div data-testid="mock-data">{String(data)}</div> | ||
| </div> | ||
| ); | ||
| }, | ||
| play: async () => { | ||
| expect(mockFn).toHaveBeenCalledWith('render'); | ||
| }, | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Potential test isolation issue with module-level mock.
The mockFn defined at module scope (line 88) is shared across test runs. If Vitest doesn't automatically reset mocks between tests, this could lead to state leakage where one test's mock setup affects another.
Consider moving the mock initialization inside the story's loaders or ensuring proper cleanup:
-const mockFn = fn();
export const WithLoader: CSF3Story<{ mockFn: (val: string) => string }> = {
- args: {
- mockFn,
- },
loaders: [
async () => {
+ const mockFn = fn();
mockFn.mockReturnValueOnce('mockFn return value');
return {
+ mockFn,
value: 'loaded data',
};
},
],Alternatively, if the current approach is intentional for testing shared mock behavior, add a comment explaining why the mock is at module scope.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsx
around lines 88 to 113, the module-level mockFn is shared across test runs which
can cause state leakage; move the mock initialization into the story (e.g.,
create the mock inside the loaders or render function) or explicitly reset it at
the start of the loader (call mockFn.mockReset()/mockClear() before configuring
mockReturnValueOnce), or if intentional, add a comment explaining why a
module-scoped mock is required for this story.
| export const Button: React.FC<ButtonProps> = (props) => { | ||
| const { primary = false, size = 'medium', backgroundColor, children, ...otherProps } = props; | ||
| const mode = primary ? 'storybook-button--primary' : 'storybook-button--secondary'; | ||
| console.log({props}) |
There was a problem hiding this comment.
Remove debug console.log statement.
The console.log adds noise to test output and serves no functional purpose. Even in test fixtures, avoid leaving debug statements in committed code.
Apply this diff:
- console.log({props})📝 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.
| console.log({props}) |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsx
around line 33, there is a debug console.log({props}) that should be removed;
delete this console.log statement so the test fixture no longer emits noisy
debug output and commit the change.
| browser: { | ||
| enabled: true, | ||
| provider: playwright({}), | ||
| headless: true, | ||
| instances: [ | ||
| { | ||
| browser: "chromium", | ||
| }, | ||
| ], | ||
| }, | ||
| setupFiles: ["./.storybook/vitest.setup.ts"], | ||
| environment: "jsdom", | ||
| }, |
There was a problem hiding this comment.
Conflicting environment configuration detected.
Setting both browser.enabled: true (Line 31) and environment: "jsdom" (Line 41) creates a configuration conflict. When browser mode is enabled in Vitest, the browser provides the environment, making the explicit environment: "jsdom" setting either redundant or potentially conflicting.
Resolution:
Remove the environment: "jsdom" line when using browser mode:
browser: {
enabled: true,
provider: playwright({}),
headless: true,
instances: [
{
browser: "chromium",
},
],
},
setupFiles: ["./.storybook/vitest.setup.ts"],
- environment: "jsdom",
},
});Additional note on Line 32:
The Playwright provider is configured with an empty object playwright({}). While this works with defaults, consider whether any explicit options (such as launch options or context configuration) would be beneficial for your test setup.
📝 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.
| browser: { | |
| enabled: true, | |
| provider: playwright({}), | |
| headless: true, | |
| instances: [ | |
| { | |
| browser: "chromium", | |
| }, | |
| ], | |
| }, | |
| setupFiles: ["./.storybook/vitest.setup.ts"], | |
| environment: "jsdom", | |
| }, | |
| browser: { | |
| enabled: true, | |
| provider: playwright({}), | |
| headless: true, | |
| instances: [ | |
| { | |
| browser: "chromium", | |
| }, | |
| ], | |
| }, | |
| setupFiles: ["./.storybook/vitest.setup.ts"], | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In test-storybooks/portable-stories-kitchen-sink/react/vite.config.ts around
lines 30 to 42, there is a conflicting Vitest config: browser.enabled is true
(line 31) while environment is explicitly set to "jsdom" (line 41); remove the
environment: "jsdom" line so the browser provider (Playwright) supplies the
environment, and optionally review the playwright({}) call on line 32 to add any
needed launch/context options if you require non-default behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.circleci/config.yml (2)
134-136: wait-on without timeout can hang jobs indefinitelyMultiple wait-on calls lack a timeout. Add a bounded timeout to avoid stuck pipelines.
Example change (replicate across all wait-on uses):
- yarn wait-on tcp:127.0.0.1:6001 - yarn wait-on tcp:127.0.0.1:6002 + yarn wait-on -t 300000 tcp:127.0.0.1:6001 + yarn wait-on -t 300000 tcp:127.0.0.1:6002Also applies to: 156-158, 604-606, 635-637, 665-667
389-391: Guard artifact uploads to avoid step failures when directories are missingstore_artifacts can fail if the path doesn't exist (e.g., when tests abort early). Create the directory conditionally before upload.
Add a guard run step before each upload:
+ - run: + name: Ensure playwright results dir exists + command: mkdir -p code/playwright-results || true - store_artifacts: destination: playwright path: code/playwright-results/Apply similarly for:
- test-storybooks/portable-stories-kitchen-sink/react/test-results/
- test-storybooks/portable-stories-kitchen-sink/react-vitest-3/test-results/
Also applies to: 419-421, 441-445, 468-469
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test-storybooks/portable-stories-kitchen-sink/react/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.circleci/README.md(1 hunks).circleci/config.yml(1 hunks).circleci/src/workflows/daily.yml(1 hunks).circleci/src/workflows/merged.yml(1 hunks).circleci/src/workflows/normal.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .circleci/README.md
- .circleci/src/workflows/daily.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
.circleci/config.yml (2)
95-95: Playwright image version aligns with guidancePinning to mcr.microsoft.com/playwright:v1.52.0-noble matches the recommended Playwright 1.52.0 for E2E.
Based on learnings
446-470: CI already exercises Vitest 4 sample viae2e-uijob
The existinge2e-uijob runs Playwright E2E againsttest-storybooks/portable-stories-kitchen-sink/react(Vitest 4).
| - run: | ||
| command: | | ||
| echo "Canceling workflow as previous step resulted in failure." | ||
| echo "To execute all checks locally, please run yarn ci-tests" | ||
| curl -X POST --header "Content-Type: application/json" "https://circleci.com/api/v2/workflow/${CIRCLE_WORKFLOW_ID}/cancel?circle-token=${WORKFLOW_CANCELER}" | ||
| name: Cancel current workflow | ||
| when: on_fail |
There was a problem hiding this comment.
Avoid leaking CircleCI token in URL; use header and fail-fast curl
Passing the token via query string risks exposure in logs/proxies. Use the Circle-Token header and fail fast on errors.
Apply:
- curl -X POST --header "Content-Type: application/json" "https://circleci.com/api/v2/workflow/${CIRCLE_WORKFLOW_ID}/cancel?circle-token=${WORKFLOW_CANCELER}"
+ curl -fsSL -X POST \
+ -H "Content-Type: application/json" \
+ -H "Circle-Token: ${WORKFLOW_CANCELER}" \
+ "https://circleci.com/api/v2/workflow/${CIRCLE_WORKFLOW_ID}/cancel"📝 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.
| - run: | |
| command: | | |
| echo "Canceling workflow as previous step resulted in failure." | |
| echo "To execute all checks locally, please run yarn ci-tests" | |
| curl -X POST --header "Content-Type: application/json" "https://circleci.com/api/v2/workflow/${CIRCLE_WORKFLOW_ID}/cancel?circle-token=${WORKFLOW_CANCELER}" | |
| name: Cancel current workflow | |
| when: on_fail | |
| - run: | |
| command: | | |
| echo "Canceling workflow as previous step resulted in failure." | |
| echo "To execute all checks locally, please run yarn ci-tests" | |
| curl -fsSL -X POST \ | |
| -H "Content-Type: application/json" \ | |
| -H "Circle-Token: ${WORKFLOW_CANCELER}" \ | |
| "https://circleci.com/api/v2/workflow/${CIRCLE_WORKFLOW_ID}/cancel" | |
| name: Cancel current workflow | |
| when: on_fail |
🤖 Prompt for AI Agents
In .circleci/config.yml around lines 5 to 11, the workflow cancel step is
leaking the CircleCI token in the query string and not failing fast; change the
curl invocation to send the token in the header (e.g., add -H "Circle-Token:
${WORKFLOW_CANCELER}") and remove the token from the URL, and add fail-fast curl
options such as --fail --silent --show-error (or at minimum --fail) so the step
returns a non-zero exit on error.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
code/addons/vitest/src/postinstall.ts (2)
86-93: Use coerced SemVer for version checks and clarify default assumptions.
satisfies(vitestVersionSpecifier, '>=x')will fail for ranges like^3.2.1orworkspace:*. Use the already computedcoercedVitestVersioninstead.Additionally, when
vitestVersionSpecifieris null (vitest not yet installed), the defaults assume Vitest 3.2+ but not 4.0+. This could lead to incorrect package installation if the user is about to install Vitest 4.Apply this diff to fix the version checks:
- const isVitest3_2OrNewer = vitestVersionSpecifier - ? satisfies(vitestVersionSpecifier, '>=3.2.0') + const isVitest3_2OrNewer = coercedVitestVersion + ? satisfies(coercedVitestVersion, '>=3.2.0') : // TODO: Set to false once we have a stable release of Vitest 4 true; - const isVitest4OrNewer = vitestVersionSpecifier - ? satisfies(vitestVersionSpecifier, '>=4.0.0') + const isVitest4OrNewer = coercedVitestVersion + ? satisfies(coercedVitestVersion, '>=4.0.0') : // TODO: Set to true once we have a stable release of Vitest 4 false;Based on learnings: Vitest 3.2.4 is the current stable release; Vitest 4 is not yet stable, which aligns with the TODO comments and current defaults.
515-531: Remove literal quotes from --config-dir argument.The literal quotes around
options.configDirwill become part of the argument string, breaking paths with spaces.execahandles spaces safely without quotes.Apply this diff:
...(options.configDir !== '.storybook' ? ['--config-dir', options.configDir] : []),
🧹 Nitpick comments (1)
code/addons/vitest/src/postinstall.ts (1)
455-466: Remove redundant conditional check.Since
getTemplateName()always returns a non-empty string (lines 397-405), theif (templateName)check at line 457 is always true and can be removed for clarity.Apply this diff to simplify the logic:
const templateName = getTemplateName(); -if (templateName) { - const configTemplate = await loadTemplate(templateName, { - CONFIG_DIR: options.configDir, - SETUP_FILE: relative(dirname(rootConfig), vitestSetupFile), - }); - - const source = babelParse(configTemplate); - target = babelParse(configFile); - updated = updateConfigFile(source, target); -} +const configTemplate = await loadTemplate(templateName, { + CONFIG_DIR: options.configDir, + SETUP_FILE: relative(dirname(rootConfig), vitestSetupFile), +}); + +const source = babelParse(configTemplate); +target = babelParse(configFile); +updated = updateConfigFile(source, target);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/vitest/src/postinstall.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
code/addons/vitest/src/postinstall.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode
Files:
code/addons/vitest/src/postinstall.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/addons/vitest/src/postinstall.ts
🧬 Code graph analysis (1)
code/addons/vitest/src/postinstall.ts (2)
code/core/src/common/utils/get-storybook-info.ts (1)
getStorybookInfo(123-131)code/addons/vitest/src/updateVitestFile.ts (1)
loadTemplate(9-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/addons/vitest/src/postinstall.ts (3)
95-104: LGTM! Version-aware dependency selection.The dependency list correctly selects
@vitest/browser-playwrightfor Vitest 4+ and@vitest/browserfor earlier versions, and the filtering prevents reinstalling existing packages.
391-394: LGTM! Version-aware shim reference.The shim reference correctly uses
@vitest/browser-playwrightfor Vitest 4+ and@vitest/browser/providers/playwrightfor earlier versions, consistent with the dependency selection.
397-405: LGTM! Clean version-aware template selection.The
getTemplateNamehelper provides a clear, version-aware template selection mechanism that aligns with the version flags defined earlier.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
code/addons/vitest/package.json(2 hunks)test-storybooks/portable-stories-kitchen-sink/react/package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test-storybooks/portable-stories-kitchen-sink/react/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
| "@vitest/browser-playwright": "^4.0.1", | ||
| "@vitest/runner": "^4.0.1", |
There was a problem hiding this comment.
🧩 Analysis chain
Vitest 4 support added with backward-compatible peer dependencies.
The changes appropriately upgrade to Vitest 4 while maintaining Vitest 3 compatibility in peer dependencies. The use of ^3.0.0 || ^4.0.0 ranges in peerDependencies allows users to continue using Vitest 3 while encouraging migration to Vitest 4.
However, there is an unresolved concern from a prior review:
Playwright version pinning — A previous review flagged that Playwright should be pinned to 1.52.0 in devDependencies for E2E consistency. Currently, no explicit Playwright dependency appears in this file. Since @vitest/browser-playwright is now a dependency, verify whether:
- Playwright is transitively pinned by
@vitest/browser-playwright@^4.0.1, or - An explicit
"playwright": "1.52.0"entry is needed in devDependencies to guarantee E2E consistency across environments.
Also applies to: 104-104, 107-111, 117-119
🏁 Script executed:
#!/bin/bash
# Verify Playwright transitive dependency resolution and check if explicit pinning is needed
cd code/addons/vitest
# Check package-lock.json or yarn.lock for resolved Playwright version
if [ -f package-lock.json ]; then
echo "=== Checking npm lock file ==="
rg -A5 '"@vitest/browser-playwright"' package-lock.json | head -20
rg '"playwright"' package-lock.json | head -10
elif [ -f yarn.lock ]; then
echo "=== Checking yarn lock file ==="
rg -A5 '@vitest/browser-playwright' yarn.lock | head -20
rg 'playwright' yarn.lock | head -10
fi
# Check if Playwright is mentioned in any template package.json files
echo -e "\n=== Checking templates for Playwright references ==="
fd -g '*.json' templates/ -x grep -l playwright {} \;Length of output: 53
I need to gather more information. The lock files may not have been present, and the grep results were inconclusive. Let me inspect the package.json directly and search for any Playwright pinning in the broader codebase.
I need to gather more information. The lock files may not have been present, and the grep results were inconclusive. Let me inspect the package.json directly and search for any Playwright pinning in the broader codebase.
🏁 Script executed:
#!/bin/bash
# Step 1: Check if the file exists and show its current state
echo "=== Listing files in code/addons/vitest/ ==="
ls -la code/addons/vitest/ | grep -E 'package.*json|lock'
echo -e "\n=== Current package.json (lines 80-125) ==="
sed -n '80,125p' code/addons/vitest/package.json
echo -e "\n=== Searching for Playwright in entire codebase ==="
rg -n -i 'playwright' --type json -C2 | grep -E '(package\.json|devDependencies|dependencies|peerDependencies)' -A2 -B2 | head -40
echo -e "\n=== Check root package.json for Playwright ==="
if [ -f package.json ]; then
rg -n '"playwright"' package.json
fiLength of output: 4352
🏁 Script executed:
#!/bin/bash
# Step 1: Check if the file exists and show its current state
echo "=== Listing files in code/addons/vitest/ ==="
ls -la code/addons/vitest/ | grep -E 'package.*json|lock'
echo -e "\n=== Current package.json (lines 80-125) ==="
sed -n '80,125p' code/addons/vitest/package.json
echo -e "\n=== Searching for Playwright in entire codebase ==="
rg -n -i 'playwright' --type json -C2 | head -50
echo -e "\n=== Check root package.json for Playwright ==="
if [ -f package.json ]; then
rg -n '"playwright"' package.json
fiLength of output: 6204
Add explicit Playwright version pinning to devDependencies.
The addon currently relies on transitive resolution of Playwright through @vitest/browser-playwright@^4.0.1, but does not explicitly pin the Playwright version. However, the entire codebase pins Playwright to 1.52.0 across all test-storybooks and scripts packages for E2E consistency:
test-storybooks/yarn-pnp/package.jsontest-storybooks/portable-stories-kitchen-sink/vue3/package.jsontest-storybooks/portable-stories-kitchen-sink/svelte/package.jsonscripts/package.json
To maintain consistency and prevent transitive version drift, add "playwright": "1.52.0" to devDependencies in code/addons/vitest/package.json.
🤖 Prompt for AI Agents
In code/addons/vitest/package.json around lines 85-86, add an explicit
devDependency entry for "playwright": "1.52.0" inside the "devDependencies"
object alongside the existing @vitest entries to pin Playwright to 1.52.0; after
updating package.json run your package manager install (yarn/npm/pnpm) to update
the lockfile so the new pinned version is recorded.
|
@valentinpalkovic I've telescoped this on top of the other release un-blocker work @yannbf did, to get a green CI we can merge with. |
ndelangen
added
ci:daily
Only add `/// <reference types="vitest/config" />` to vite.config files, not vitest.config files, because vitest.config files already have the vitest/config types available by default. Co-authored-by: yannbf <[email protected]>
When transforming vitest configs with coverage settings, the addon now correctly: - Extracts the coverage property from the existing test config - Keeps it at the top-level test object (where it's global) - Moves other test properties to workspace/projects array items - Adds test cases to verify the fix works for both workspace and projects modes Co-authored-by: yannbf <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test-storybooks/portable-stories-kitchen-sink/react/package.json (1)
57-59: Align React type packages with React 18You’re on react/react-dom ^18, but using @types/react and @types/react-dom ^19. Major mismatch can cause TS errors.
- "@types/react": "^19.0.8", - "@types/react-dom": "^19.0.3", + "@types/react": "^18", + "@types/react-dom": "^18",Also applies to: 73-75
🧹 Nitpick comments (3)
test-storybooks/portable-stories-kitchen-sink/react/.storybook/preview.tsx (1)
2-2: Run Prettier on this fileMinor formatting: missing semicolon.
-import { sb } from 'storybook/test' +import { sb } from 'storybook/test';As per coding guidelines (Prettier/ESLint).
code/addons/vitest/src/vitest-plugin/test-utils.ts (2)
17-24: Make convertToFilePath robust and cross‑platformDecode all URL encodings and handle Windows/posix uniformly via URL parsing.
-export const convertToFilePath = (url: string): string => { - // Remove the file:// protocol - const path = url.replace(/^file:\/\//, ''); - // Handle Windows paths - const normalizedPath = path.replace(/^\/+([a-zA-Z]:)/, '$1'); - // Convert %20 to spaces - return normalizedPath.replace(/%20/g, ' '); -}; +export const convertToFilePath = (url: string): string => { + try { + const u = new URL(url); + let p = decodeURIComponent(u.pathname); + // Windows drive letter (e.g., /C:/path -> C:/path) + p = p.replace(/^\/+([a-zA-Z]:)/, '$1'); + return p; + } catch { + // Fallback: treat as already a path + return url; + } +};
44-47: Safer access to getInitialGlobalsAvoid destructuring from possibly missing
server.commands. Use optional chaining.- const { server } = await getVitestBrowserContext(); - - const { getInitialGlobals } = server.commands; + const ctx = await getVitestBrowserContext(); + const getInitialGlobals = ctx?.server?.commands?.getInitialGlobals;Also applies to: 48-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.locktest-storybooks/portable-stories-kitchen-sink/react-vitest-3/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.circleci/config.yml(4 hunks).circleci/src/workflows/daily.yml(1 hunks).circleci/src/workflows/merged.yml(1 hunks).circleci/src/workflows/normal.yml(1 hunks)code/addons/vitest/src/vitest-plugin/test-utils.ts(2 hunks)code/addons/vitest/src/vitest-plugin/viewports.ts(2 hunks)code/addons/vitest/src/vitest-plugin/vitest-context.ts(1 hunks)code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/.storybook/preview.tsx(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .circleci/src/workflows/merged.yml
- .circleci/config.yml
- code/addons/vitest/src/vitest-plugin/viewports.ts
- .circleci/src/workflows/daily.yml
- .circleci/src/workflows/normal.yml
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.tstest-storybooks/portable-stories-kitchen-sink/react/package.json
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts
test-storybooks/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain test Storybook configurations under
test-storybooks/for E2E and visual testing scenarios
Files:
test-storybooks/portable-stories-kitchen-sink/react/package.json
🧠 Learnings (2)
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
PR: storybookjs/storybook#32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
test-storybooks/portable-stories-kitchen-sink/react/.storybook/preview.tsx
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Mock external dependencies in tests using `vi.mock()` (e.g., filesystem, loggers)
Applied to files:
code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts
🧬 Code graph analysis (2)
test-storybooks/portable-stories-kitchen-sink/react/.storybook/preview.tsx (1)
code/core/src/test/index.ts (1)
sb(54-60)
code/addons/vitest/src/vitest-plugin/test-utils.ts (1)
code/addons/vitest/src/vitest-plugin/vitest-context.ts (1)
getVitestBrowserContext(14-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts (1)
43-44: LGTM! Vitest 4 node mocker support correctly implemented.The changes consistently extend the existing pattern:
'@vitest/mocker/node'is added tooptimizeDeps.includealongside the existing browser and base variants- The corresponding alias mapping follows the same resolution pattern used for other mocker modules
- Excluding
fsevents(a macOS-specific native module) from optimization is a standard best practiceAll three additions work together to ensure proper resolution and optimization of Vitest 4's node-specific mocker module, particularly for package managers like pnpm as noted in the existing comment.
Also applies to: 51-51
test-storybooks/portable-stories-kitchen-sink/react/.storybook/preview.tsx (1)
5-5: The review comment requires verification against official Storybook documentationBased on official Storybook documentation, when registering a mock for a local module, the path must include the file extension (e.g., .ts or .js). The code as written—
sb.mock('./get-decorator-string.ts', { spy: true })—actually follows this requirement correctly by including the.tsextension.The review comment suggests removing the extension, but this contradicts Storybook's documented requirement that local module paths must include the file extension. Without access to verify the actual import statements in the file or evidence that imports omit the extension while the mock includes it, the suggestion to remove the extension appears to conflict with Storybook's explicit guidance.
Likely an incorrect or invalid review comment.
test-storybooks/portable-stories-kitchen-sink/react/package.json (1)
77-81: I need to gather more specific information to complete the verification. Let me search for the exact peer dependencies and Storybook 10 compatibility:Let me search for the actual package.json file to verify the complete context and see lines 89-93 mentioned in the review:
Compatibility verified—no issues found.
Vite 7 is supported by Vitest 4 and is compatible with @vitejs/plugin-react v5. @vitest/browser-playwright v4 is a separate provider package paired with Vitest 4. Storybook maintains compatibility across Vitest versions and supports Vitest 2.1.x, with no constraints preventing Storybook 10 from working with Vitest 4. The version ranges in lines 77–81 and 89–93 satisfy all peer dependencies and are mutually compatible.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/vitest/src/postinstall.ts (1)
295-301: Now let me check the actual context in the postinstall.ts file to understand what dependencies are being processed:Correct the version pinning logic to only match the exact 'vitest' package.
The current
includes('vitest')match is overly broad and will incorrectly pin @vitest/browser and other @vitest/* packages to the vitest version, which can resolve to non-existent versions since these packages won't play nicely if they differ in versions, but require independent version management.Use exact string matching instead:
- const versionedDependencies = uniqueDependencies.map((p) => { - if (p.includes('vitest')) { - return vitestVersionSpecifier ? `${p}@${vitestVersionSpecifier}` : p; - } - return p; - }); + const versionedDependencies = uniqueDependencies.map((p) => + p === 'vitest' && vitestVersionSpecifier ? `${p}@${vitestVersionSpecifier}` : p + );
♻️ Duplicate comments (2)
code/addons/vitest/src/postinstall.ts (2)
86-91: Use coerced SemVer for range checks (specifier can mis-evaluate).Compare against
coercedVitestVersion, not the raw specifier (handles^3.2.1,workspace:*, etc.).Apply:
- const isVitest3_2To4 = vitestVersionSpecifier - ? satisfies(vitestVersionSpecifier, '>=3.2.0 <4.0.0') + const isVitest3_2To4 = coercedVitestVersion + ? satisfies(coercedVitestVersion, '>=3.2.0 <4.0.0') : false; - const isVitest4OrNewer = vitestVersionSpecifier - ? satisfies(vitestVersionSpecifier, '>=4.0.0') + const isVitest4OrNewer = coercedVitestVersion + ? satisfies(coercedVitestVersion, '>=4.0.0') : true;
515-531: Do not quote argv values passed to execa (quotes become literal).Let
execahandle spaces; pass raw path.- ...(options.configDir !== '.storybook' ? ['--config-dir', `"${options.configDir}"`] : []), + ...(options.configDir !== '.storybook' ? ['--config-dir', options.configDir] : []),
🧹 Nitpick comments (2)
code/addons/vitest/src/updateVitestFile.ts (1)
252-269: Coverage extraction looks right; add a duplicate guard.Logic is solid. To be defensive, avoid inserting a second
coverageif a template already provides one.Apply this small guard:
- if (coverageProp && templateTestProp.value.type === 'ObjectExpression') { - templateTestProp.value.properties.unshift(coverageProp); - } + if ( + coverageProp && + templateTestProp.value.type === 'ObjectExpression' && + !templateTestProp.value.properties.some( + (p) => + p.type === 'ObjectProperty' && + p.key.type === 'Identifier' && + p.key.name === 'coverage' + ) + ) { + templateTestProp.value.properties.unshift(coverageProp); + }Also applies to: 300-304
code/addons/vitest/src/updateVitestFile.test.ts (1)
10-16: Adopt spy-based mocks and vi.mocked() per repo test guidelines.Use
spy: trueand move behavior intobeforeEachwithvi.mocked(...)for type-safety.As per coding guidelines
Example minimal change for the logger mock:
-vi.mock('storybook/internal/node-logger', () => ({ - logger: { - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - }, -})); +vi.mock('storybook/internal/node-logger', { spy: true }); +// later at top-level or in beforeEach(): +// const { logger } = await import('storybook/internal/node-logger'); +// vi.mocked(logger.info).mockReset(); +// vi.mocked(logger.warn).mockReset(); +// vi.mocked(logger.error).mockReset();For
resolvePackageDir, you can similarly:-vi.mock('../../../core/src/shared/utils/module', () => ({ - resolvePackageDir: vi.fn().mockImplementation(() => join(__dirname, '..')), -})); +vi.mock('../../../core/src/shared/utils/module', { spy: true }); +// in beforeEach: +// const { resolvePackageDir } = await import('../../../core/src/shared/utils/module'); +// vi.mocked(resolvePackageDir).mockReturnValue(join(__dirname, '..'));Also applies to: 18-20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/addons/vitest/src/postinstall.ts(8 hunks)code/addons/vitest/src/updateVitestFile.test.ts(11 hunks)code/addons/vitest/src/updateVitestFile.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/src/postinstall.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/src/updateVitestFile.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/addons/vitest/src/postinstall.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/src/updateVitestFile.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/addons/vitest/src/postinstall.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/src/updateVitestFile.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/postinstall.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/src/updateVitestFile.ts
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/addons/vitest/src/updateVitestFile.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/addons/vitest/src/updateVitestFile.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/addons/vitest/src/updateVitestFile.test.ts
🧠 Learnings (2)
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/addons/vitest/src/postinstall.tscode/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
🧬 Code graph analysis (2)
code/addons/vitest/src/postinstall.ts (2)
code/core/src/common/utils/get-storybook-info.ts (1)
getStorybookInfo(123-131)code/addons/vitest/src/updateVitestFile.ts (1)
loadTemplate(9-16)
code/addons/vitest/src/updateVitestFile.test.ts (1)
code/addons/vitest/src/updateVitestFile.ts (2)
loadTemplate(9-16)updateConfigFile(79-325)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/addons/vitest/src/postinstall.ts (3)
390-392: TS shim reference looks good; minor: broaden provider fallback?Reference selection by version is correct. If you later support non-Playwright providers on Vitest 4+, consider deriving types from chosen provider.
395-403: Template selection logic LGTM.Clear 4.x/3.2+ split with default fallback.
471-479: Nice: triple-slash reference only for vite.config files.Good guard to avoid duplicate references in vitest.config.*.
| try { | ||
| const storybookVersion = await packageManager.getInstalledVersion('storybook'); | ||
| dependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); | ||
| uniqueDependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); | ||
| } catch (e) { | ||
| console.error('Failed to install @storybook/nextjs-vite. Please install it manually'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Use Storybook logger instead of console.error (repo guideline).
Replace console.error with logger.error for consistency.
As per coding guidelines
- console.error('Failed to install @storybook/nextjs-vite. Please install it manually');
+ logger.error('Failed to install @storybook/nextjs-vite. Please install it manually');
@@
- console.error('Failed to install Playwright. Please install it manually');
+ logger.error('Failed to install Playwright. Please install it manually');Also applies to: 324-330
🏁 Script executed:
#!/bin/bash
rg -n "console\\.(error|warn|log)\\(" code/addons/vitest/src/postinstall.ts -nLength of output: 181
Now I'll check the actual file to see if the logger is already imported:
🏁 Script executed:
#!/bin/bash
# Check imports at the top of the vitest postinstall file
head -40 code/addons/vitest/src/postinstall.ts
echo "---"
# Check the specific lines with console.error
sed -n '270,280p' code/addons/vitest/src/postinstall.ts
echo "---"
sed -n '320,335p' code/addons/vitest/src/postinstall.tsLength of output: 2570
Replace console.error with logger.error to comply with Storybook coding guidelines.
The logger is already imported in the file at line 20 and used consistently elsewhere. Replace both occurrences:
- console.error('Failed to install @storybook/nextjs-vite. Please install it manually');
+ logger.error('Failed to install @storybook/nextjs-vite. Please install it manually');- console.error('Failed to install Playwright. Please install it manually');
+ logger.error('Failed to install Playwright. Please install it manually');📝 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.
| try { | |
| const storybookVersion = await packageManager.getInstalledVersion('storybook'); | |
| dependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); | |
| uniqueDependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); | |
| } catch (e) { | |
| console.error('Failed to install @storybook/nextjs-vite. Please install it manually'); | |
| } | |
| try { | |
| const storybookVersion = await packageManager.getInstalledVersion('storybook'); | |
| uniqueDependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); | |
| } catch (e) { | |
| logger.error('Failed to install @storybook/nextjs-vite. Please install it manually'); | |
| } |
🤖 Prompt for AI Agents
In code/addons/vitest/src/postinstall.ts around lines 272 to 277, replace the
console.error call with logger.error (the logger is already imported at line
20); change the catch block to call logger.error('Failed to install
@storybook/nextjs-vite. Please install it manually', e) so the error is logged
consistently and preserve the original error details, and remove any unused
console references elsewhere if present.
… object" This reverts commit ad8d67f.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
code/addons/vitest/src/updateVitestFile.test.ts (2)
1-1: Add required imports for the ESM-safe mock.Needed by the refactor above.
-import { join } from 'node:path'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url';
18-20: Add{ spy: true }to bothvi.mock()calls per Vitest testing guidelines.The test file has two
vi.mock()calls (lines 10–16 and 18–20) that lack the{ spy: true }option. Per your guidelines, all module mocks in Vitest tests must include this option to enable proper spy behavior while preserving original implementations.Apply to both mocks:
vi.mock('storybook/internal/node-logger', () => ({ logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), }, -})); +}), { spy: true }); vi.mock('../../../core/src/shared/utils/module', () => ({ resolvePackageDir: vi.fn().mockImplementation(() => join(__dirname, '..')), -})); +}), { spy: true });Optionally, add
afterEach(() => vi.restoreAllMocks())after the mock declarations to reset mock state between tests.
🧹 Nitpick comments (4)
code/addons/vitest/src/updateVitestFile.test.ts (4)
10-16: Use spy mocks and ensure cleanup.Per guidelines, enable spies and restore between tests.
Apply:
-vi.mock('storybook/internal/node-logger', () => ({ +vi.mock('storybook/internal/node-logger', () => ({ logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), }, -})); +}), { spy: true });Also add an afterEach to restore mocks (see next comment). As per coding guidelines.
3-3: Restore mocks after each test.Prevents cross-test leakage.
-import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi, afterEach } from 'vitest';+afterEach(() => { + vi.restoreAllMocks(); +});Also applies to: 22-22
56-101: Reduce snapshot brittleness; assert stable invariants instead.Inline snapshots of full diffs are fragile across formatter/Babel/Vitest version shifts. Prefer focused assertions on key invariants (e.g., presence of storybookTest import, dirname fallback, workspace/projects entries, and the browser block) or AST-level checks.
Example direction (illustrative):
- expect(after).toContain("@storybook/addon-vitest/vitest-plugin")
- expect(after).toMatch(/workspace:\s*[/s)
- expect(after).toMatch(/browser:\s*{[\s\S]?provider:\s'playwright'/)
Keeps tests resilient while still verifying behavior.
Also applies to: 135-180, 247-291, 327-371, 406-450, 482-525, 564-612, 646-688, 725-772, 800-830, 858-889
141-149: Unused import in “object notation” snapshot.The expected diff adds
import { defineConfig } from 'vitest/config'but the resulting file continues to export a plain object. That import would be unused and likely fail linting in user projects.
- If intentional, document why the import is needed.
- Otherwise, adjust updateConfigFile to avoid injecting unused imports and update this snapshot accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/addons/vitest/src/updateVitestFile.test.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/addons/vitest/src/updateVitestFile.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/addons/vitest/src/updateVitestFile.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/src/updateVitestFile.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/addons/vitest/src/updateVitestFile.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/addons/vitest/src/updateVitestFile.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/addons/vitest/src/updateVitestFile.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/updateVitestFile.test.ts
🧠 Learnings (2)
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
- Removed optional peer dependencies for `@vitest/browser-playwright` and `vitest` in package.json and yarn.lock. - Updated import statement in `vitest-context.ts` to use `vitest/browser` instead of `@vitest/browser`. - Adjusted dependency inclusion in `vite-inject-mocker/plugin.ts` for better readability. - Added `@vitest/browser` as a direct dependency in the portable stories kitchen sink package.json. These changes streamline the Vitest integration and ensure compatibility with version 4.0.0.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
code/addons/vitest/package.json (1)
85-110: Backward-compatible Vitest 4 support is well-structured.The upgrade to Vitest 4 in devDependencies while maintaining backward-compatible peer dependency ranges (
^3.0.0 || ^4.0.0) allows users to gradually migrate from Vitest 3 to 4. The switch from@vitest/browserto@vitest/browser-playwrightin devDependencies aligns with Vitest 4's architecture.However, a previous review comment flagged that Playwright should be explicitly pinned to
1.52.0in devDependencies for E2E consistency across the codebase. This remains unaddressed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
code/addons/vitest/package.json(2 hunks)code/addons/vitest/src/vitest-plugin/vitest-context.ts(1 hunks)code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/core-server/presets/vitePlugins/vite-inject-mocker/plugin.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
test-storybooks/portable-stories-kitchen-sink/react/package.jsoncode/addons/vitest/package.jsoncode/addons/vitest/src/vitest-plugin/vitest-context.ts
test-storybooks/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain test Storybook configurations under
test-storybooks/for E2E and visual testing scenarios
Files:
test-storybooks/portable-stories-kitchen-sink/react/package.json
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/vitest-plugin/vitest-context.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Use Vitest as the test runner for this repository
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
test-storybooks/portable-stories-kitchen-sink/react/package.json (1)
63-93: LGTM! Version upgrades align with Vitest 4 and Storybook 10 migration.All dependency updates are consistent and support the PR's goal of adding Vitest 4 compatibility while upgrading the Storybook ecosystem to v10.
code/addons/vitest/src/vitest-plugin/vitest-context.ts (1)
3-11: LGTM! Type definitions appropriately extend Vitest context.The type structure correctly extends
VitestBrowserContextwith the customgetInitialGlobalscommand, providing proper typing for the server context augmentation.
| @@ -0,0 +1,24 @@ | |||
| import semver from 'semver'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace version sniffing with feature detection to improve robustness.
The current approach uses JSON import attributes and semver to detect Vitest version, but this has several drawbacks:
- Import attributes brittleness: JSON import attributes (
with: { type: 'json' }) are not universally supported across all Node versions and bundlers, and the module shape may vary (direct properties vsdefault.version). - Runtime overhead: Adding semver as a runtime dependency just for version detection is heavy.
- Maintenance burden: Version checks require updates when Vitest changes its versioning scheme.
A previous review comment suggested using feature detection instead, which is more robust and eliminates the semver dependency.
Apply the feature detection approach suggested in the prior review:
-import semver from 'semver';
-
type VitestBrowserContext = typeof import('@vitest/browser/context');
type VitestServerContext = VitestBrowserContext & {
server: {
commands: typeof import('@vitest/browser/context').server.commands & {
getInitialGlobals: () => Promise<Record<string, any>>;
};
};
};
-/** Gets the Vitest browser context based on which version of Vitest is installed. */
+/** Gets the Vitest browser context by feature detection (v4 first, then v3). */
export const getVitestBrowserContext = async (): Promise<VitestServerContext> => {
- const vitestVersion = await import('vitest/package.json', { with: { type: 'json' } }).then(
- (v) => v.version
- );
-
- if (semver.major(vitestVersion) >= 4) {
- return import('vitest/browser') as unknown as Promise<VitestServerContext>;
+ try {
+ return (await import('vitest/browser')) as unknown as VitestServerContext;
+ } catch {
+ return (await import('@vitest/browser/context')) as unknown as VitestServerContext;
}
-
- return import('@vitest/browser/context') as Promise<VitestServerContext>;
};Optionally, add caching to avoid repeated dynamic imports:
let _ctxPromise: Promise<VitestServerContext> | undefined;
export const getVitestBrowserContext = async (): Promise<VitestServerContext> => {
if (_ctxPromise) return _ctxPromise;
_ctxPromise = (async () => {
try {
return (await import('vitest/browser')) as unknown as VitestServerContext;
} catch {
return (await import('@vitest/browser/context')) as unknown as VitestServerContext;
}
})();
return _ctxPromise;
};Also applies to: 15-20
🤖 Prompt for AI Agents
In code/addons/vitest/src/vitest-plugin/vitest-context.ts around lines 1 and
15-20, replace the current Vitest version-sniffing logic (including the semver
dependency and JSON import-attributes) with feature-detection via attempting to
import the newer module path first and falling back to the older one; implement
an async cached loader (a top-level Promise variable) so repeated calls reuse
the same dynamic import result; remove the semver import and any JSON-with-type
import logic and update exports to use the resolved module shape directly as the
VitestServerContext.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/vitest/src/vitest-plugin/test-utils.ts (1)
17-24: Incomplete URL decoding: usedecodeURIComponentfor all encoded characters.The function only decodes
%20(spaces) but URLs can contain many other encoded characters (%2F,%3A,%28,%29, etc.). This will cause issues with file paths containing these characters.Apply this diff to properly decode all URL-encoded characters:
export const convertToFilePath = (url: string): string => { // Remove the file:// protocol const path = url.replace(/^file:\/\//, ''); // Handle Windows paths const normalizedPath = path.replace(/^\/+([a-zA-Z]:)/, '$1'); - // Convert %20 to spaces - return normalizedPath.replace(/%20/g, ' '); + // Decode all URL-encoded characters + return decodeURIComponent(normalizedPath); };
♻️ Duplicate comments (1)
code/addons/vitest/package.json (1)
87-88: Add explicit Playwright pinning for E2E consistency.As noted in previous reviews, explicit Playwright version pinning to
1.52.0is missing. While@vitest/browser-playwright@^4.0.1brings Playwright transitively, the version could drift across installs. Other test storybooks in the repo (test-storybooks/yarn-pnp, portable-stories-kitchen-sink/vue3, etc.) explicitly pin to1.52.0for E2E consistency.Apply this diff to pin Playwright:
"@types/semver": "^7", "@vitest/browser-playwright": "^4.0.1", "@vitest/runner": "^4.0.1", + "playwright": "1.52.0", "boxen": "^8.0.1",Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
code/addons/vitest/build-config.ts(1 hunks)code/addons/vitest/package.json(3 hunks)code/addons/vitest/src/vitest-plugin/index.ts(7 hunks)code/addons/vitest/src/vitest-plugin/test-utils.ts(2 hunks)code/addons/vitest/src/vitest-plugin/types.ts(1 hunks)code/addons/vitest/src/vitest-plugin/viewports.ts(2 hunks)code/addons/vitest/src/vitest-plugin/vitest-context-legacy.ts(1 hunks)code/addons/vitest/src/vitest-plugin/vitest-context-modern.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/cypress/support/component.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- code/addons/vitest/src/vitest-plugin/vitest-context-legacy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/addons/vitest/src/vitest-plugin/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/src/vitest-plugin/test-utils.tstest-storybooks/portable-stories-kitchen-sink/react/cypress/support/component.tscode/addons/vitest/src/vitest-plugin/types.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/build-config.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/package.jsontest-storybooks/portable-stories-kitchen-sink/react/package.json
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/addons/vitest/src/vitest-plugin/test-utils.tstest-storybooks/portable-stories-kitchen-sink/react/cypress/support/component.tscode/addons/vitest/src/vitest-plugin/types.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/build-config.tscode/addons/vitest/src/vitest-plugin/viewports.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/addons/vitest/src/vitest-plugin/test-utils.tscode/addons/vitest/src/vitest-plugin/types.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/build-config.tscode/addons/vitest/src/vitest-plugin/viewports.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/vitest-plugin/test-utils.tscode/addons/vitest/src/vitest-plugin/types.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/build-config.tscode/addons/vitest/src/vitest-plugin/viewports.ts
test-storybooks/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain test Storybook configurations under
test-storybooks/for E2E and visual testing scenarios
Files:
test-storybooks/portable-stories-kitchen-sink/react/cypress/support/component.tstest-storybooks/portable-stories-kitchen-sink/react/package.json
🧠 Learnings (1)
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
code/addons/vitest/build-config.ts
🧬 Code graph analysis (2)
code/addons/vitest/src/vitest-plugin/test-utils.ts (1)
code/addons/vitest/src/vitest-plugin/types.ts (1)
VitestBrowserContext(13-18)
code/addons/vitest/src/vitest-plugin/viewports.ts (1)
code/addons/vitest/src/vitest-plugin/types.ts (1)
VitestBrowserContext(13-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (14)
test-storybooks/portable-stories-kitchen-sink/react/cypress/support/component.ts (1)
20-20: No issues found—the import change is correct and follows Cypress best practices.The change from
cypress/react18tocypress/reactis the official Cypress migration path, as the separate react18 harness has been merged into cypress/react. For React 18+, the correct import iscypress/react. Verification confirms no othercypress/react18imports remain in the codebase, so the migration is complete and consistent.code/addons/vitest/src/vitest-plugin/test-utils.ts (2)
1-9: LGTM: Imports are clean and necessary.The removal of
toTestIdis appropriate since it's not used in the file, and the addition ofVitestBrowserContextsupports the new dynamic context retrieval pattern.
51-57: LGTM: Defensive handling ofgetInitialGlobals.The use of optional chaining (
getInitialGlobals?.()) and nullish coalescing (?? {}) properly handles cases where the command might not be available, ensuring the story composition doesn't fail due to missing globals.code/addons/vitest/src/vitest-plugin/vitest-context-modern.ts (1)
1-1: LGTM: Clean re-export for Vitest 4+ browser context.The barrel export correctly proxies the modern Vitest browser API surface for version-aware resolution.
code/addons/vitest/src/vitest-plugin/types.ts (2)
3-18: Well-structured type extensions for Vitest browser context.The type declarations cleanly model the extended browser context with optional a11y configuration and server commands. The structure properly extends Vitest's base types.
1-1: Based on the web search results, I found critical information. Vitest 4's migration guide states that imports should be updated from@vitest/browser/contexttovitest/browser. This is a breaking API change in Vitest 4.Now I have key information. Let me check one more aspect: how types.ts is used and whether it's exposed as part of the public API or only internally.
Based on my comprehensive verification, I can now provide the final assessment:
Update import path for Vitest 4 forward compatibility.
While
@vitest/browser/contextwill keep working until the next major version in Vitest 4, it is deprecated. The recommended path for Vitest 4+ isvitest/browser. Sincetypes.tsis not version-gated unlikevitest-context.tsandvitest-context-legacy.ts, update the import to usevitest/browserto ensure forward compatibility:import type { BrowserCommands, BrowserPage } from 'vitest/browser';This aligns with Vitest's migration guidance and maintains support for both Vitest 3 and 4 in the supported version range (^3.0.0 || ^4.0.0).
code/addons/vitest/src/vitest-plugin/viewports.ts (2)
5-7: Good: Streamlined imports.The change correctly narrows the import to only
ViewportMapand adds the localVitestBrowserContexttype, improving clarity and reducing unnecessary imports.
71-74: Correct: Version-aware context import with proper error handling.The switch to the internal vitest-context path enables runtime resolution based on Vitest version. The error handling and type cast are appropriate for this dynamic import pattern.
code/addons/vitest/build-config.ts (1)
25-34: Correct: Build entries for internal context modules.The new browser build entries properly expose the version-aware context modules as internal exports. The
dts: falsesetting is appropriate for these runtime-resolved shims.test-storybooks/portable-stories-kitchen-sink/react/package.json (1)
63-66: LGTM: Comprehensive dependency upgrades for Vitest 4 support.The coordinated upgrade of Storybook (to v10), Vitest (to v4), and related tooling is consistent and complete. The addition of
@testing-library/domand@vitest/browser-playwrightproperly supports Vitest 4's browser testing capabilities.Also applies to: 69-69, 77-82, 90-90, 92-93
code/addons/vitest/package.json (4)
49-50: Correct: Internal context exports added.The new export paths properly expose the version-aware context modules and align with the build configuration entries.
87-88: Good: DevDependencies upgraded for Vitest 4.The addition of
@vitest/browser-playwrightand upgrade of@vitest/runnerto v4 properly support Vitest 4's browser testing infrastructure.
109-112: Excellent: Backward-compatible peer dependency ranges.The
^3.0.0 || ^4.0.0ranges correctly enable users to continue using Vitest 3 while supporting migration to Vitest 4, aligning with the PR's compatibility goals.
114-117: Based on my investigation, the original review comment is accurate. The addon-vitest is designed to provide a better testing experience and projects using React, Vue, or Svelte built with Vite should consider migrating to it, but the change to make@vitest/browserandvitestrequired peer dependencies (by removing them frompeerDependenciesMeta) is indeed a breaking change that requires all users to have these packages installed.However, I found no documentation in the migration guide specifically addressing this breaking change for the vitest addon. While the change itself is valid (these are core dependencies for the addon to function), the lack of explicit documentation about this requirement is a concern.
Ensure this breaking change is documented in migration materials.
The removal of
@vitest/browserandvitestfrompeerDependenciesMetamakes them required peer dependencies (previously optional). While this may be necessary for the addon's functionality, update the Storybook 9 migration guide to explicitly document:
- That
@vitest/browserandvitest^3.0.0 || ^4.0.0 are now required- Migration steps for users who had these as optional
- Compatibility implications for existing projects
| const { server } = (await import( | ||
| // @ts-expect-error - This is an internal alias that will be resolved by the vitest plugin at runtime | ||
| '@storybook/addon-vitest/internal/vitest-context' | ||
| )) as unknown as VitestBrowserContext; | ||
|
|
||
| const { getInitialGlobals } = server.commands; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify runtime alias resolution and consider adding error handling.
The dynamic import relies on the vitest plugin to resolve the '@storybook/addon-vitest/internal/vitest-context' alias at runtime. If the alias is not properly configured or the imported module doesn't match the expected VitestBrowserContext shape, the type assertion will fail silently, leading to unclear runtime errors when accessing server.commands.
Run the following script to verify that the vitest plugin properly sets up this alias:
Consider adding runtime error handling to provide clearer error messages if the import fails:
let server: VitestBrowserContext['server'];
try {
const context = (await import(
// @ts-expect-error - This is an internal alias that will be resolved by the vitest plugin at runtime
'@storybook/addon-vitest/internal/vitest-context'
)) as unknown as VitestBrowserContext;
server = context.server;
} catch (error) {
throw new Error(
'Failed to load vitest context. Ensure the vitest plugin is properly configured.',
{ cause: error }
);
}
const { getInitialGlobals } = server.commands;🏁 Script executed:
#!/bin/bash
# Description: Verify the vitest-context alias is configured in the vitest plugin
# Find the vitest plugin configuration
echo "=== Searching for vitest-context alias configuration ==="
rg -n "vitest-context" --type=ts -C 3
echo ""
echo "=== Searching for alias resolution in vitest plugin ==="
rg -n "resolve.*alias" code/addons/vitest/src/vitest-plugin/ --type=ts -C 5Length of output: 3824
🏁 Script executed:
#!/bin/bash
# Verify vitest-context exports and error handling patterns
echo "=== Checking vitest-context.ts exports ==="
cat code/addons/vitest/src/vitest-plugin/vitest-context.ts
echo ""
echo "=== Checking vitest-context-legacy.ts exports ==="
cat code/addons/vitest/src/vitest-plugin/vitest-context-legacy.ts
echo ""
echo "=== Checking full context of test-utils.ts function ==="
cat code/addons/vitest/src/vitest-plugin/test-utils.tsLength of output: 3703
🏁 Script executed:
#!/bin/bash
# Verify the actual module exports and usage patterns
echo "=== Searching for other imports from vitest-context alias ==="
rg -n "@storybook/addon-vitest/internal/vitest-context" --type=ts -B2 -A2
echo ""
echo "=== Checking how @vitest/browser is imported elsewhere ==="
rg -n "from ['\"](vitest|@vitest)" code/addons/vitest/src/vitest-plugin/ --type=ts | head -20
echo ""
echo "=== Check if there's module augmentation in types.ts ==="
cat code/addons/vitest/src/vitest-plugin/types.ts | head -50Length of output: 5022
Add error handling and verify vitest-context.ts exports destructurable members for Vitest 4+ compatibility.
The dynamic import relies on @storybook/addon-vitest/internal/vitest-context exporting a server property, but vitest-context.ts (Vitest 4+ path) only exports a named function getVitestBrowserContext and does not re-export destructurable members like vitest-context-legacy.ts does. This will cause a runtime error when test-utils.ts attempts to destructure { server } on Vitest 4+.
The pattern shown in viewports.ts with error handling demonstrates the appropriate defensive approach:
const { server } = (await import(
'@storybook/addon-vitest/internal/vitest-context'
).catch(() => ({
server: { commands: {} }
}))) as unknown as VitestBrowserContext;
const { getInitialGlobals } = server.commands;Alternatively, ensure vitest-context.ts re-exports the required members like vitest-context-legacy.ts does by adding export * from 'vitest/browser';.
🤖 Prompt for AI Agents
In code/addons/vitest/src/vitest-plugin/test-utils.ts around lines 44-49 the
dynamic import assumes the module exports a destructurable `server` property
which breaks for Vitest 4+ (vitest-context.ts exports a named function only);
either add defensive error handling and a fallback object when importing (mirror
the pattern used in viewports.ts so the import catches failures and returns an
object with server: { commands: {} } to avoid destructure runtime errors) or
modify the Vitest 4+ module (vitest-context.ts) to re-export the expected
members (e.g., `export * from 'vitest/browser'` or explicitly re-export
`server`/commands) so the destructuring succeeds.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
code/addons/vitest/src/vitest-plugin/vitest-context.ts (1)
1-26: Unresolved: Replace version sniffing with feature detection.A previous review comment (see above in "Comments from reviews on previous commits") suggested replacing the current version-detection approach with feature detection using try/catch on dynamic imports. This recommendation remains valid and should be addressed:
Current issues:
- JSON import attributes (
with: { type: 'json' }) lack universal support across Node versions/bundlers- The
semverdependency adds unnecessary runtime overhead- Version checks create maintenance burden
Recommended approach:
let _ctxPromise: Promise<VitestServerContext> | undefined; export const getVitestBrowserContext = async (): Promise<VitestServerContext> => { if (_ctxPromise) return _ctxPromise; _ctxPromise = (async () => { try { return (await import('vitest/browser')) as unknown as VitestServerContext; } catch { return (await import('@vitest/browser/context')) as unknown as VitestServerContext; } })(); return _ctxPromise; };This eliminates the semver dependency, avoids JSON import attributes, and improves performance through caching.
🧹 Nitpick comments (1)
code/addons/vitest/src/node/vitest-manager.ts (1)
167-181: Type cast suggests missing type definitions; consider extracting common invalidation logic.The
anytype cast on line 170 indicates thatserver.moduleGraph.invalidateModulemay have type mismatches. This warrants investigation:
- Type safety: The cast bypasses type checking. If possible, update the type definitions or use a more specific type.
- Code duplication: The server invalidation logic (lines 168-171) is nearly identical to the vite invalidation logic (lines 172-175). Consider extracting a helper function.
Example refactor:
private updateLastChanged(filepath: string) { const invalidateModules = (moduleGraph: any, filepath: string) => { const mods = moduleGraph.getModulesByFile(filepath); mods?.forEach((mod: any) => moduleGraph.invalidateModule(mod)); }; this.vitest!.projects.forEach(({ browser, vite, server }) => { if (server) { invalidateModules(server.moduleGraph, filepath); } if (vite) { invalidateModules(vite.moduleGraph, filepath); } if (browser) { invalidateModules(browser.vite.moduleGraph, filepath); } }); }Alternatively, if the type cast is necessary due to Vitest API changes, add a comment explaining why.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/addons/vitest/src/node/reporter.ts(1 hunks)code/addons/vitest/src/node/vitest-manager.ts(2 hunks)code/addons/vitest/src/vitest-plugin/vitest-context-modern.ts(1 hunks)code/addons/vitest/src/vitest-plugin/vitest-context.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/src/node/reporter.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/vitest-plugin/vitest-context.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/addons/vitest/src/node/reporter.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/vitest-plugin/vitest-context.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/addons/vitest/src/node/reporter.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/vitest-plugin/vitest-context.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/node/reporter.tscode/addons/vitest/src/vitest-plugin/vitest-context-modern.tscode/addons/vitest/src/node/vitest-manager.tscode/addons/vitest/src/vitest-plugin/vitest-context.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to test-storybooks/** : Maintain test Storybook configurations under `test-storybooks/` for E2E and visual testing scenarios
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/vitest.workspace.ts : Keep and use the Vitest workspace configuration at code/vitest.workspace.ts
Applied to files:
code/addons/vitest/src/node/vitest-manager.ts
🧬 Code graph analysis (1)
code/addons/vitest/src/vitest-plugin/vitest-context.ts (1)
code/addons/vitest/src/vitest-plugin/types.ts (1)
VitestBrowserContext(13-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/addons/vitest/src/node/reporter.ts (1)
63-64: Good backward-compatible fix.The optional chaining correctly guards against
processTimeoutCausesbeing undefined in Vitest < 4, and the TODO comment makes the temporary nature of this workaround clear.code/addons/vitest/src/vitest-plugin/vitest-context-modern.ts (1)
1-3: LGTM - Clean compatibility shim.The re-export pattern with the documented TypeScript ignore is appropriate for handling the Vitest 3/4 compatibility layer during local development.
code/addons/vitest/src/node/vitest-manager.ts (1)
512-514: Good API simplification.Narrowing the signature to accept only
TestProjectand usingenv.__STORYBOOK_URL__as the detection criterion makes the logic clearer and more focused.
- Changed import statement for `page` to use the internal vitest context from `@storybook/addon-vitest/internal/vitest-context`. - Updated mock for `@vitest/browser/context` to reflect the new import path. These changes ensure compatibility with the latest Vitest plugin structure.
…figuration files.
yannbf
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
code/addons/vitest/package.json (1)
87-118: Unresolved: Playwright version pinning for E2E consistency.A previous review flagged (critical) that Playwright should be pinned to
1.52.0in devDependencies to match the codebase-wide pinning across test-storybooks and scripts packages. Although@vitest/browser-playwright@^4.0.1is present in devDependencies, without an explicit"playwright": "1.52.0"entry, transitive resolution may drift.To maintain E2E consistency across all packages, add
"playwright": "1.52.0"to devDependencies.Apply this diff to add explicit Playwright pinning:
"devDependencies": { "@types/istanbul-lib-report": "^3.0.3", "@types/micromatch": "^4.0.0", "@types/node": "^22.0.0", "@types/semver": "^7", "@vitest/browser-playwright": "^4.0.1", "@vitest/runner": "^4.0.1", "boxen": "^8.0.1", "empathic": "^2.0.0", "es-toolkit": "^1.36.0", "execa": "^8.0.1", "istanbul-lib-report": "^3.0.1", "micromatch": "^4.0.8", "pathe": "^1.1.2", + "playwright": "1.52.0", "picocolors": "^1.1.0", "react": "^18.2.0", "react-dom": "^18.2.0", "semver": "^7.6.3", "sirv": "^2.0.4", "slash": "^5.0.0", "tinyglobby": "^0.2.10", "tree-kill": "^1.2.2", "ts-dedent": "^2.2.0", "typescript": "^5.8.3", "vitest": "^3.2.4" }After updating, run your package manager (
yarn,npm, orpnpm) to update the lock file.
🧹 Nitpick comments (1)
code/addons/vitest/package.json (1)
49-50: Exports correctly configured for Vitest 3/4 compatibility support.Build configuration properly maps both
./internal/vitest-contextand./internal/vitest-context-legacyto their source files. Thevitest-context.tsimplementation uses runtime semver detection to conditionally importvitest/browser(v4) or@vitest/browser/context(v3), enabling seamless support for both versions.Note:
vitest-context-modern.tsis orphaned dead code (never imported or exported) and should be removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
code/addons/vitest/package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/addons/vitest/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/addons/vitest/package.json (1)
106-106: Verify rationale for downgrading vitest devDependency.The devDependency for vitest was downgraded from
^4.0.1to^3.2.4, while peerDependencies declare support for both^3.0.0 || ^4.0.0. This downgrade is likely intentional—testing against the latest Vitest 3 ensures backward compatibility—but clarify this design choice to confirm it aligns with the PR's Vitest 4 support objectives.
Closes #32803, #32804, #32806
What I did
Added Vitest 4 support for
@storybook/addon-vitestby:@storybook/addon-vitestChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
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 pull request has been released as version
0.0.0-pr-32600-sha-ad8d67f3. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-32600-sha-ad8d67f3valentin/vitest-4-supportad8d67f31761220969)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=32600Summary by CodeRabbit
Release Notes
New Features
Chores