Addon Vitest: Fix dynamic import failure with Vitest 3#34927
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Vitest 3 compatibility issues in @storybook/addon-vitest by avoiding a failing dynamic import path for browser commands and instead selecting the correct import target based on the Vitest version injected into the test context.
Changes:
- Add a new provided-context key to carry the running Vitest version through to the setup file.
- Provide
context.vitest.versionfrom the Vitest plugin (configureVitest) for consumption viainject(). - Update the Vitest setup file to choose
@vitest/browser/contextfor Vitest 3 vsvitest/browserotherwise.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
code/addons/vitest/src/vitest-provided-context.d.ts |
Extends Vitest ProvidedContext typing with a new key for the Vitest version. |
code/addons/vitest/src/vitest-plugin/setup-file.ts |
Switches browser command import path based on injected Vitest version. |
code/addons/vitest/src/vitest-plugin/index.ts |
Provides Vitest version into the project context during configureVitest. |
code/addons/vitest/src/constants.ts |
Introduces the new provide-key constant for the Vitest version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a dependency-injection key for Vitest's runtime version, registers that version on the project during plugin configuration, and uses the injected version in the setup-file to conditionally import the correct browser command module (supporting both Vitest 3 and pre-3 layouts). It also refactors setup-files array construction and updates test workspace configuration. ChangesVitest version provision for runtime compatibility
Test workspace and debug updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Comment |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
…d missing modules Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/addons/vitest/src/vitest-plugin/index.ts (1)
328-332: ⚡ Quick winAvoid
filter(Boolean) as string[]and encode the invariant in types.Line 332 relies on a type assertion after runtime filtering, which weakens static guarantees. Build the array with conditional spread so the type is
string[]withoutas.♻️ Proposed refactor
- const internalSetupFiles = [ - '`@storybook/addon-vitest/internal/setup-file`', - areProjectAnnotationRequired && - '`@storybook/addon-vitest/internal/setup-file-with-project-annotations`', - ].filter(Boolean) as string[]; + const internalSetupFiles: string[] = [ + '`@storybook/addon-vitest/internal/setup-file`', + ...(areProjectAnnotationRequired + ? ['`@storybook/addon-vitest/internal/setup-file-with-project-annotations`'] + : []), + ];As per coding guidelines, "Encode assumptions with static checks first; if an assumption is expected to always hold, prefer making it impossible via TypeScript types and existing lint rules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/vitest/src/vitest-plugin/index.ts` around lines 328 - 332, The current construction of internalSetupFiles uses runtime filter(Boolean) and a type assertion, weakening type safety; instead build the array with a conditional spread so it is statically a string[] (e.g., include '`@storybook/addon-vitest/internal/setup-file`' and then ...(areProjectAnnotationRequired ? ['`@storybook/addon-vitest/internal/setup-file-with-project-annotations`'] : []) ), remove the filter(Boolean) and the "as string[]" assertion so the type of internalSetupFiles is inferred as string[]; update any references expecting string[] accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@code/addons/vitest/src/vitest-plugin/index.ts`:
- Around line 328-332: The current construction of internalSetupFiles uses
runtime filter(Boolean) and a type assertion, weakening type safety; instead
build the array with a conditional spread so it is statically a string[] (e.g.,
include '`@storybook/addon-vitest/internal/setup-file`' and then
...(areProjectAnnotationRequired ?
['`@storybook/addon-vitest/internal/setup-file-with-project-annotations`'] : [])
), remove the filter(Boolean) and the "as string[]" assertion so the type of
internalSetupFiles is inferred as string[]; update any references expecting
string[] accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2c4939c-4273-4c8d-bbc1-d5fd8c5dfaa8
📒 Files selected for processing (3)
code/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/src/vitest-plugin/setup-file.tstest-storybooks/portable-stories-kitchen-sink/react-vitest-3/vitest.workspace.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/addons/vitest/src/vitest-plugin/setup-file.ts
What I did
Fixes recent CI failures on
ci:dailywhere dynamic imports of Vitest Browser failed with Vitest 3 due to how the addon code is transformed. We now directly import the right dependency from the get go.Checklist for Contributors
Testing
ø
Manual testing
ci:dailynow passesDocumentation
ø
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Bug Fixes
Chores