Repository navigation
Vitest: Removal of workarounds obsoleted by the Vitest 4 floor - #36200
Conversation
… 4 floor (re-opens #36188)
AriPerkkio
left a comment
There was a problem hiding this comment.
Vitest 4 also allows us to use programatic way for controlling coverage. We don't need to restart whole Vitest instance anymore: https://vitest.dev/api/advanced/vitest.html#enablecoverage
storybook/code/addons/vitest/src/node/vitest-manager.ts
Lines 384 to 385 in eb9f3f8
|
Thanks — confirmed against the Vitest 4 docs: Two things shape how I can act on this right now: this PR is the narrow re-land of the previously reviewed removals, and the closed-instance drop before coverage restarts was just restored at the maintainer's request (see the review thread above) — which lives in the same |
There was a problem hiding this comment.
Also:
storybook/code/addons/vitest/src/node/reporter.ts
Lines 63 to 65 in eb9f3f8
I remember removing processTimeoutCauses from Vitest 4 when rewriting pools without tinypool.
|
Confirmed — in the current tree, // TODO: Remove this once we don't support Vitest < 4
// @ts-expect-error processTimeoutCauses does not exist in Vitest 4
this.ctx.state.processTimeoutCauses?.clear();With Vitest 4 as the floor, Two constraints on acting here: this PR's diff does not touch |
Closes #
What I did
Follow-up to #36164 (the Vitest 4 floor): removes the compatibility workarounds that PR identified as gated on the new floor, and documents the one filter that stays.
server.deps.inline: ['@storybook/addon-vitest']injection from the Vitest plugin (code/addons/vitest/src/vitest-plugin/index.ts). Vitest 4 unconditionally inlinessetupFiles, and the only test-runtime import of the addon —@storybook/addon-vitest/internal/test-utils, injected into transformed story tests — loads cleanly as an externalized module. A user's ownserver.deps.inlineconfig is now passed through untouched, soinline: trueand custom lists behave exactly as configured.this.vitest = nullinVitestManager.restartVitest,code/addons/vitest/src/node/vitest-manager.ts). It existed because Vite 6 exposed the module runner'simport.meta.envas an own property, so Vitest deep-cloning the coverage reporter options hit its throwing get trap; with the Vite floor now 7 that traversal is gone. The restart path itself (close +startVitest({ coverage })) is unchanged and still powers the coverage feature.vitest.projects.*probe fromvalidateConfigFiles(code/core/src/cli/AddonVitestService.ts), plus the now caller-lesscanUpdateVitestWorkspaceFilebabel helper, its export, and its tests (code/core/src/babel/index.ts,code/core/src/babel/vitest-config-helpers.ts,code/core/src/babel/vitest-config-helpers.test.ts). No Vitest version ever loadedvitest.projects.*files — the workspace file was deprecated in Vitest 3.2 in favor of theprojectsconfig field and removed in Vitest 4 — and Storybook only ever writes projects intovitest.config.*, so the probe could only flag inert files.'spy'mock-name filter incode/core/src/actions/loaders.ts, replacing its stale TODO with the actual retention rationale. The filter is load-bearing whilestorybook/testships its own pinned@vitest/spy@3.2.4:onMockCallfires only forstorybook/test's ownfn/spyOnwrappers, and that pinned runtime still defaultsgetMockName()to'spy'(Vitest 4's own spy defaults to'vi.fn()'and never reaches the listener). Removing the filter would log every unnamed mock asaction("spy")in the Actions panel. Revisiting it belongs with rebasingstorybook/testonto Vitest 4 internals, which is out of scope here.The pinned
@vitest/expect/@vitest/spy3.2.4 runtime instorybook/testis intentionally untouched.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Updated
AddonVitestService.test.tsandvitest-config-helpers.test.tsfor the removed probe/helper (projects-file cases deleted, config-file cases re-anchored on the single remainingfind.anyprobe); the rest of the addon-vitest and core-actions suites pass unchanged.Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
This PR is backend/tooling-only (Node-side plugin config, CLI service, babel helpers) — there is no visual surface to verify in a browser, which is why no screenshot/video evidence is attached. The removed workarounds do change runtime behavior for addon-vitest users, so a manual check of the test addon flow is worthwhile:
yarn task vitest-integration --start-from auto --template react-vite/default-tsyarn test:vitest/ the addon's "Run tests" flow in the Storybook UI) and confirm stories' component and interaction tests run and pass.Documentation
MIGRATION.MD
No documentation changes: these are internal compatibility removals with unchanged user-facing behavior; the Vitest 4 upgrade requirements were already documented by #36164.
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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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 - 🚫 Not run
This PR does not have a canary release associated.
In-repo PRs: add the
ci:canarylabel. Later pushes republish while the label remains.Fork PRs: the label does nothing (a later push must not auto-publish). A maintainer publishes from this repository with Run workflow and the
prinput.branchandshaare optional; if more than one is set, they must be the same commit. The fork author does not need to do anything.Test Evidence
Backend-only change (Vitest/Babel/test-infrastructure integration) — no visual product surface, so visual/screenshot evidence does not apply; the verification below was recorded against this PR's head commit.
yarn task vitest-integration --start-from auto --template react-vite/default-tswith the workspace build linked from this PR's sources: the sandbox was built end-to-end (create → install → addon init → vitest setup) andyarn vitest runpassed 247/247 tests (82 test files passed, 8 skipped) in real browser mode (Chromium via Playwright) on Vitest 4.1.11.restartVitest, where the removed Vite 6 workaround used to pre-dropthis.vitest): replicated the exact close→recreate sequence headlessly against the sandbox's Vitest/Vite runtime —createVitestwith coverage enabled →close()→ re-create with coverage reporter options embedding the closed instance'sconfig.coverage(the deep-clone hazard the workaround dodged) — completes with no throw; the coverage-off restart cycle was verified as well.code/addons/vitest/src/node/vitest-manager.ts.🔗 Obvious Project · 🧵 Obvious Thread