From f450eadb54ea9daf9ce62a227c782b39bf834573 Mon Sep 17 00:00:00 2001 From: unional Date: Thu, 20 Aug 2026 22:36:47 -0700 Subject: [PATCH] fix(storybook-addon-vis): await the vitest module load before using the proxies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `commands` and `page` are populated by a dynamic import that nothing awaited, so a hook reading `commands.setupVisSuite` before it settled got `undefined` and failed with `commands.setupVisSuite is not a function` — inside a run that genuinely is vitest browser mode. The imports stay dynamic (this module is also reached from a plain Storybook preview, where the vitest modules do not exist), but the ordering is now enforced: - a command read before the import settles returns a function that waits for it and then delegates, instead of `undefined` - `whenVitestProxyReady()` exposes the load, and the addon's `beforeAll` awaits it before reading `page` or the current test `createVitestProxy` is exported so the pending window can be driven directly in a test; the module-level proxy's import has long settled by then. Closes #835 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NLB1dHgUEekHmMNj4G7R63 --- .changeset/olive-moons-shave.md | 12 ++ .../src/client/vitest_proxy.spec.ts | 119 ++++++++++++++++++ .../src/client/vitest_proxy.ts | 110 +++++++++++----- packages/storybook-addon-vis/src/index.ts | 5 +- 4 files changed, 217 insertions(+), 29 deletions(-) create mode 100644 .changeset/olive-moons-shave.md create mode 100644 packages/storybook-addon-vis/src/client/vitest_proxy.spec.ts diff --git a/.changeset/olive-moons-shave.md b/.changeset/olive-moons-shave.md new file mode 100644 index 00000000..47cefc00 --- /dev/null +++ b/.changeset/olive-moons-shave.md @@ -0,0 +1,12 @@ +--- +'storybook-addon-vis': patch +--- + +Wait for the vitest browser modules before using them. + +`commands` and `page` are backed by a dynamic import that nothing awaited, so a +hook reading `commands.setupVisSuite` before it settled got `undefined` and +failed with `commands.setupVisSuite is not a function` — inside a genuine vitest +browser run. Command reads now return a function that waits for the import, and +the addon's `beforeAll` awaits the module load before touching `page` or the +current test. diff --git a/packages/storybook-addon-vis/src/client/vitest_proxy.spec.ts b/packages/storybook-addon-vis/src/client/vitest_proxy.spec.ts new file mode 100644 index 00000000..ee462189 --- /dev/null +++ b/packages/storybook-addon-vis/src/client/vitest_proxy.spec.ts @@ -0,0 +1,119 @@ +import { describe, it } from 'vitest' +import { commands, createVitestProxy, whenVitestProxyReady } from './vitest_proxy.ts' + +/** + * Regression tests for #835. + * + * The vitest modules behind the proxies are loaded by a dynamic import. Nothing + * used to await it, so a hook reading `commands.setupVisSuite` before the import + * settled got `undefined` and failed with `setupVisSuite is not a function` — + * in a run that genuinely is vitest browser mode. + * + * The timing cannot be observed on the module-level proxy (its import has long + * settled by the time a test runs), so these drive `createVitestProxy` with + * loaders that stay pending until the test resolves them. + */ +describe('while the dynamic import is still pending', () => { + function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((r) => { + resolve = r + }) + return { promise, resolve } + } + + function fakeBrowserModule(calls: unknown[][]) { + return { + page: { extend() {} }, + commands: { + async setupVisSuite(...args: unknown[]) { + calls.push(args) + return { subject: '[data-testid="subject"]' } + }, + }, + } as any + } + + function pendingProxy() { + const browser = deferred() + const vitest = deferred() + const proxy = createVitestProxy({ + loadBrowser: () => browser.promise, + loadVitest: () => vitest.promise, + }) + return { proxy, browser, vitest } + } + + it('a command is still a function, not undefined', ({ expect }) => { + const { proxy } = pendingProxy() + + expect(typeof proxy.commands.setupVisSuite).toBe('function') + }) + + it('a command called before the import settles waits for it and delegates once', async ({ expect }) => { + const calls: unknown[][] = [] + const { proxy, browser } = pendingProxy() + + const pending = proxy.commands.setupVisSuite() + expect(calls).toEqual([]) + + browser.resolve(fakeBrowserModule(calls)) + + await expect(pending).resolves.toEqual({ subject: '[data-testid="subject"]' }) + expect(calls).toEqual([[]]) + }) + + it('the command receives its arguments', async ({ expect }) => { + const calls: unknown[][] = [] + const { proxy, browser } = pendingProxy() + + const pending = (proxy.commands as any).setupVisSuite('a', 1) + browser.resolve(fakeBrowserModule(calls)) + await pending + + expect(calls).toEqual([['a', 1]]) + }) + + it('`whenReady` does not resolve until both modules are loaded', async ({ expect }) => { + let resolved = false + const { proxy, browser, vitest } = pendingProxy() + proxy.whenReady().then(() => { + resolved = true + }) + + browser.resolve(fakeBrowserModule([])) + await Promise.resolve() + expect(resolved).toBe(false) + + vitest.resolve({ TestRunner: { getCurrentTest: () => undefined } }) + await proxy.whenReady() + expect(resolved).toBe(true) + }) + + it('`getCurrentTest` reports the current test once vitest is loaded', async ({ expect }) => { + const { proxy, browser, vitest } = pendingProxy() + expect(proxy.getCurrentTest()).toBeUndefined() + + browser.resolve(fakeBrowserModule([])) + vitest.resolve({ TestRunner: { getCurrentTest: () => ({ name: 'a test' }) } }) + await proxy.whenReady() + + expect(proxy.getCurrentTest()).toEqual({ name: 'a test' }) + }) +}) + +describe('outside a vitest browser run', () => { + it('commands stay empty so the addon guards decide (#829)', ({ expect }) => { + const proxy = createVitestProxy(undefined) + + expect(proxy.commands.setupVisSuite).toBeUndefined() + }) +}) + +describe('in this vitest browser run', () => { + it('the real commands are in place once `whenVitestProxyReady` resolves', async ({ expect }) => { + await whenVitestProxyReady() + + expect(typeof commands.setupVisSuite).toBe('function') + }) +}) diff --git a/packages/storybook-addon-vis/src/client/vitest_proxy.ts b/packages/storybook-addon-vis/src/client/vitest_proxy.ts index 3bc4d3df..f64b7777 100644 --- a/packages/storybook-addon-vis/src/client/vitest_proxy.ts +++ b/packages/storybook-addon-vis/src/client/vitest_proxy.ts @@ -3,36 +3,90 @@ import type { SnapshotTestMeta } from 'vitest-plugin-vis/client-api' import { isVitestBrowser } from './is_vitest_browser.ts' import { toMatchImageSnapshot } from './page/to_match_image_snapshot.ts' -let browserContext: Awaited -let vitest: Awaited +type BrowserModule = Awaited +type VitestModule = Awaited -if (isVitestBrowser()) { - import('vitest/browser').then((m) => { - m.page.extend({ toMatchImageSnapshot }) - browserContext = m +/** + * How the proxy gets hold of the vitest modules. + * + * They are loaded dynamically because this module is also reached from a plain + * Storybook preview, where `vitest` and `vitest/browser` are not available. + * Passing `undefined` stands for "not a vitest browser run": nothing is loaded + * and the proxies stay empty. + */ +export type VitestProxyLoaders = { + loadBrowser: () => Promise + loadVitest: () => Promise +} + +/** + * Build the `page` / `commands` / `getCurrentTest` proxies over `loaders`. + * + * Exported for tests: it is the only way to observe the window between module + * load and the dynamic imports settling. + */ +export function createVitestProxy(loaders: VitestProxyLoaders | undefined) { + let browserContext: BrowserModule | undefined + let vitest: VitestModule | undefined + + const browserReady = loaders + ? loaders.loadBrowser().then((m) => { + m.page.extend({ toMatchImageSnapshot }) + browserContext = m + }) + : Promise.resolve() + const vitestReady = loaders + ? loaders.loadVitest().then((m) => { + vitest = m + }) + : Promise.resolve() + const ready = Promise.all([browserReady, vitestReady]).then(() => undefined) + + const page = new Proxy({} as any, { + get(_target, prop) { + const r = (browserContext?.page as any)?.[prop] + if (prop === 'toMatchImageSnapshot' && r === undefined) { + return () => {} + } + return r + }, }) - import('vitest').then((m) => { - vitest = m + + const commands = new Proxy({} as any, { + get(_target, prop) { + // Outside a vitest browser run there is nothing to wait for: keep + // yielding `undefined` so the addon guards stay in charge (#829). + if (!loaders) return undefined + if (browserContext) return (browserContext.commands as any)[prop] + // The import has not settled yet (#835). Commands are async RPC calls, + // so hand back a function that waits for the module instead of + // `undefined`, which fails as ` is not a function`. + return (...args: unknown[]) => browserReady.then(() => (browserContext!.commands as any)[prop](...args)) + }, }) + + const getCurrentTest = () => + vitest?.TestRunner.getCurrentTest() as + | (ReturnType & SnapshotTestMeta) + | undefined + + /** + * Resolves once the vitest modules behind `page`, `commands`, and + * `getCurrentTest` are loaded. + * + * `page` and `getCurrentTest` are read synchronously, so hooks that depend on + * them must await this first (#835). + */ + const whenReady = () => ready + + return { page, commands, getCurrentTest, whenReady } } -export const page = new Proxy({} as any, { - get(_target, prop) { - const r = (browserContext?.page as any)?.[prop] - if (prop === 'toMatchImageSnapshot' && r === undefined) { - return () => {} - } - return r - }, -}) - -export const commands = new Proxy({} as any, { - get(_target, prop) { - return (browserContext?.commands as any)?.[prop] - }, -}) - -export const getCurrentTest = () => - vitest?.TestRunner.getCurrentTest() as - | (ReturnType & SnapshotTestMeta) - | undefined +const proxy = createVitestProxy( + isVitestBrowser() ? { loadBrowser: () => import('vitest/browser'), loadVitest: () => import('vitest') } : undefined, +) + +export const page = proxy.page +export const commands = proxy.commands +export const getCurrentTest = proxy.getCurrentTest +export const whenVitestProxyReady = proxy.whenReady diff --git a/packages/storybook-addon-vis/src/index.ts b/packages/storybook-addon-vis/src/index.ts index a82f3f99..b456264b 100644 --- a/packages/storybook-addon-vis/src/index.ts +++ b/packages/storybook-addon-vis/src/index.ts @@ -7,7 +7,7 @@ import type { SetupVisOptions } from 'vitest-plugin-vis' import { autoSnapshotMatcher, setAutoSnapshotOptions } from 'vitest-plugin-vis/client-api' import { toMatchImageSnapshot } from './client/expect/to_match_image_snapshot.ts' import { isVitestBrowser } from './client/is_vitest_browser.ts' -import { commands, page } from './client/vitest_proxy.ts' +import { commands, page, whenVitestProxyReady } from './client/vitest_proxy.ts' import { visAnnotations } from './preview/vis_annotation.ts' // Register at module load time so it's available in Storybook dev mode. @@ -28,6 +28,9 @@ export default (options: SetupVisOptions<{ tags: string[] }> = { auto: false }) tags: options.auto === true ? ['snapshot'] : [], async beforeAll() { if (!isVitestBrowser()) return + // `page` and `getCurrentTest` come from a dynamic import; the hooks that + // follow read them synchronously, so wait for it here (#835). + await whenVitestProxyReady() matcher = autoSnapshotMatcher(commands, expect) const suiteDefaults = options?.createMissingBaseline !== undefined