-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Addon-Vitest: Support Vitest 4 #32819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/tasks/sandbox.ts (1)
10-10: Usestorybook/internal/node-loggerinstead of console.As per coding guidelines for Node.js scripts, replace the direct console usage with the appropriate logger from
storybook/internal/node-logger.Apply this diff to use the proper logger:
+import { logger as nodeLogger } from '@storybook/core/node-logger'; + -const logger = console; +const logger = nodeLogger;As per coding guidelines
code/addons/vitest/package.json (1)
100-105: Duplicate dependency: ts-dedent appears in both dependencies and devDependenciesRemove the duplicate from devDependencies to avoid confusion and lockfile churn.
"devDependencies": { @@ - "ts-dedent": "^2.2.0", "typescript": "^5.8.3", "vitest": "^4.0.1" },code/addons/vitest/src/postinstall.ts (1)
303-321: Honor --skip-install for dependency installationoptions.skipInstall currently only skips Playwright binary install, not the package install step. Respect it to avoid surprising users/CI.
- await packageManager.installDependencies(); + if (!options.skipInstall) { + await packageManager.installDependencies(); + } else { + logger.plain('Skipping dependency installation (--skip-install).'); + logger.plain('Please run your package manager install before running tests.'); + }
🧹 Nitpick comments (9)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsx (1)
66-66: Inconsistent quote style.Double quotes are used here while single quotes are used elsewhere (lines 57, 75). Prettier should normalize this when run before commit.
- children: "foo", + children: 'foo',test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.ts (1)
1-3: Consider inlining this function unless needed for testing module resolution.The
getButtonStringfunction is trivial and could be inlined directly inAddonTest.tsx. However, if this separate module is intentionally created to test module imports/exports in the Vitest setup, then the current structure is appropriate.test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.js (1)
6-10: Consider a more robust approach than modifying source files.Modifying source files during test execution is problematic because:
- It can lead to race conditions if tests run in parallel
- It creates side effects that could affect subsequent runs
- No error handling for file operations (read/write failures)
- Path resolution depends on the current working directory
Consider these alternatives:
- Use environment variables to control the
forceFailurebehavior- Use Storybook parameters or test configuration
- Create separate story files for E2E vs. unit tests
- At minimum, add error handling and verification:
const testStoryPath = path.resolve("stories/AddonTest.stories.tsx"); console.log(`Pre-e2e script: clearing ${testStoryPath}`); -const storyContent = fs.readFileSync(testStoryPath).toString(); -fs.writeFileSync(testStoryPath, storyContent.replace("forceFailure: true", "forceFailure: false")); +try { + const storyContent = fs.readFileSync(testStoryPath, 'utf8'); + const updatedContent = storyContent.replace("forceFailure: true", "forceFailure: false"); + + if (storyContent === updatedContent) { + console.warn("Warning: No replacement made - 'forceFailure: true' not found"); + } + + fs.writeFileSync(testStoryPath, updatedContent, 'utf8'); + console.log("Successfully updated forceFailure to false"); +} catch (error) { + console.error(`Failed to update ${testStoryPath}:`, error); + process.exit(1); +}.circleci/README.md (1)
56-56: Clarify the purpose of the Vitest 3 test in documentation.The PR aims to add Vitest 4 support, but this job is specifically for Vitest 3. Consider updating the description to clarify this tests backward compatibility with Vitest 3.2, making the relationship to the overall Vitest 4 upgrade clearer.
-- **`e2e-ui-vitest-3`**: End-to-end tests for Storybook's manager UI using Vitest 3 +- **`e2e-ui-vitest-3`**: End-to-end tests for Storybook's manager UI using Vitest 3 (backward compatibility)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/main.ts (1)
25-28: Consider simplifying redundant optimizeDeps configuration.The
optimizeDepssection spreads the existing config and then recreates theincludearray without adding any new values. This is redundant and can be simplified.If you're not adding any new dependencies, you can simplify to:
return { ...config, - optimizeDeps: { - ...config.optimizeDeps, - include: [...(config.optimizeDeps?.include || [])], - }, resolve: {Or keep just the spread if preserving the structure is desired for future additions:
optimizeDeps: { ...config.optimizeDeps, - include: [...(config.optimizeDeps?.include || [])], },test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.ts (1)
10-121: Optional: Consider extracting shared test logic.The desktop and mobile tests share significant logic for verifying composed Storybook visibility, filtering, and story selection. Consider extracting a shared helper function to reduce duplication and improve maintainability.
However, this is a low-priority suggestion—test clarity and readability often outweigh DRY principles in e2e tests.
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts (2)
198-205: DRY: factor “ensure story loaded” into a helperThe repeated pattern to locate and wait for the same story element can be a small helper to reduce duplication and keep consistent timeouts.
async function waitForStoryButton(sbPage: SbPage, name: string) { const el = sbPage.getCanvasBodyElement().getByRole('button', { name }); await expect(el).toBeVisible({ timeout: 30_000 }); return el; }Use as: await waitForStoryButton(sbPage, 'test');
Also applies to: 273-278, 440-447, 502-506, 592-596, 642-646, 695-699
56-58: Now let me examine the actual code context to understand what's being tested:Based on my verification, I've confirmed the issues identified in your review comment. Fixed timeouts using
page.waitForTimeout()can lead to flaky tests, unnecessary delays when conditions are met sooner, and fragile tests that don't adapt to actual application state. Thepage.waitForTimeout()method is discouraged in recent versions of Playwright.The script confirmed 6 instances of fixed waits across the file (lines 282, 336, 375, 415, 451, 703). The
page.waitForFunction()approach is recommended as it's versatile and allows passing a custom function to wait for specific conditions.
Replace fixed waits with deterministic conditions for HMR and page reload detection
Fixed
page.waitForTimeout()calls cause flaky and slow tests. Replace with explicit wait conditions. For HMR/reload scenarios, usepage.waitForFunction()to poll for a measurable state change (e.g., element text updates, visibility changes, or navigation completion) rather than arbitrary delays.Example pattern:
// after modifyFile(...) await page.waitForFunction( () => !!document.querySelector('#testing-module-description:has-text("Ran ")'), { timeout: 30000 } );Affected lines: 282, 336, 375, 415, 451, 703
code/addons/vitest/src/postinstall.ts (1)
273-276: Avoid re-adding @storybook/nextjs-vite if already presentPrevents overriding existing version ranges unnecessarily.
- const storybookVersion = await packageManager.getInstalledVersion('storybook'); - uniqueDependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); + const storybookVersion = await packageManager.getInstalledVersion('storybook'); + if (!allDeps['@storybook/nextjs-vite']) { + uniqueDependencies.push(`@storybook/nextjs-vite@^${storybookVersion}`); + }
📜 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 (58)
.circleci/README.md(1 hunks).circleci/config.yml(1 hunks).circleci/src/jobs/e2e-ui-vitest-3.yml(1 hunks).circleci/src/jobs/stories-tests.yml(1 hunks).circleci/src/workflows/daily.yml(2 hunks).circleci/src/workflows/merged.yml(3 hunks).circleci/src/workflows/normal.yml(3 hunks)code/addons/vitest/package.json(2 hunks)code/addons/vitest/src/node/reporter.ts(1 hunks)code/addons/vitest/src/node/vitest-manager.ts(2 hunks)code/addons/vitest/src/postinstall.ts(7 hunks)code/addons/vitest/src/updateVitestFile.test.ts(11 hunks)code/addons/vitest/src/vitest-plugin/index.ts(1 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/addons/vitest/tsconfig.json(1 hunks)code/core/src/actions/loaders.ts(1 hunks)code/core/src/test/preview.ts(1 hunks)code/frameworks/nextjs-vite/package.json(1 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(4 hunks)code/renderers/react/template/stories/test-fn.stories.tsx(1 hunks)scripts/tasks/sandbox.ts(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(3 hunks)test-storybooks/portable-stories-kitchen-sink/react/vite.config.mts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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-vitest-3/tsconfig.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jscode/frameworks/nextjs-vite/package.jsoncode/addons/vitest/templates/vitest.config.3.2.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vite.config.tstest-storybooks/portable-stories-kitchen-sink/react/package.jsoncode/addons/vitest/src/node/reporter.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsxcode/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/templates/vitest.config.4.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxcode/addons/vitest/src/vitest-plugin/index.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsxcode/addons/vitest/src/node/vitest-manager.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tsscripts/tasks/sandbox.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxcode/lib/cli-storybook/src/sandbox-templates.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.node.jsoncode/core/src/test/preview.tscode/addons/vitest/tsconfig.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.jsoncode/renderers/react/template/stories/test-fn.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tscode/core/src/actions/loaders.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/src/postinstall.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.tscode/addons/vitest/templates/vitest.config.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsxcode/addons/vitest/src/vitest-plugin/test-utils.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.tscode/addons/vitest/src/updateVitestFile.test.tscode/addons/vitest/package.json
**/tsconfig*.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep TypeScript strict mode enabled in tsconfig (e.g.,
"strict": true)
Files:
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.node.jsoncode/addons/vitest/tsconfig.json
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-vitest-3/tsconfig.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vite.config.tstest-storybooks/portable-stories-kitchen-sink/react/package.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react/vite.config.mtstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/button.csstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/tsconfig.node.jsontest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.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/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.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:
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jscode/addons/vitest/templates/vitest.config.3.2.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vite.config.tscode/addons/vitest/src/node/reporter.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsxcode/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/templates/vitest.config.4.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.tsxcode/addons/vitest/src/vitest-plugin/index.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsxcode/addons/vitest/src/node/vitest-manager.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/OtherComponent.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tsscripts/tasks/sandbox.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxcode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/test/preview.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/Button.stories.tsxcode/renderers/react/template/stories/test-fn.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tscode/core/src/actions/loaders.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/src/postinstall.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/get-button-string.tscode/addons/vitest/templates/vitest.config.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/MyButton.stories.tsxcode/addons/vitest/src/vitest-plugin/test-utils.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.tscode/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/templates/vitest.config.3.2.template.tscode/addons/vitest/src/node/reporter.tscode/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/templates/vitest.config.4.template.tscode/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/src/node/vitest-manager.tscode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/test/preview.tscode/renderers/react/template/stories/test-fn.stories.tsxcode/core/src/actions/loaders.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/src/postinstall.tscode/addons/vitest/templates/vitest.config.template.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/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/templates/vitest.config.3.2.template.tscode/addons/vitest/src/node/reporter.tscode/addons/vitest/templates/vitest.workspace.template.tscode/addons/vitest/templates/vitest.config.4.template.tscode/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/src/node/vitest-manager.tsscripts/tasks/sandbox.tscode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/test/preview.tscode/renderers/react/template/stories/test-fn.stories.tsxcode/core/src/actions/loaders.tscode/addons/vitest/src/vitest-plugin/viewports.tscode/addons/vitest/src/postinstall.tscode/addons/vitest/templates/vitest.config.template.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/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:
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tscode/addons/vitest/src/updateVitestFile.test.ts
scripts/**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In Node.js scripts, use
storybook/internal/node-loggerinstead ofconsole.*
Files:
scripts/tasks/sandbox.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
🧠 Learnings (12)
📓 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-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:
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.jscode/addons/vitest/templates/vitest.config.3.2.template.ts.circleci/README.md.circleci/src/workflows/daily.ymlcode/addons/vitest/templates/vitest.workspace.template.tstest-storybooks/portable-stories-kitchen-sink/react/vite.config.mtscode/addons/vitest/templates/vitest.config.4.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/main.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/save-from-controls.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/composition.spec.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tscode/addons/vitest/templates/vitest.config.template.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.tscode/addons/vitest/src/updateVitestFile.test.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/get-decorator-string.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/.storybook/preview.tsx
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
PR: storybookjs/storybook#32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/.yarnrc.yml
📚 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.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.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-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/actions/loaders.ts
🧬 Code graph analysis (16)
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/vite.config.mts (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-448)
code/addons/vitest/templates/vitest.config.4.template.ts (2)
scripts/ecosystem-ci/before-test.js (1)
__dirname(15-15)code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-448)
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/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/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/playwright-e2e.config.ts (1)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.js (1)
path(4-4)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.stories.tsx (3)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/AddonTest.tsx (1)
Component(3-5)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/.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/e2e-tests/composition.spec.ts (1)
code/e2e-tests/util.ts (1)
SbPage(8-255)
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)
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)
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/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/vitest.workspace.ts (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
storybookTest(100-448)
| const isVitest3_2To4 = vitestVersionSpecifier | ||
| ? satisfies(vitestVersionSpecifier, '>=3.2.0 <4.0.0') | ||
| : false; | ||
| const isVitest4OrNewer = vitestVersionSpecifier | ||
| ? satisfies(vitestVersionSpecifier, '>=4.0.0') | ||
| : true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: semver.satisfies used on a range; use the coerced version
satisfies() expects a version, not a range like ^3.2.4. Current logic can mis-detect v3 vs v4 and pick wrong templates/deps.
- const isVitest3_2To4 = vitestVersionSpecifier
- ? satisfies(vitestVersionSpecifier, '>=3.2.0 <4.0.0')
- : false;
- const isVitest4OrNewer = vitestVersionSpecifier
- ? satisfies(vitestVersionSpecifier, '>=4.0.0')
- : true;
+ const vitestVersion = coercedVitestVersion?.version ?? null;
+ const isVitest3_2To4 = vitestVersion ? satisfies(vitestVersion, '>=3.2.0 <4.0.0') : false;
+ const isVitest4OrNewer = vitestVersion ? satisfies(vitestVersion, '>=4.0.0') : true;Committable suggestion skipped: line range outside the PR's diff.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Storybook logger instead of console.error
Follow repo logging guidelines for server code.
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: 329-329
🤖 Prompt for AI Agents
In code/addons/vitest/src/postinstall.ts around lines 276 and 329, replace the
direct console.error calls with the Storybook server logger per repo guidelines
(e.g. import the appropriate logger from Storybook's node logger package and
call logger.error(...)) so logs go through the centralized logger; ensure you
add the import at the top of the file, use logger.error with the same message
text, and preserve any existing control flow (exit/return) after logging.
| ...(options.configDir !== '.storybook' ? ['--config-dir', `"${options.configDir}"`] : []), | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quoted argument breaks execa arg parsing
Passing quotes in args yields literal quotes. Let execa handle spaces.
- ...(options.configDir !== '.storybook' ? ['--config-dir', `"${options.configDir}"`] : []),
+ ...(options.configDir !== '.storybook' ? ['--config-dir', options.configDir] : []),📝 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.
| ...(options.configDir !== '.storybook' ? ['--config-dir', `"${options.configDir}"`] : []), | |
| ], | |
| ...(options.configDir !== '.storybook' ? ['--config-dir', options.configDir] : []), | |
| ], |
🤖 Prompt for AI Agents
In code/addons/vitest/src/postinstall.ts around lines 523 to 524, the config-dir
arg is being passed with literal quotes ("${options.configDir}") which breaks
execa arg parsing; remove the surrounding quotes and pass the directory string
directly (i.e. use options.configDir) so execa can handle any spaces correctly,
keeping the existing ternary conditional and argument array structure.
| if ((await testStoryElement.getAttribute("aria-expanded")) !== "true") { | ||
| testStoryElement.click(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing await on Playwright click() can cause race/flakes
The click promise isn’t awaited; subsequent assertions may run before the expansion.
- testStoryElement.click();
+ 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 Playwright click() call is not awaited which can
cause race conditions; update the code to await testStoryElement.click() so the
test waits for the click action to complete before proceeding (optionally follow
with an assertion or a waitFor to confirm aria-expanded becomes "true").
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential test isolation issue with module-level mock.
The mockFn is defined at module scope and shared across test runs, which can cause state pollution. Mock state from one test run could leak into subsequent runs, especially with Vitest's module caching.
Consider moving the mock creation into the loader to ensure test isolation:
-const mockFn = fn();
-export const WithLoader: CSF3Story<{ mockFn: (val: string) => string }> = {
- args: {
- mockFn,
- },
+export const WithLoader: CSF3Story<{ mockFn: (val: string) => string }> = {
loaders: [
async () => {
+ const mockFn = fn();
mockFn.mockReturnValueOnce('mockFn return value');
return {
+ mockFn,
value: 'loaded data',
};
},
],
render: (args, { loaded }) => {
- const data = args.mockFn('render');
+ const data = loaded.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');
+ expect(loaded.mockFn).toHaveBeenCalledWith('render');
},
};Note: You'll need to adjust the play function to accept the context parameter to access loaded.
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-113, the module-level mockFn causes test-state leakage; move
mock creation into the loader (create a fresh vi.fn() inside the loader, set its
mockReturnValueOnce, and return it as part of the loader result), remove the
module-scoped mock from args, update render to read and call the mock from the
loaded object (e.g., use loaded.mockFn(...) instead of args.mockFn), and change
play to accept the context parameter (e.g., async (_args, { loaded }) => {
expect(loaded.mockFn).toHaveBeenCalledWith('render'); }) so each run gets an
isolated mock.
| async function unhandledRejection() { | ||
| throwError('I THREW AN UNHANDLED REJECTION!'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unhandledRejection function doesn't create an actual unhandled promise rejection.
The function is declared async but throws synchronously without returning a rejected promise. To create an actual unhandled rejection, use Promise.reject():
Apply this diff to create a real unhandled rejection:
-async function unhandledRejection() {
- throwError('I THREW AN UNHANDLED REJECTION!');
+function unhandledRejection() {
+ Promise.reject(new Error('I THREW AN UNHANDLED REJECTION!'));
}📝 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.
| async function unhandledRejection() { | |
| throwError('I THREW AN UNHANDLED REJECTION!'); | |
| } | |
| function unhandledRejection() { | |
| Promise.reject(new Error('I THREW AN UNHANDLED REJECTION!')); | |
| } |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsx
around lines 3 to 5, the async function unhandledRejection currently throws
synchronously and does not produce a real unhandled promise rejection; change it
to return a rejected promise (e.g. return Promise.reject(new Error('I THREW AN
UNHANDLED REJECTION!')) or otherwise call Promise.reject) so the rejection is
truly unhandled by the runtime.
| const meta = { | ||
| title: 'Example/UnhandledErrors', | ||
| args: { | ||
| errorType: null, | ||
| forceFailure: false, | ||
| }, | ||
| component: ({ errorType, forceFailure }) => { | ||
| if (forceFailure) { | ||
| if (errorType === 'rejection') { | ||
| setTimeout(unhandledRejection, 0); | ||
| } else if (errorType === 'error') { | ||
| setTimeout(unhandledError, 0); | ||
| } | ||
| } | ||
| return 'Hello world'; | ||
| }, | ||
| } as Meta<{ errorType: 'rejection' | 'error' | null; forceFailure?: boolean }>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component requires forceFailure: true to throw errors.
The component only triggers errors when forceFailure is true (line 21), but the default is false (line 18). The UnhandledError and UnhandledRejection stories below don't set forceFailure: true, so they won't actually throw errors despite their names suggesting they should.
If these stories are meant to throw errors by default, update the default args:
const meta = {
title: 'Example/UnhandledErrors',
args: {
errorType: null,
- forceFailure: false,
+ forceFailure: true,
},Or set forceFailure in each story that should throw (see comments on lines 34-44).
📝 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: 'Example/UnhandledErrors', | |
| args: { | |
| errorType: null, | |
| forceFailure: false, | |
| }, | |
| component: ({ errorType, forceFailure }) => { | |
| if (forceFailure) { | |
| if (errorType === 'rejection') { | |
| setTimeout(unhandledRejection, 0); | |
| } else if (errorType === 'error') { | |
| setTimeout(unhandledError, 0); | |
| } | |
| } | |
| return 'Hello world'; | |
| }, | |
| } as Meta<{ errorType: 'rejection' | 'error' | null; forceFailure?: boolean }>; | |
| const meta = { | |
| title: 'Example/UnhandledErrors', | |
| args: { | |
| errorType: null, | |
| forceFailure: true, | |
| }, | |
| component: ({ errorType, forceFailure }) => { | |
| if (forceFailure) { | |
| if (errorType === 'rejection') { | |
| setTimeout(unhandledRejection, 0); | |
| } else if (errorType === 'error') { | |
| setTimeout(unhandledError, 0); | |
| } | |
| } | |
| return 'Hello world'; | |
| }, | |
| } as Meta<{ errorType: 'rejection' | 'error' | null; forceFailure?: boolean }>; |
🤖 Prompt for AI Agents
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsx
around lines 14-30: the Meta default args set forceFailure: false so the
component will never trigger the unhandled error/rejection in the example
stories; either change the default in meta.args to forceFailure: true so stories
throw by default, or leave the default false and explicitly set forceFailure:
true on each UnhandledError/UnhandledRejection story that should produce an
error.
| export const UnhandledError: Story = { | ||
| args: { | ||
| errorType: 'error', | ||
| }, | ||
| }; | ||
|
|
||
| export const UnhandledRejection: Story = { | ||
| args: { | ||
| errorType: 'rejection', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stories need forceFailure: true to actually throw errors.
Based on the component logic (lines 21-27), these stories won't throw any errors because forceFailure defaults to false and is not overridden here.
Apply this diff to make the stories throw errors as their names suggest:
export const UnhandledError: Story = {
args: {
errorType: 'error',
+ forceFailure: true,
},
};
export const UnhandledRejection: Story = {
args: {
errorType: 'rejection',
+ forceFailure: true,
},
};📝 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.
| export const UnhandledError: Story = { | |
| args: { | |
| errorType: 'error', | |
| }, | |
| }; | |
| export const UnhandledRejection: Story = { | |
| args: { | |
| errorType: 'rejection', | |
| }, | |
| }; | |
| export const UnhandledError: Story = { | |
| args: { | |
| errorType: 'error', | |
| forceFailure: true, | |
| }, | |
| }; | |
| export const UnhandledRejection: Story = { | |
| args: { | |
| errorType: 'rejection', | |
| forceFailure: true, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/stories/UnhandledErrors.stories.tsx
around lines 34 to 44, both UnhandledError and UnhandledRejection stories
currently set only errorType and so will not trigger the component’s error path
(forceFailure defaults to false); update each story's args to include
forceFailure: true so the component actually throws as intended (i.e., add
forceFailure: true alongside errorType for both exports).
| test: { | ||
| name: "storybook", | ||
| pool: "threads", | ||
| deps: { | ||
| optimizer: { | ||
| web: { | ||
| enabled: false, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify pool configuration with browser mode.
Setting pool: "threads" (line 22) when browser.enabled: true may be redundant or conflict with the browser provider's own pool management. Vitest browser mode typically manages its own execution context.
Additionally, deps.optimizer.web.enabled: false (lines 24-28) disables web dependency optimization—consider documenting why this is necessary for this test configuration.
🏁 Script executed:
#!/bin/bash
# Description: Check Vitest documentation and examples for browser mode pool configuration patterns
# Search for pool configuration in Vitest-related files
rg -n -C3 'pool.*threads' --type=ts -g '*vitest*.ts' -g '*vitest*.mts'
# Search for browser mode configurations to see common patterns
rg -n -C3 'browser.*enabled.*true' --type=ts -g '*vitest*.ts' -g '*vitest*.mts'Length of output: 2092
🌐 Web query:
Vitest browser mode pool configuration best practices
💡 Result:
Here are concise best practices for configuring Vitest’s browser mode and test pool (with docs cited):
- Prefer a real provider (Playwright or WebdriverIO) over the default preview; Playwright is recommended for CI and parallel runs. [1][2]
- Use browser.instances (rather than the deprecated browser.name) to declare browsers and setup files—this shares one Vite server across instances and improves caching. [2][3]
- Route only browser-specific tests to browser pools via poolMatchGlobs (e.g., ['/tests/browser/','browser']) so Node and browser tests use appropriate pools. [4]
- Control parallelism with pool / poolOptions / maxWorkers: keep heavy browser/fixture tests more isolated (fewer workers) and use threads/forks for fast Node tests. Tune maxWorkers based on CI/runner CPU and browser process memory. [5]
- For multiple browsers, reuse instances + setup files rather than separate Vitest projects to avoid duplicate transforms and speed up runs. [3]
- Run browsers headless in CI (browser.headless default = process.env.CI) and only enable UI locally. [1][2]
- Use browser.isolate (default true) or per-test iframe isolation when tests mutate global state; disable for performance when safe. [2]
- Set browser.api port if you need to avoid conflicts with dev servers (default 63315). [1]
- When debugging, disable file parallelism or run a single worker to allow breakpoints/inspector. [5]
- Start with conservative worker counts and increase until CPU/memory or flaky browser failures appear; rely on CI machine specs and test runtime profiling to choose final values. [5]
Sources: Vitest Browser guide & config, Browser config reference, Parallelism guide, Multiple Setups docs.
[1] Vitest Browser guide (main).
[2] Vitest Browser config reference / instances.
[3] Vitest Multiple Setups (browser.instances).
[4] poolMatchGlobs / config reference.
[5] Parallelism guide / poolOptions.
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the actual vite.config.mts file mentioned in the review
cat test-storybooks/portable-stories-kitchen-sink/react/vite.config.mts
# Also check if there's a vitest config or workspace file in the same directory
fd -e "vitest\.*" -e "workspace\.*" test-storybooks/portable-stories-kitchen-sink/react/
Length of output: 1424
Remove or replace pool: "threads" when using browser mode with Playwright.
Setting pool: "threads" globally (line 22) while browser.enabled: true (line 32) with Playwright creates a configuration conflict. When using browser mode, control parallelism via poolMatchGlobs to route tests to appropriate pools—don't set conflicting global pool configurations. Since this appears to be browser-only testing, either remove the pool line or change it to align with browser mode requirements.
Additionally, document why deps.optimizer.web.enabled: false (lines 24-28) is necessary—clarify if this is for Storybook stability or to work around optimization issues.
🤖 Prompt for AI Agents
In test-storybooks/portable-stories-kitchen-sink/react/vite.config.mts around
lines 20 to 29, remove or replace the global pool: "threads" setting because it
conflicts with browser mode (Playwright) — either delete the pool line or change
it to a browser-compatible approach and instead control parallelism with
poolMatchGlobs so tests are routed to the correct pools; also add a short
comment above deps.optimizer.web.enabled = false explaining why disabling the
web optimizer is required (e.g., Storybook stability or specific optimization
issues) so future maintainers understand the rationale.
| setupFiles: ["./.storybook/vitest.setup.ts"], | ||
| environment: "jsdom", | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant or conflicting environment: "jsdom" setting.
Setting environment: "jsdom" (line 41) when browser.enabled: true is likely incorrect. In Vitest browser mode, tests run in an actual browser environment (Chromium in this case), not in the Node.js-based jsdom simulation. The jsdom environment is incompatible with browser mode.
This configuration may be ignored by Vitest, cause unexpected behavior, or conflict with browser mode's environment.
Apply this diff to remove the redundant setting:
setupFiles: ["./.storybook/vitest.setup.ts"],
- environment: "jsdom",
},
});Vitest browser mode will automatically use the browser environment when browser.enabled: true.
📝 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.
| setupFiles: ["./.storybook/vitest.setup.ts"], | |
| environment: "jsdom", | |
| }, | |
| setupFiles: ["./.storybook/vitest.setup.ts"], | |
| }, |
🤖 Prompt for AI Agents
In test-storybooks/portable-stories-kitchen-sink/react/vite.config.mts around
lines 40 to 42, the config sets environment: "jsdom" while using
browser.enabled: true which is redundant/conflicting; remove the environment:
"jsdom" line so Vitest uses the actual browser environment provided by
browser.enabled: true and ensure no other parts of the config reintroduce a
jsdom override.
…figuration files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.circleci/config.yml(1 hunks).circleci/src/workflows/daily.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .circleci/src/workflows/daily.yml
⏰ 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)
.circleci/config.yml (2)
1030-1032: New jobs properly integrated into all workflows.The
e2e-ui-vitest-3andstories-testsjobs are consistently added to the daily, merged, and normal workflows with correct dependencies onbuild. This aligns with the PR objective to support Vitest 4 testing. Integration looks correct; no issues detected.Also applies to: 1076-1078, 1152-1154, 1125-1127, 1191-1193
446-470: ****The
react-vitest-3test storybook is properly scaffolded and configured. The directory exists attest-storybooks/portable-stories-kitchen-sink/react-vitest-3/with a completepackage.jsonthat includes the requiredplaywright-e2escript configured to runplaywright test -c playwright-e2e.config.ts. Playwright is pinned to version 1.52.0, matching the CircleCI job configuration. The CircleCI job setup is correct and follows the established pattern for other E2E test jobs.
| stories-tests: | ||
| executor: | ||
| class: xlarge | ||
| name: sb_playwright | ||
| parallelism: 2 | ||
| steps: | ||
| - git-shallow-clone/checkout_advanced: | ||
| clone_options: --depth 1 --verbose | ||
| - attach_workspace: | ||
| at: . | ||
| - run: | ||
| command: | | ||
| cd code | ||
| TEST_FILES=$(circleci tests glob "**/*.{stories}.{ts,tsx,js,jsx,cjs}" | sed "/^e2e-tests\//d" | sed "/^node_modules\//d") | ||
| echo "$TEST_FILES" | circleci tests run --command="xargs yarn test --reporter=junit --reporter=default --outputFile=../test-results/junit-${CIRCLE_NODE_INDEX}.xml" --verbose | ||
| name: Run tests | ||
| - store_test_results: | ||
| path: test-results | ||
| - report-workflow-on-failure | ||
| - cancel-workflow-on-failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify parallelism count for stories-tests is appropriate.
The stories-tests job is hardcoded to parallelism: 2, unlike most other test jobs that parameterize parallelism via the workflows. Confirm this is intentional and adequate. If the number of story files is large, you may want to increase parallelism or make it workflow-aware (daily=higher, normal=lower).
🏁 Script executed:
#!/bin/bash
# Count story files to gauge if parallelism=2 is sufficient
find code -type f \( -name "*.stories.ts" -o -name "*.stories.tsx" -o -name "*.stories.js" -o -name "*.stories.jsx" \) ! -path "*/node_modules/*" ! -path "*/e2e-tests/*" | wc -lLength of output: 3
---
🏁 Script executed:
```shell
#!/bin/bash
# Check how other test jobs define parallelism in config.yml
# Look for parallelism entries and surrounding context to see if they're parameterized
rg -n -C2 "parallelism:" .circleci/config.yml | head -60
Length of output: 1766
🏁 Script executed:
#!/bin/bash
# Check if there are workflow-scoped parameters or variables for parallelism
rg -n "parameters:|parallelism_|workflows:" .circleci/config.yml | head -30Length of output: 557
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the global parameters section and how parallelism is defined
rg -n -A10 "^parameters:" .circleci/config.yml
Length of output: 319
🏁 Script executed:
#!/bin/bash
# Check how workflows pass parallelism to jobs (look at workflow definitions)
rg -n -A30 "workflows:" .circleci/config.yml | grep -A30 "jobs:"Length of output: 889
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full workflow section with all job definitions to see how parallelism is passed
sed -n '966,1300p' .circleci/config.yml | head -200
Length of output: 5715
Update parallelism configuration for stories-tests to match other jobs.
The stories-tests job hardcodes parallelism: 2 (line 579), unlike all other test jobs which accept parallelism: << parameters.parallelism >>. With 475 story files in the codebase, each node processes ~238 files. The workflows should explicitly pass parallelism values for stories-tests like they do for other jobs (e.g., create-sandboxes: 34/daily, 19/merged, 13/normal; e2e-dev: 28/daily, 14/merged).
Recommended actions:
- Update the
stories-testsjob definition to accept a parallelism parameter instead of hardcoding to 2 - Add explicit parallelism parameters to workflow calls:
stories-testsin daily/merged/normal workflows
🤖 Prompt for AI Agents
In .circleci/config.yml around lines 575-594, the stories-tests job hardcodes
"parallelism: 2"; change the job to accept a parallelism parameter (use
parallelism: << parameters.parallelism >>) and update every workflow invocation
that runs stories-tests to pass explicit parallelism values (e.g., set the
daily/merged/normal workflow calls to the project-standard values such as 34 for
daily, 19 for merged, 13 for normal or the equivalent values used by other test
jobs) so the job scales consistently with other test jobs.
Addon-Vitest: Support Vitest 4 (cherry picked from commit 48a5a18)
| const projects = this.vitest!.getModuleProjects(filepath); | ||
| projects.forEach(({ server, browser }) => { | ||
| // @ts-expect-error `server` only exists in Vitest 3 | ||
| this.vitest!.projects.forEach(({ browser, vite, server }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vite already exists in Vitest 3
| this.vitest?.setGlobalTestNamePattern(''); | ||
| } | ||
|
|
||
| private updateLastChanged(filepath: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole function can be replaced with invalidateFile which is available from Vitest 3 and was added specifically for Storybook
| 'vitest.shims.d.ts', | ||
| '/// <reference types="@vitest/browser/providers/playwright" />' | ||
| isVitest4OrNewer | ||
| ? '/// <reference types="@vitest/browser-playwright" />' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need for shims in Vitest 4
Closes #
What I did
Checklist 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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Tests
Chores