Addon-Vitest: Normalize Windows paths in addon-vitest automigration#33340
Conversation
Fixes storybookjs#31768 When running `storybook add @storybook/addon-vitest` with Windows-style paths (e.g., `--config-dir .\apps\frontend\.storybook`), the backslashes were being inserted directly into the generated JavaScript files, causing invalid syntax. This fix uses the `slash` package to normalize all Windows paths to forward slashes before inserting them into templates, ensuring cross-platform compatibility. Changes: - Import and use `slash` package in `loadTemplate` function - Add tests to verify Windows path normalization - Add tests to verify Unix paths are preserved
|
LGTM! Thank you for your contribution! |
|
View your CI Pipeline Execution ↗ for commit e54d23a ☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughUpdate adds Windows path normalization to template replacements in the Vitest config loader and adjusts test expectations for the default workspace Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing touches
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/addons/vitest/src/updateVitestFile.test.ts (1)
1173-1200: LGTM! Tests adequately cover the path normalization behavior.The new test suite validates both Windows path normalization and Unix path preservation. The tests are clear and focused.
Note: These tests follow the existing pattern in this file of not mocking
fs.readFile, which means they read actual template files. While the coding guidelines recommend mocking all dependencies withspy: true, maintaining consistency with existing test patterns in this file is also valuable.As a future enhancement, consider refactoring all tests in this file to follow the strict mocking guidelines:
- Mock
node:fs/promiseswithspy: true- Implement mock behaviors in
beforeEachblocks- Use
vi.mocked()for type-safe mock accessThis would make tests more isolated and faster, though it would require mocking the template file contents.
As per coding guidelines on test mocking patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/addons/vitest/src/updateVitestFile.test.ts(1 hunks)code/addons/vitest/src/updateVitestFile.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/updateVitestFile.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/updateVitestFile.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/updateVitestFile.test.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/updateVitestFile.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/updateVitestFile.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
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}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/addons/vitest/src/updateVitestFile.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/addons/vitest/src/updateVitestFile.test.ts
🧠 Learnings (13)
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/addons/vitest/src/updateVitestFile.tscode/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/addons/vitest/src/updateVitestFile.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/addons/vitest/src/updateVitestFile.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/addons/vitest/src/updateVitestFile.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Test files should follow the naming pattern `*.test.ts`, `*.test.tsx`, `*.spec.ts`, or `*.spec.tsx`
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/addons/vitest/src/updateVitestFile.test.ts
🧬 Code graph analysis (1)
code/addons/vitest/src/updateVitestFile.test.ts (1)
code/addons/vitest/src/updateVitestFile.ts (1)
loadTemplate(10-20)
⏰ 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: normal
🔇 Additional comments (2)
code/addons/vitest/src/updateVitestFile.ts (2)
6-6: LGTM! Appropriate dependency for path normalization.The
slashpackage is the standard solution for normalizing Windows paths to forward slashes in JavaScript/TypeScript projects.
15-18: TypeScript type safety already protects against edge cases—code is safe to approve.The function parameter
replacements: Record<string, string>ensures that only string values reachslash(). Theslashpackage safely handles empty strings and returns them unchanged. Since the type system prevents null and undefined from entering this code path, no additional edge case handling is needed.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 11 | 17 | 🚨 +6 🚨 |
| Self size | 123 KB | 125 KB | 🚨 +2 KB 🚨 |
| Dependency size | 1.30 MB | 2.00 MB | 🚨 +701 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 192 | 🚨 +4 🚨 |
| Self size | 75 KB | 75 KB | 🚨 +71 B 🚨 |
| Dependency size | 32.05 MB | 32.26 MB | 🚨 +215 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 49 | 0 |
| Self size | 20.40 MB | 20.32 MB | 🎉 -77 KB 🎉 |
| Dependency size | 16.52 MB | 16.52 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 192 | 🚨 +4 🚨 |
| Self size | 139 KB | 139 KB | 0 B |
| Dependency size | 30.27 MB | 30.48 MB | 🚨 +215 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 192 | 196 | 🚨 +4 🚨 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 28.77 MB | 28.98 MB | 🚨 +215 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 14 | 20 | 🚨 +6 🚨 |
| Self size | 22 KB | 22 KB | 🚨 +24 B 🚨 |
| Dependency size | 1.46 MB | 2.16 MB | 🚨 +703 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 538 | 🚨 +4 🚨 |
| Self size | 646 KB | 646 KB | 0 B |
| Dependency size | 59.54 MB | 59.75 MB | 🚨 +215 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 123 | 127 | 🚨 +4 🚨 |
| Self size | 1.12 MB | 1.12 MB | 🎉 -36 B 🎉 |
| Dependency size | 22.11 MB | 22.33 MB | 🚨 +216 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 14 | 20 | 🚨 +6 🚨 |
| Self size | 13 KB | 13 KB | 🚨 +12 B 🚨 |
| Dependency size | 1.44 MB | 2.15 MB | 🚨 +703 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 155 | 159 | 🚨 +4 🚨 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 23.41 MB | 23.62 MB | 🚨 +216 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 113 | 117 | 🚨 +4 🚨 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 19.91 MB | 20.12 MB | 🚨 +216 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 274 | 278 | 🚨 +4 🚨 |
| Self size | 24 KB | 24 KB | 0 B |
| Dependency size | 44.44 MB | 44.65 MB | 🚨 +215 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 200 | 204 | 🚨 +4 🚨 |
| Self size | 16 KB | 16 KB | 0 B |
| Dependency size | 33.30 MB | 33.52 MB | 🚨 +215 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 19 | 24 | 🚨 +5 🚨 |
| Self size | 55 KB | 55 KB | 0 B |
| Dependency size | 26.58 MB | 26.82 MB | 🚨 +243 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 25 | 🚨 +5 🚨 |
| Self size | 56 KB | 56 KB | 0 B |
| Dependency size | 26.63 MB | 26.88 MB | 🚨 +243 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 108 | 114 | 🚨 +6 🚨 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 43.67 MB | 43.96 MB | 🚨 +293 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 15 | 21 | 🚨 +6 🚨 |
| Self size | 19 KB | 19 KB | 0 B |
| Dependency size | 1.50 MB | 2.21 MB | 🚨 +703 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 776 KB | 775 KB | 🎉 -914 B 🎉 |
| Dependency size | 67.57 MB | 67.49 MB | 🎉 -77 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -9 B 🎉 |
| Dependency size | 66.14 MB | 66.06 MB | 🎉 -77 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 1000 KB | 1000 KB | 🎉 -189 B 🎉 |
| Dependency size | 36.92 MB | 36.84 MB | 🎉 -77 KB 🎉 |
| Bundle Size Analyzer | node | node |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@code/addons/vitest/src/updateVitestFile.ts`:
- Around line 14-17: The forEach callback currently uses an implicit return from
the assignment (Object.entries(replacements).forEach(([key, value]) => (template
= template.replace(key, normalize(value))))), which static analysis flags;
change it so the callback does not return a value by using a block body and
explicit statement or replace the forEach with a for...of loop iterating over
Object.entries(replacements) — in either case update the code that references
template, replacements, normalize and Object.entries to assign template =
template.replace(key, normalize(value)) as a standalone statement (no expression
returned).
| // Normalize Windows paths (backslashes) to forward slashes for JavaScript string compatibility | ||
| Object.entries(replacements).forEach( | ||
| ([key, value]) => (template = template.replace(key, normalize(value))) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix the forEach callback to not return a value.
The arrow function implicitly returns the result of the assignment expression, but forEach ignores return values. This is flagged by static analysis and can be confusing to readers.
🔧 Proposed fix
// Normalize Windows paths (backslashes) to forward slashes for JavaScript string compatibility
- Object.entries(replacements).forEach(
- ([key, value]) => (template = template.replace(key, normalize(value)))
- );
+ Object.entries(replacements).forEach(([key, value]) => {
+ template = template.replace(key, normalize(value));
+ });Alternatively, a for...of loop would be slightly more idiomatic here:
for (const [key, value] of Object.entries(replacements)) {
template = template.replace(key, normalize(value));
}🧰 Tools
🪛 Biome (2.3.13)
[error] 15-15: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@code/addons/vitest/src/updateVitestFile.ts` around lines 14 - 17, The forEach
callback currently uses an implicit return from the assignment
(Object.entries(replacements).forEach(([key, value]) => (template =
template.replace(key, normalize(value))))), which static analysis flags; change
it so the callback does not return a value by using a block body and explicit
statement or replace the forEach with a for...of loop iterating over
Object.entries(replacements) — in either case update the code that references
template, replacements, normalize and Object.entries to assign template =
template.replace(key, normalize(value)) as a standalone statement (no expression
returned).
…est-31768 Addon-Vitest: Normalize Windows paths in addon-vitest automigration (cherry picked from commit 3802b16)
Summary
Fixes #31768
When running
storybook add @storybook/addon-vitestwith Windows-style paths (e.g.,--config-dir .\apps\frontend\.storybook), the backslashes were being inserted directly into the generated JavaScript template files, causing invalid syntax.This PR normalizes all Windows paths to forward slashes before inserting them into templates using the existing
slashdependency.Changes
slashpackage inupdateVitestFile.tsloadTemplatefunctionTest Results
All tests pass ✅
Before
After
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.