Merge pull request storybookjs#35410 from storybookjs/norbert/fix-channel-runtime-install

Channel: Ensure every runtime installs the real channel and stop the mock fallback from poisoning it

Author: ndelangen

code/.storybook/ensure-channel.ts

@@ -0,0 +1,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({}));
+}

code/.storybook/storybook.setup.ts

@@ -6,6 +6,9 @@ import { userEvent as storybookEvent, expect as storybookExpect } from 'storyboo
 
 import '../core/src/shared/utils/toHaveLiveRegion.ts';
 
+// Must be imported before `./preview.tsx` so a channel exists when preview open-services register.
+import './ensure-channel.ts';
+
 import preview from './preview.tsx';
 
 vi.spyOn(console, 'warn').mockImplementation((...args) => console.log(...args));

code/core/src/manager-api/lib/addons.test.ts

@@ -0,0 +1,43 @@
+// @vitest-environment happy-dom
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+import { Channel } from 'storybook/internal/channels';
+
+import { clearChannel, getChannel as readInstalledChannel } from '../../channels/channel-slot.ts';
+import { AddonStore } from './addons.ts';
+
+describe('AddonStore channel', () => {
+  beforeEach(() => {
+    clearChannel();
+  });
+
+  afterEach(() => {
+    clearChannel();
+  });
+
+  it('returns a throwaway mock before a channel is installed, without poisoning the shared slot', () => {
+    const store = new AddonStore();
+
+    // Reading early must not crash...
+    expect(store.getChannel()).toBeDefined();
+
+    // ...but it must not install anything into the shared slot, so the real channel installed later
+    // at the runtime entry point can still take over.
+    expect(readInstalledChannel()).toBeNull();
+    expect(store.hasChannel()).toBe(false);
+  });
+
+  it('lets a real setChannel() take over after an early getChannel() fallback', async () => {
+    const store = new AddonStore();
+
+    // Simulate an errant early read (e.g. at manager module-eval before the channel is installed).
+    store.getChannel();
+
+    const real = new Channel({});
+    store.setChannel(real);
+
+    expect(store.getChannel()).toBe(real);
+    expect(readInstalledChannel()).toBe(real);
+    await expect(store.ready()).resolves.toBe(real);
+  });
+});

code/core/src/manager-api/lib/addons.ts

@@ -50,18 +50,22 @@ export class AddonStore {
   private resolve: any;
 
   getChannel = (): Channel => {
-    if (!this.channel) {
-      const installed = readInstalledChannel();
-      if (installed) {
-        this.channel = installed as Channel;
-        this.resolve();
-        return this.channel;
-      }
-
-      this.setChannel(mockChannel() as unknown as Channel);
+    if (this.channel) {
+      return this.channel;
     }
 
-    return this.channel!;
+    const installed = readInstalledChannel();
+    if (installed) {
+      this.channel = installed as Channel;
+      this.resolve();
+      return this.channel;
+    }
+
+    // No channel has been installed in this runtime yet. Return a throwaway mock so callers do not
+    // crash, but do NOT cache it, mirror it to the shared channel slot, or resolve `ready()` with
+    // it. Otherwise a single early read would poison the whole runtime, and the real channel
+    // installed later (at the runtime entry point) would never take over.
+    return mockChannel() as unknown as Channel;
   };
 
   ready = (): Promise<Channel> => this.promise;

code/core/src/manager/runtime.tsx

@@ -29,26 +29,20 @@ addons.register(TOOLBAR_ID, () =>
   })
 );
 
+// 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);
+
 class ReactProvider extends Provider {
-  addons: AddonStore;
+  addons: AddonStore = addons;
 
-  channel: Channel;
+  channel: Channel = channel;
 
   wsDisconnected = false;
 
-  constructor() {
-    super();
-
-    const channel = createBrowserChannel({ page: 'manager' });
-
-    addons.setChannel(channel);
-
-    channel.emit(CHANNEL_CREATED);
-
-    this.addons = addons;
-    this.channel = channel;
-  }
-
   getElements(type: Addon_Types) {
     return this.addons.getElements(type);
   }

code/core/src/preview-api/modules/addons/main.ts

@@ -19,20 +19,22 @@ export class AddonStore {
   private resolve: any;
 
   getChannel = (): Channel => {
-    if (!this.channel) {
-      const installed = readInstalledChannel();
-      if (installed) {
-        this.channel = installed as Channel;
-        this.resolve();
-        return this.channel;
-      }
-
-      const channel = mockChannel() as unknown as Channel;
-      this.setChannel(channel);
-      return channel;
+    if (this.channel) {
+      return this.channel;
     }
 
-    return this.channel;
+    const installed = readInstalledChannel();
+    if (installed) {
+      this.channel = installed as Channel;
+      this.resolve();
+      return this.channel;
+    }
+
+    // No channel has been installed in this runtime yet. Return a throwaway mock so callers do not
+    // crash, but do NOT cache it, mirror it to the shared channel slot, or resolve `ready()` with
+    // it. Otherwise a single early read would poison the whole runtime, and the real channel
+    // installed later (at the runtime entry point) would never take over.
+    return mockChannel() as unknown as Channel;
   };
 
   ready = (): Promise<Channel> => this.promise;