Repository navigation
fix: wait for inline stories before rerendering docs - #35618
valentinpalkovic merged 2 commits into
Conversation
|
📝 WalkthroughWalkthroughDocs-mode global updates now await superclass processing and inline story rerenders before rerendering the docs container. Tests cover the updated event synchronization and deferred rerender ordering. ChangesDocs global update ordering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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.
🧹 Nitpick comments (1)
code/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts (1)
972-979: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove global and mock setup into the test fixture.
Line 972 directly assigns
document.location.search, and Line 975 creates an inline mock implementation inside the test. Configure the location viavi.stubGlobalwith restoration, and create/configure the gate mock throughvi.mocked()inbeforeEach.As per coding guidelines, “Do not directly assign ambient globals” and “Implement mock behaviors in
beforeEachblocks in Vitest tests.”🤖 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/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts` around lines 972 - 979, Move the document location setup and inline story rerender mock configuration out of the test body into the test fixture: use vi.stubGlobal with automatic restoration for document.location.search, and initialize/configure the createGate-backed mock via vi.mocked() in beforeEach. Keep the test focused on rendering and rerender assertions while preserving its existing behavior.Source: Coding guidelines
🤖 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/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts`:
- Around line 972-979: Move the document location setup and inline story
rerender mock configuration out of the test body into the test fixture: use
vi.stubGlobal with automatic restoration for document.location.search, and
initialize/configure the createGate-backed mock via vi.mocked() in beforeEach.
Keep the test focused on rendering and rerender assertions while preserving its
existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e5457bbd-bcee-45ec-a708-fb306d05f7eb
📒 Files selected for processing (2)
code/core/src/preview-api/modules/preview-web/PreviewWeb.test.tscode/core/src/preview-api/modules/preview-web/PreviewWithSelection.tsx
mrginglymus
left a comment
There was a problem hiding this comment.
This fixed the observed issues on https://github.com/mrginglymus/sb-rerender-doom/
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 16 | 18 | 🚨 +2 🚨 |
| Self size | 1.34 MB | 1.29 MB | 🎉 -47 KB 🎉 |
| Dependency size | 10.02 MB | 10.05 MB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook-addon-pseudo-states
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 24 KB | 21 KB | 🎉 -3 KB 🎉 |
| Dependency size | 689 B | 689 B | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 458 KB | 428 KB | 🎉 -31 KB 🎉 |
| Dependency size | 350 KB | 350 KB | 🚨 +102 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 1 | 11 | 🚨 +10 🚨 |
| Self size | 208 KB | 136 KB | 🎉 -72 KB 🎉 |
| Dependency size | 28 KB | 1.33 MB | 🚨 +1.30 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 181 | 🎉 -24 🎉 |
| Self size | 92 KB | 79 KB | 🎉 -13 KB 🎉 |
| Dependency size | 46.11 MB | 38.84 MB | 🎉 -7.27 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 21.59 MB | 21.31 MB | 🎉 -279 KB 🎉 |
| Dependency size | 31.24 MB | 31.02 MB | 🎉 -218 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 213 | 186 | 🎉 -27 🎉 |
| Self size | 240 KB | 160 KB | 🎉 -80 KB 🎉 |
| Dependency size | 39.41 MB | 31.67 MB | 🎉 -7.73 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 29 | 🚨 +9 🚨 |
| Self size | 23.62 MB | 23.39 MB | 🎉 -228 KB 🎉 |
| Dependency size | 11.57 MB | 12.70 MB | 🚨 +1.14 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 204 | 180 | 🎉 -24 🎉 |
| Self size | 13 KB | 13 KB | 🚨 +44 B 🚨 |
| Dependency size | 41.43 MB | 34.15 MB | 🎉 -7.28 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 14 | 🚨 +10 🚨 |
| Self size | 22 KB | 22 KB | 🚨 +10 B 🚨 |
| Dependency size | 271 KB | 1.50 MB | 🚨 +1.23 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 469 | 527 | 🚨 +58 🚨 |
| Self size | 641 KB | 640 KB | 🎉 -1 KB 🎉 |
| Dependency size | 69.84 MB | 65.64 MB | 🎉 -4.20 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 100 | 94 | 🎉 -6 🎉 |
| Self size | 1.42 MB | 1.37 MB | 🎉 -55 KB 🎉 |
| Dependency size | 24.16 MB | 24.36 MB | 🚨 +200 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 14 | 🚨 +10 🚨 |
| Self size | 12 KB | 12 KB | 🎉 -8 B 🎉 |
| Dependency size | 289 KB | 1.52 MB | 🚨 +1.23 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 125 | 🚨 +8 🚨 |
| Self size | 104 KB | 29 KB | 🎉 -75 KB 🎉 |
| Dependency size | 26.10 MB | 26.86 MB | 🚨 +761 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 78 | 83 | 🚨 +5 🚨 |
| Self size | 33 KB | 32 KB | 🎉 -1 KB 🎉 |
| Dependency size | 21.28 MB | 21.95 MB | 🚨 +674 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 289 | 267 | 🎉 -22 🎉 |
| Self size | 23 KB | 23 KB | 🚨 +24 B 🚨 |
| Dependency size | 58.57 MB | 51.31 MB | 🎉 -7.27 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 217 | 193 | 🎉 -24 🎉 |
| Self size | 15 KB | 15 KB | 🚨 +12 B 🚨 |
| Dependency size | 47.39 MB | 40.11 MB | 🎉 -7.28 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 12 | 20 | 🚨 +8 🚨 |
| Self size | 56 KB | 54 KB | 🎉 -2 KB 🎉 |
| Dependency size | 30.73 MB | 26.71 MB | 🎉 -4.02 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 13 | 21 | 🚨 +8 🚨 |
| Self size | 58 KB | 56 KB | 🎉 -2 KB 🎉 |
| Dependency size | 30.79 MB | 26.76 MB | 🎉 -4.03 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 79 | 84 | 🚨 +5 🚨 |
| Self size | 124 KB | 113 KB | 🎉 -11 KB 🎉 |
| Dependency size | 21.31 MB | 21.99 MB | 🚨 +673 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 95 | 103 | 🚨 +8 🚨 |
| Self size | 32 KB | 34 KB | 🚨 +3 KB 🚨 |
| Dependency size | 18.78 MB | 43.27 MB | 🚨 +24.49 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 6 | 15 | 🚨 +9 🚨 |
| Self size | 19 KB | 19 KB | 🎉 -121 B 🎉 |
| Dependency size | 530 KB | 1.57 MB | 🚨 +1.04 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 179 | 205 | 🚨 +26 🚨 |
| Self size | 883 KB | 827 KB | 🎉 -56 KB 🎉 |
| Dependency size | 87.04 MB | 86.86 MB | 🎉 -182 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 172 | 198 | 🚨 +26 🚨 |
| Self size | 44 KB | 32 KB | 🎉 -12 KB 🎉 |
| Dependency size | 85.52 MB | 85.34 MB | 🎉 -186 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 74 | 0 |
| Self size | 1.07 MB | 1.09 MB | 🚨 +16 KB 🚨 |
| Dependency size | 52.83 MB | 52.34 MB | 🎉 -497 KB 🎉 |
| Bundle Size Analyzer | node | node |
@storybook/csf-plugin
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 9 | 🚨 +9 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.30 MB | 🚨 +1.30 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-dom-shim
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 19 KB | 🚨 +19 KB 🚨 |
| Dependency size | 0 B | 1 KB | 🚨 +1 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-create-react-app
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 68 | 🚨 +68 🚨 |
| Self size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Dependency size | 0 B | 6.15 MB | 🚨 +6.15 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 152 | 156 | 🚨 +4 🚨 |
| Self size | 15 KB | 19 KB | 🚨 +4 KB 🚨 |
| Dependency size | 35.49 MB | 35.75 MB | 🚨 +264 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 58 | 59 | 🚨 +1 🚨 |
| Self size | 1.52 MB | 1.49 MB | 🎉 -31 KB 🎉 |
| Dependency size | 12.36 MB | 12.38 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 3 | 🎉 -1 🎉 |
| Self size | 253 KB | 49 KB | 🎉 -204 KB 🎉 |
| Dependency size | 4.85 MB | 612 KB | 🎉 -4.23 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 91 | 4 | 🎉 -87 🎉 |
| Self size | 232 KB | 67 KB | 🎉 -165 KB 🎉 |
| Dependency size | 18.22 MB | 595 KB | 🎉 -17.62 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 3 | 🎉 -1 🎉 |
| Self size | 196 KB | 80 KB | 🎉 -116 KB 🎉 |
| Dependency size | 126 KB | 48 KB | 🎉 -78 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
There was a problem hiding this comment.
🤖 I am the Storybook contribution review bot. This message was posted automatically. A human maintainer makes the final decision on this PR.
Review phase: Runtime verification (stage 3 of 3)We are trying a three-stage automated review: architecture, code quality, and runtime verification. This message states which stages ran and which were skipped. Messages are posted automatically, and a human maintainer makes the final decision. Please tell us whether the feedback and evidence are useful.
Thanks for this fix, @wanxiankai! All three stages ran on 6982781 and passed: architecture, code quality, and runtime verification. Nothing blocks this PR.
- Bug reproduced and fixed: with the sb-rerender-doom reproduction, one globals change on a docs page with several stories reloaded the preview iframe on the base commit
665b1f1in 5 of 5 runs, and in 0 of 5 runs with this PR. - Tests: the new test fails when the
awaitis removed and passes with it. Thepreview-apiunit tests pass.
I left two optional notes inline. You don't need to address them for this PR.
Closes #29007
What I did
This prevents overlapping docs and inline-story lifecycles from causing repeated reloads, missing event sources, and stale globals when backgrounds change.
Implementation and tests were developed with assistance from OpenAI Codex. I reviewed the resulting diff and validation results.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
Documentation is not required because this fixes internal preview render ordering without changing the public API or documented behavior.
Checklist for Maintainers
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.tsqa:neededorqa:skipSummary by CodeRabbit