diff --git a/packages/@overeng/effect-schema-form-aria/vitest.gate.config.ts b/packages/@overeng/effect-schema-form-aria/vitest.gate.config.ts index 93e67267bd..3da21cdf62 100644 --- a/packages/@overeng/effect-schema-form-aria/vitest.gate.config.ts +++ b/packages/@overeng/effect-schema-form-aria/vitest.gate.config.ts @@ -1,4 +1,4 @@ -import { defineConfig, mergeConfig } from 'vitest/config' +import { defineConfig } from 'vitest/config' import { createStoryGateConfig } from '@overeng/utils/node/storybook/gate' import { createStylexVitePlugins } from '@overeng/utils/node/stylex' @@ -7,14 +7,18 @@ import { createStylexVitePlugins } from '@overeng/utils/node/stylex' // on demand and never committed, so this config needs the runner to tell it // which ref-derived directory to compare against. // -// The StyleX plugin has to be here rather than inherited. Vitest treats this -// file as the Vite config for the run, so `vite.config.ts` is never loaded — -// unlike a Storybook build, where the builder merges it. Without the compiler -// transform the stories throw `Unexpected 'stylex.keyframes' call at runtime`. -// No `entries`: the virtual stylesheet is a build-only concern and this run is -// served, not bundled. +// The StyleX plugin goes through `plugins` rather than being merged into the +// returned config. Vitest treats this file as the Vite config for the run, so +// `vite.config.ts` is never loaded — unlike a Storybook build, where the builder +// merges it — and without the compiler transform the stories throw +// `Unexpected 'stylex.keyframes' call at runtime`. Merging at the root happened +// to work here only because this package declares no themes, so the factory +// returned a bare project config and the merge target *was* the project; the +// same code silently stopped applying for a two-theme consumer. No `entries`: +// the virtual stylesheet is a build-only concern and this run is served, not +// bundled. export default defineConfig( - mergeConfig(createStoryGateConfig({}), { + createStoryGateConfig({ plugins: [createStylexVitePlugins({ useCSSLayers: { before: ['overeng.reset'] } })], }), ) diff --git a/packages/@overeng/utils/src/node/storybook/gate/project.ts b/packages/@overeng/utils/src/node/storybook/gate/project.ts index 4b2ca98c80..3bde3f0127 100644 --- a/packages/@overeng/utils/src/node/storybook/gate/project.ts +++ b/packages/@overeng/utils/src/node/storybook/gate/project.ts @@ -49,6 +49,24 @@ export interface StoryGateConfigOptions { readonly themes?: readonly StoryGateTheme[] /** @default true */ readonly headless?: boolean + /** + * Vite plugins the stories need in order to load, e.g. a compiler transform. + * + * These MUST arrive here rather than being merged into the returned config, + * because **Vitest projects do not inherit root-level `plugins`** — each + * project is its own Vite config. Merging at the root works only by accident + * in the single-theme case, where this factory returns a bare project config + * and the caller's merge target *is* the project. Add a second theme and the + * same merge silently stops applying: the return shape becomes + * `{ test: { projects } }`, the plugins land beside `test` where nothing reads + * them, and every story fails to load with a runtime error from the + * untransformed source. + * + * That failure was reproduced end to end: one-theme consumer ran 4 stories, + * two-theme consumer reported `Tests no tests`, and threading the same + * plugins through this option moved it to 10 stories executed. + */ + readonly plugins?: ViteUserConfig['plugins'] } const readBaselineRoot = (): string => { @@ -67,27 +85,48 @@ const recordResolvedBaseline = (path: string): void => { appendFileSync(manifest, `${path}\n`) } +/** + * Build the Storybook test plugin for one project. + * + * Injected rather than called inline so the plugin-placement invariant is + * unit-testable. `storybookTest` eagerly loads a real Storybook config + * directory, so a test that invoked it would need a Storybook install and a + * fixture `main.ts` in a package that has neither — and the thing worth + * guarding is *where the plugins land*, which is independent of what they are. + */ +export type StorybookPluginFor = (args: { + configDir: string + theme: StoryGateTheme | undefined +}) => NonNullable[number] + +const defaultStorybookPluginFor: StorybookPluginFor = ({ configDir, theme }) => + storybookTest({ + configDir, + ...(theme === undefined ? {} : { initialGlobals: { [theme.name]: theme.value } }), + }) + const createProject = ({ configDir, theme, headless, baselineRoot, + plugins, + storybookPluginFor, }: { configDir: string theme: StoryGateTheme | undefined headless: boolean baselineRoot: string + plugins: ViteUserConfig['plugins'] + storybookPluginFor: StorybookPluginFor }): ViteUserConfig => { const projectName = theme === undefined ? 'story-gate' : `story-gate-${theme.value}` const baselineDir = join(baselineRoot, projectName) return { - plugins: [ - storybookTest({ - configDir, - ...(theme === undefined ? {} : { initialGlobals: { [theme.name]: theme.value } }), - }), - ], + // Caller plugins come first: a compiler transform has to see the source + // before the Storybook plugin turns it into a test module. + plugins: [...(plugins ?? []), storybookPluginFor({ configDir, theme })], // The baseline half of a run happens inside a git worktree that borrows the // main tree's `node_modules` by symlink, so workspace sources — this gate's // own setup file among them — resolve to paths outside the served root and @@ -155,10 +194,15 @@ export const createStoryGateConfig = ({ configDir = '.storybook', themes, headless = true, -}: StoryGateConfigOptions = {}): ViteUserConfig => { + plugins, + storybookPluginFor = defaultStorybookPluginFor, +}: StoryGateConfigOptions & { + /** Seam for unit tests; production callers never pass this. */ + readonly storybookPluginFor?: StorybookPluginFor +} = {}): ViteUserConfig => { const baselineRoot = readBaselineRoot() const projects = (themes ?? [undefined]).map((theme) => - createProject({ configDir, theme, headless, baselineRoot }), + createProject({ configDir, theme, headless, baselineRoot, plugins, storybookPluginFor }), ) return projects.length === 1 && projects[0] !== undefined ? projects[0] : { test: { projects } } diff --git a/packages/@overeng/utils/src/node/storybook/gate/project.unit.test.ts b/packages/@overeng/utils/src/node/storybook/gate/project.unit.test.ts new file mode 100644 index 0000000000..0c63979bfd --- /dev/null +++ b/packages/@overeng/utils/src/node/storybook/gate/project.unit.test.ts @@ -0,0 +1,130 @@ +import { beforeAll, describe, expect, it } from 'vitest' + +import { baselineDirEnvVar, createStoryGateConfig } from './project.ts' + +/** + * These tests exist because of a defect that was invisible to every local + * check: a consumer merging its plugins into the returned config got a working + * gate with one theme and a gate that could not load a single story with two. + * + * The asymmetry is the whole bug. So each assertion is written against **both** + * return shapes, and the one-theme case is not treated as the trivial case. + */ + +const marker = { name: 'test-marker-plugin' } + +/** + * Stand-in for the Storybook plugin. + * + * The real one eagerly loads a Storybook config directory, which this package + * does not have and should not need in order to check where plugins land. + */ +const fakeStorybookPluginFor = () => ({ name: 'fake-storybook-test' }) + +/** Read a `plugins` member without asserting a shape onto the value. */ +const readPlugins = (value: unknown): unknown => + value !== null && typeof value === 'object' && 'plugins' in value ? value.plugins : undefined + +/** Plugin names on a config or project, flattened — Vite accepts nested arrays. */ +const pluginNames = (value: unknown): readonly string[] => { + const plugins = readPlugins(value) + if (Array.isArray(plugins) === false) return [] + + const names: string[] = [] + for (const plugin of plugins.flat(Number.POSITIVE_INFINITY)) { + if (plugin === null || typeof plugin !== 'object' || 'name' in plugin === false) continue + const { name } = plugin + if (typeof name === 'string') names.push(name) + } + return names +} + +/** The projects array, or a thrown error naming what was found instead. */ +const projectsOf = (config: unknown): readonly unknown[] => { + const projects = + config !== null && typeof config === 'object' && 'test' in config + ? readProjects(config.test) + : undefined + if (Array.isArray(projects) === false) { + throw new Error(`expected a projects array, got ${JSON.stringify(projects)}`) + } + return projects +} + +const readProjects = (value: unknown): unknown => + value !== null && typeof value === 'object' && 'projects' in value ? value.projects : undefined + +beforeAll(() => { + process.env[baselineDirEnvVar] = '/tmp/story-gate-unit-test' +}) + +describe('createStoryGateConfig plugin threading', () => { + it('puts caller plugins on the single project when there are no themes', () => { + const config = createStoryGateConfig({ plugins: [marker], storybookPluginFor: fakeStorybookPluginFor }) + + // No themes returns a bare project config rather than a projects array, so + // the plugins belong at this level. + expect(config.test?.projects).toBeUndefined() + expect(pluginNames(config)).toContain(marker.name) + }) + + it('puts caller plugins on EVERY project when there are themes', () => { + const config = createStoryGateConfig({ + themes: [ + { name: 'theme', value: 'light' }, + { name: 'theme', value: 'dark' }, + ], + plugins: [marker], + storybookPluginFor: fakeStorybookPluginFor, + }) + + const projects = projectsOf(config) + expect(projects).toHaveLength(2) + + // The defect this guards: plugins reaching the root of a projects-shaped + // config, where no project reads them. Asserting on the root would pass + // while the gate loaded nothing, so the assertion has to be per project. + for (const project of projects) { + expect(pluginNames(project)).toContain(marker.name) + } + + // And the root must NOT be where they live, or a future refactor could + // satisfy the per-project assertion by duplicating them everywhere. + expect(pluginNames(config)).not.toContain(marker.name) + }) + + it('orders caller plugins before the Storybook plugin', () => { + // A compiler transform has to see the source before the Storybook plugin + // turns it into a test module. Order is behaviour here, not style. + const names = pluginNames(createStoryGateConfig({ plugins: [marker], storybookPluginFor: fakeStorybookPluginFor })) + + const markerIndex = names.indexOf(marker.name) + const storybookIndex = names.findIndex((name) => name.includes('storybook')) + + expect(markerIndex).toBeGreaterThanOrEqual(0) + expect(storybookIndex).toBeGreaterThanOrEqual(0) + expect(markerIndex).toBeLessThan(storybookIndex) + }) + + it('still builds both projects when no plugins are supplied', () => { + // Negative control: the threading must not make `plugins` load-bearing for + // consumers that need no transform, and an empty spread must not shift the + // Storybook plugin out of the array. + const projects = projectsOf( + createStoryGateConfig({ + themes: [ + { name: 'theme', value: 'light' }, + { name: 'theme', value: 'dark' }, + ], + storybookPluginFor: fakeStorybookPluginFor, + }), + ) + expect(projects).toHaveLength(2) + + for (const project of projects) { + const names = pluginNames(project) + expect(names.some((name) => name.includes('storybook'))).toBe(true) + expect(names).not.toContain(marker.name) + } + }) +})