fix(storybook-addon-vis): await the vitest module load before using the proxies

.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.

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<T>() {
+		let resolve!: (value: T) => void
+		const promise = new Promise<T>((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<any>()
+		const vitest = deferred<any>()
+		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')
+	})
+})

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<typeof import('vitest/browser')>
-let vitest: Awaited<typeof import('vitest')>
+type BrowserModule = Awaited<typeof import('vitest/browser')>
+type VitestModule = Awaited<typeof import('vitest')>
 
-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<BrowserModule>
+	loadVitest: () => Promise<VitestModule>
+}
+
+/**
+ * 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<BrowserPage>({} 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<BrowserCommands>({} 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 `<command> is not a function`.
+			return (...args: unknown[]) => browserReady.then(() => (browserContext!.commands as any)[prop](...args))
+		},
 	})
+
+	const getCurrentTest = () =>
+		vitest?.TestRunner.getCurrentTest() as
+			| (ReturnType<VitestModule['TestRunner']['getCurrentTest']> & 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<BrowserPage>({} 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<BrowserCommands>({} as any, {
-	get(_target, prop) {
-		return (browserContext?.commands as any)?.[prop]
-	},
-})
-
-export const getCurrentTest = () =>
-	vitest?.TestRunner.getCurrentTest() as
-		| (ReturnType<typeof vitest.TestRunner.getCurrentTest> & 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

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