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