Repository navigation
Conversation
1c71eaa to
ac1de14
Compare
|
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:
📝 WalkthroughWalkthroughThe Vitest addon gains a new ChangesVitest plugin initialGlobals support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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.
Pull request overview
Adds support for pinning Storybook globals per Vitest project in @storybook/addon-vitest, enabling workflows like “one theme per project” runs by threading initialGlobals through Vitest’s provide/inject and merging it beneath Storybook’s internal run-control globals.
Changes:
- Add
UserOptions.initialGlobalstostorybookTest()and provide it into Vitest’s project context. - Inject
initialGlobalsduringtestStory()composition and merge it under Storybook’s internal globals (e.g.a11y.manual). - (Stacked) Update portable stories to flush preview-api hook effects after render, with a regression test.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| code/renderers/react/src/test/portable-stories.test.tsx | Adds a regression test ensuring preview-api useEffect decorator side-effects are flushed under portable stories. |
| code/core/src/preview-api/modules/store/csf/portable-stories.ts | Flushes preview-api hook effects in the portable render path by invoking the hooks render listener post-render. |
| code/addons/vitest/src/vitest-plugin/types.ts | Introduces the public initialGlobals option on the vitest addon plugin options. |
| code/addons/vitest/src/vitest-plugin/test-utils.ts | Injects project initialGlobals and merges them into the composed story’s globals for each run. |
| code/addons/vitest/src/vitest-plugin/index.ts | Provides initialGlobals via Vitest test.provide for the project. |
| code/addons/vitest/src/constants.ts | Adds a new provide/inject key constant for initialGlobals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ac1de14 to
d2f7b94
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code/addons/vitest/src/vitest-plugin/compose-initial-globals.test.ts (1)
19-43: ⚡ Quick winAdd regression cases for disabled-flag precedence and partial
a11yoverride.Current tests don’t cover the case where
ghostStoriesEnabled/renderAnalysisEnabledarefalseanduserInitialGlobalstries to set them to enabled. Add that assertion to lock the precedence contract. Also add a case ensuring onlya11y.manualis overridden (and othera11ykeys are preserved), if that is the intended behavior.🤖 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/compose-initial-globals.test.ts` around lines 19 - 43, The test file needs additional regression cases for the composeInitialGlobals function. Add a test that verifies when ghostStoriesEnabled or renderAnalysisEnabled are explicitly set to false, the function prevents userInitialGlobals from overriding those disabled flags to enabled (testing the precedence contract). Additionally, add a test case that provides a partial a11y override in userInitialGlobals (for example, only overriding a11y.manual while leaving other a11y properties) to ensure only the specified a11y properties are overridden and any other existing a11y keys are preserved from the base or run configuration.
🤖 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.
Inline comments:
In `@code/addons/vitest/src/vitest-plugin/compose-initial-globals.ts`:
- Line 26: The a11y field assignment on line 26 is replacing the entire a11y
object with only the manual property, discarding any other user-provided a11y
configuration. Instead of assigning a new object with just the manual field,
merge the manual property into the existing a11y configuration to preserve any
other user-defined a11y globals. Use object spreading or a merge operation to
combine the user's a11y configuration with the computed manual property so that
manual takes precedence while other user fields remain intact.
- Around line 24-25: The conditional spreading of ghostStories and
renderAnalysis globals allows them to be bypassed if userInitialGlobals already
contains these properties. Instead of conditionally spreading these objects only
when enabled, set them unconditionally in the globals object with their
respective enabled flags (ghostStoriesEnabled and renderAnalysisEnabled) so that
the run-control flags always take precedence over any values in
userInitialGlobals.
---
Nitpick comments:
In `@code/addons/vitest/src/vitest-plugin/compose-initial-globals.test.ts`:
- Around line 19-43: The test file needs additional regression cases for the
composeInitialGlobals function. Add a test that verifies when
ghostStoriesEnabled or renderAnalysisEnabled are explicitly set to false, the
function prevents userInitialGlobals from overriding those disabled flags to
enabled (testing the precedence contract). Additionally, add a test case that
provides a partial a11y override in userInitialGlobals (for example, only
overriding a11y.manual while leaving other a11y properties) to ensure only the
specified a11y properties are overridden and any other existing a11y keys are
preserved from the base or run configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06f9d2ec-ba2c-4cd5-9339-e3abbd3b4394
📒 Files selected for processing (9)
code/addons/vitest/src/constants.tscode/addons/vitest/src/vitest-plugin/compose-initial-globals.test.tscode/addons/vitest/src/vitest-plugin/compose-initial-globals.tscode/addons/vitest/src/vitest-plugin/index.tscode/addons/vitest/src/vitest-plugin/test-utils.tscode/addons/vitest/src/vitest-plugin/types.tscode/addons/vitest/src/vitest-provided-context.d.tscode/core/src/preview-api/modules/store/csf/portable-stories.tscode/renderers/react/src/__test__/portable-stories.test.tsx
✅ Files skipped from review due to trivial changes (2)
- code/addons/vitest/src/vitest-provided-context.d.ts
- code/addons/vitest/src/constants.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- code/addons/vitest/src/vitest-plugin/types.ts
- code/addons/vitest/src/vitest-plugin/index.ts
- code/addons/vitest/src/vitest-plugin/test-utils.ts
- code/core/src/preview-api/modules/store/csf/portable-stories.ts
- code/renderers/react/src/test/portable-stories.test.tsx
a6ac4b6 to
f011f2b
Compare
f011f2b to
b64233b
Compare
|
@lifeiscontent looks good to me codewise, though we'll need documentation for this new feature. Would you like to give this a go? As for testing, you could add an |
|
@Sidnioulz sounds good, I'll try putting it together tomorrow 🙌 |
b64233b to
83ae1d6
Compare
|
Done on both fronts:
|
83ae1d6 to
70a362e
Compare
Note
Stacked on #35224. Until that merges this PR also shows its portable-stories commit; review only the
Addon-vitest: add an initialGlobals option ...commit here. Kept as a draft until #35224 lands.What I did
Adds an
initialGlobalsoption tostorybookTest(). It's threaded into each test run via Vitest'sprovide/injectand merged underneath Storybook's own run-control globals (soa11y.manualand the internal keys still win). It's the per-project equivalent ofinitialGlobalsin.storybook/preview.The motivating use is theme testing. Once #35224 makes
@storybook/addon-themesdecorators apply under the test runner, you can run the a11y/interaction gate across every theme by defining one Vitest project per theme:How it works
UserOptions.initialGlobals(default{}).storybookTest()provides it under a new key intest.provide.testStory()injects it and spreads it first into the composed story'sinitialGlobals, so the addon's own globals override it.Open questions
initialGlobalsvsglobals).Summary by CodeRabbit
Release Notes
New Features
.storybook/previewsettings.Tests