Build: Simplify setupVitest function and remove obsolete handling for CSF4 #33103
Conversation
valentinpalkovic
added
build
📝 WalkthroughWalkthroughRemoved the CSF4-specific vitest setup generation path; the scripts now always generate the richer non-CSF4 vitest setup with additional imports and a composite setProjectAnnotations call. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Task as sandbox-parts task
participant FS as File system
Note over Task: Previous logic (removed)
Task->>Task: if shouldUseCsf4 == true\nwrite minimal CSF4 setup
Task->>FS: write csf4-vitest-setup.ts (removed)
Note over Task: New logic (current)
Task->>Task: if shouldUseCsf4 == true\n(now skips CSF4 path)
Task->>Task: else\ncompose rich annotations & imports
Task->>FS: write vitest-setup.ts (non-CSF4, composite annotations)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/tasks/sandbox-parts.ts (1)
469-490: ClarifyshouldUseCsf4behavior and tighten vitest.setup generationA few points around this block are worth tightening up:
Behavior for
@storybook/react-vite/ CSF4
shouldUseCsf4is now only used as a negative guard, so fortemplate.expected.framework === '@storybook/react-vite'no.storybook/vitest.setup.*file is written at all. That’s a material behavior change from the prior CSF4 path.
- If the intent is to stop generating any custom vitest setup for React‑Vite and rely purely on addon defaults, a short comment here and a more neutral name (e.g.
isReactVite) would make that clearer.- If React‑Vite should instead follow the new shared setup path, the
if (!shouldUseCsf4)guard can be dropped and the setup written unconditionally.Unused
beforeAllimport in generated file
Inside thededenttemplate the generated setup currently starts with:import { beforeAll } from 'vitest' import { setProjectAnnotations } from '${storybookPackage}'but
beforeAllis never used. Unless you intend to wrapsetProjectAnnotationsin abeforeAllcall, this is dead code in every generated sandbox file.Hard‑coded
../src/stories/componentsimport
The setup file unconditionally does:import '../src/stories/components'whereas
extendPreviewalready handles both../src/stories/componentsand../stories/componentsbased on whethersrc/storiesexists in the sandbox. For templates that only have astories/folder, this import will fail even though the rest of the sandbox wiring works.You can mirror the
extendPreviewlogic and inject the correct import path into the generated file, e.g.:
- const shouldUseCsf4 = template.expected.framework === '@storybook/react-vite';
- if (!shouldUseCsf4) {
- await writeFile(
setupFilePath,dedent`import { beforeAll } from 'vitest'import { setProjectAnnotations } from '${storybookPackage}'
- const shouldUseCsf4 = template.expected.framework === '@storybook/react-vite';
- if (!shouldUseCsf4) {
- const hasSrcStories = await pathExists(join(sandboxDir, 'src', 'stories'));
- const storiesComponentsImport = hasSrcStories
? '../src/stories/components': '../stories/components';- await writeFile(
setupFilePath,dedent`import { setProjectAnnotations } from '${storybookPackage}' import * as rendererDocsAnnotations from '${template.expected.renderer}/entry-preview-docs' import * as addonA11yAnnotations from '@storybook/addon-a11y/preview'
import '../src/stories/components'
import '${storiesComponentsImport}' import * as templateAnnotations from '../template-stories/core/preview' import * as projectAnnotations from './preview' ${isVue ? 'import * as vueAnnotations from "../src/stories/renderers/vue3/preview.js"' : ''}@@
])`
])`
}(This also drops the unused `beforeAll` import.)Please double‑check the intended behavior for React‑Vite sandboxes and whether you want them to use this new shared setup path or truly have no generated vitest setup file anymore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/tasks/sandbox-parts.ts(1 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
|
View your CI Pipeline Execution ↗ for commit b497ddb
☁️ Nx Cloud last updated this comment at |
…
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
✏️ Tip: You can customize this high-level summary in your review settings.