Repository navigation
Channel: Ensure every runtime installs the real channel and stop the mock fallback from poisoning it - #35410
Merged
Conversation
…llback from poisoning it The shared channel slot introduced in the channel-management refactor made `addons.getChannel()`'s mock fallback global: a single early read would mirror a throwaway mock into `__STORYBOOK_ADDONS_CHANNEL__` and permanently resolve `ready()` to it, so the real channel installed later never took over. - Install the manager channel at module load (before the deferred render) so `getChannel()` returns the real channel instead of a fallback. - Make the mock fallback in both the manager-api and preview-api AddonStores non-poisoning: return a throwaway mock without caching it, mirroring it to the shared slot, or resolving `ready()`. Co-authored-by: Cursor <cursoragent@cursor.com>
ndelangen
added
bug
ci:normal
Sidnioulz
added
the
upgrade:10.5
Sidnioulz
approved these changes
Jul 8, 2026
Sidnioulz
left a comment
Sidnioulz
left a comment
Contributor
There was a problem hiding this comment.
This perfectly matches the symptoms I was seeing. LGTM!
The Storybook Vitest runtime (browser mode) has no builder preamble to install a channel, and preview open-services register at import time. This previously worked only by accident: the mock-fallback in getChannel() installed one. Now that the fallback no longer poisons the shared slot, install a channel explicitly at the setup entry point, before preview.tsx evaluates. Co-authored-by: Cursor <cursoragent@cursor.com>
Align with AddonStore.setChannel's parameter type (Channel from storybook/internal/channels) to fix a TS2345 nominal mismatch against the source-module Channel. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAddonStore.getChannel implementations in manager-api and preview-api were changed to avoid caching a mock channel when no real channel is installed yet, preventing it from blocking later real channel installation. Manager runtime channel creation moved to module load time, and a new Storybook ensure-channel setup module was added with tests. ChangesAddonStore channel fallback fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant AddonStore
participant SharedChannelSlot
Caller->>AddonStore: getChannel() (early call)
AddonStore->>SharedChannelSlot: readInstalledChannel()
SharedChannelSlot-->>AddonStore: null
AddonStore-->>Caller: mockChannel() (uncached, ready not resolved)
Caller->>AddonStore: setChannel(realChannel)
AddonStore->>SharedChannelSlot: install realChannel
Caller->>AddonStore: getChannel() (later call)
AddonStore-->>Caller: realChannel (cached, ready resolved)
✨ Finishing Touches📝 Generate docstrings
Comment |
Merged
2 tasks done
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression where an early addons.getChannel() call could permanently install a mock channel into the shared global slot, causing addons to talk to a dead channel and addons.ready() to resolve incorrectly.
Changes:
- Updated manager-api and preview-api addon stores so the “no channel yet” fallback returns a throwaway mock without caching/mirroring/resolving
ready()with it. - Made the manager runtime install its channel earlier (module load) to reduce the chance of any early reads seeing a missing channel.
- Added unit coverage for the “early getChannel fallback must not poison the shared slot” contract, plus a Storybook Vitest setup hook to ensure a channel exists in that environment.
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/core/src/preview-api/modules/addons/main.ts | Stops getChannel() from caching/mirroring the mock fallback into the shared channel slot. |
| code/core/src/manager/runtime.tsx | Installs the manager browser channel at module load (before deferred render). |
| code/core/src/manager-api/lib/addons.ts | Aligns manager-api addon store behavior with the new non-poisoning fallback contract. |
| code/core/src/manager-api/lib/addons.test.ts | Adds tests asserting early getChannel() doesn’t poison the shared slot and setChannel() takes over correctly. |
| code/.storybook/storybook.setup.ts | Ensures a channel exists before preview side-effect modules run in Storybook Vitest runtime. |
| code/.storybook/ensure-channel.ts | Installs a noop channel for Storybook Vitest browser-mode runs that lack a builder preamble. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
+37
| // Install the manager channel at module load, before the deferred render below and before any | ||
| // manager entry can read it. This guarantees `addons.getChannel()` returns the real channel in the | ||
| // manager runtime instead of falling back to a throwaway mock. | ||
| const channel = createBrowserChannel({ page: 'manager' }); | ||
| addons.setChannel(channel); | ||
| channel.emit(CHANNEL_CREATED); |
Comment on lines
+1
to
+12
| import { Channel, getChannel, setChannel } from 'storybook/internal/channels'; | ||
|
|
||
| // The Storybook Vitest runtime (browser mode) has no builder preamble to install an addons channel, | ||
| // yet preview side-effect modules (open services) call `registerService()` at import time. Install a | ||
| // channel here so those registrations have one to bind to. | ||
| // | ||
| // This lives in its own module and is imported before `./preview.tsx` in the setup file: ES modules | ||
| // evaluate their imports in source order, so an inline statement would run *after* the preview import | ||
| // (and its service registrations), too late to help. | ||
| if (!getChannel()) { | ||
| setChannel(new Channel({})); | ||
| } |
This was referenced Jul 10, 2026
Merged
This was referenced Jul 13, 2026
Merged
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
What I did
The symptom
addons.getChannel()started returning a dummy (mock) channel instead of the real one. Any addon that grabbed the channel — directly, viagetService(...), or by awaitingaddons.ready()— silently ended up talking to a dead channel, so its events never reached the preview. It was reported as a regression between10.5.0-alpha.5andalpha.6.Why it happened
getChannel()has always had a safety net: if no channel is installed yet, it hands back a throwawaymockChannel()so callers don't crash. Historically that mock lived only on the local addon store and was quietly replaced the moment the real channel arrived — no harm done.The alpha.5 → alpha.6 channel-management refactor introduced a single shared channel slot (
globalThis.__STORYBOOK_ADDONS_CHANNEL__) as the source of truth. As a side effect, the safety net changed: the throwaway mock now getsready()promise.Both are permanent. So a single early
getChannel()call — one that happens before the runtime finishes installing its real channel — now poisons the entire runtime: the real channel that arrives later is ignored, and everyready()consumer stays bound to the mock forever.The manager made this easy to trigger because it created and installed its channel inside a deferred
setTimeout(…, 0), leaving a window during startup where a read could fall into the safety net. (The preview installs its channel in the builder preamble, and Node/server installs a no-op channel on import, so those runtimes were far less exposed.)The fix
Two complementary changes so that calling
getChannel()in any runtime always returns the right channel:manager/runtime.tsx, before the deferred render — so there is no startup window left to fall into.ready()with it. The real channel installed at the runtime's entry point stays authoritative.Alternative considered
Doing only change 1 (close the manager timing window) would fix the reported case, but it leaves the sticky-mock foot-gun in place for any other early caller. Doing both removes the window and neutralizes the safety net, which matches the intended contract: "call
getChannel(), always get the right thing".Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Added
code/core/src/manager-api/lib/addons.test.ts, which asserts the contract directly: an earlygetChannel()returns a mock without touching the shared slot or flippinghasChannel(), and a later realsetChannel()takes over and resolvesready()with the real channel.Manual testing
yarn task sandbox --template react-vite/default-ts --start-from autostorybook-addon-tag-badges, and anyexperimental_setFilter-based filtering applied to the initial story index.registercallback (or a manager entry), calladdons.getChannel()and verify events actually reach the preview, i.e. it is the real browser channel rather than a dummy.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.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
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