From 2f7c1c6271e01e43cd18986935c54451ce73b78b Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Wed, 18 Mar 2026 11:07:27 +0100 Subject: [PATCH 1/2] Addon-Vitest: Streamline vite(st) config detection across init and postinstall script --- code/addons/vitest/src/postinstall.ts | 1 - code/addons/vitest/src/updateVitestFile.ts | 123 +------ code/core/src/babel/index.ts | 9 + .../src/babel/vitest-config-helpers.test.ts | 334 ++++++++++++++++++ code/core/src/babel/vitest-config-helpers.ts | 237 +++++++++++++ code/core/src/cli/AddonVitestService.test.ts | 134 ++++++- code/core/src/cli/AddonVitestService.ts | 108 +----- 7 files changed, 721 insertions(+), 225 deletions(-) create mode 100644 code/core/src/babel/vitest-config-helpers.test.ts create mode 100644 code/core/src/babel/vitest-config-helpers.ts diff --git a/code/addons/vitest/src/postinstall.ts b/code/addons/vitest/src/postinstall.ts index f11c43eeff99..7c426e701c9e 100644 --- a/code/addons/vitest/src/postinstall.ts +++ b/code/addons/vitest/src/postinstall.ts @@ -1,4 +1,3 @@ -import { existsSync } from 'node:fs'; import * as fs from 'node:fs/promises'; import { writeFile } from 'node:fs/promises'; import os from 'node:os'; diff --git a/code/addons/vitest/src/updateVitestFile.ts b/code/addons/vitest/src/updateVitestFile.ts index 8aa792b52a1e..8e1a80062f38 100644 --- a/code/addons/vitest/src/updateVitestFile.ts +++ b/code/addons/vitest/src/updateVitestFile.ts @@ -1,4 +1,10 @@ -import { resolveExpression } from 'storybook/internal/babel'; +import { + getConfigObjectFromMergeArg, + getEffectiveMergeConfigCall, + getTargetConfigObject, + isDefineConfigLike, + resolveExpression, +} from 'storybook/internal/babel'; import type { BabelFile, types as t } from 'storybook/internal/babel'; import { normalize } from 'pathe'; @@ -69,56 +75,8 @@ const mergeProperties = ( } }; -/** - * Returns true if the identifier is a local alias for `defineConfig`/`defineProject` imported from - * either `vitest/config` or `vite`. - */ -const isImportedDefineConfigLikeIdentifier = (localName: string, ast: BabelFile['ast']): boolean => - ast.program.body.some( - (node): boolean => - node.type === 'ImportDeclaration' && - (node.source.value === 'vitest/config' || node.source.value === 'vite') && - node.specifiers.some( - (specifier) => - specifier.type === 'ImportSpecifier' && - specifier.local.type === 'Identifier' && - specifier.local.name === localName && - specifier.imported.type === 'Identifier' && - (specifier.imported.name === 'defineConfig' || - specifier.imported.name === 'defineProject') - ) - ); - -/** Returns true if the call expression is a defineConfig or defineProject call (including aliases). */ -const isDefineConfigLike = (node: t.CallExpression, ast: BabelFile['ast']): boolean => - node.callee.type === 'Identifier' && - (node.callee.name === 'defineConfig' || - node.callee.name === 'defineProject' || - isImportedDefineConfigLikeIdentifier(node.callee.name, ast)); - -/** - * Resolves a mergeConfig argument to a config object expression when possible. Supports both direct - * object args and wrapped forms like `defineConfig({ ... })`. - */ -const getConfigObjectFromMergeArg = ( - arg: t.Expression, - ast: BabelFile['ast'] -): t.ObjectExpression | null => { - const resolved = resolveExpression(arg, ast); - if (!resolved) { - return null; - } - - if (resolved.type === 'ObjectExpression') { - return resolved; - } - - if (resolved.type === 'CallExpression' && resolved.arguments[0]?.type === 'ObjectExpression') { - return resolved.arguments[0] as t.ObjectExpression; - } - - return null; -}; +// isDefineConfigLike, getConfigObjectFromMergeArg, getEffectiveMergeConfigCall, and +// getTargetConfigObject are imported from 'storybook/internal/babel' (vitest-config-helpers) /** * Resolves the value of a `test` ObjectProperty to an ObjectExpression. Handles both inline objects @@ -368,69 +326,6 @@ const mergeTemplateIntoConfigObject = ( mergeProperties(properties, targetConfigObject.properties); }; -/** - * Extracts the effective mergeConfig call from a declaration, handling wrappers: - * - * - TypeScript type annotations (as X, satisfies X) - * - DefineConfig(mergeConfig(...)) outer wrapper - * - Variable references (export default config where config = mergeConfig(...)) - */ -const getEffectiveMergeConfigCall = ( - decl: t.Expression | t.Declaration, - ast: BabelFile['ast'] -): t.CallExpression | null => { - const resolved = resolveExpression(decl, ast); - if (!resolved || resolved.type !== 'CallExpression') { - return null; - } - - // Handle defineConfig(mergeConfig(...)) – arg may itself be wrapped in a TS type expression - if (isDefineConfigLike(resolved, ast) && resolved.arguments.length > 0) { - const innerArg = resolveExpression(resolved.arguments[0] as t.Expression, ast); - if ( - innerArg?.type === 'CallExpression' && - innerArg.callee.type === 'Identifier' && - innerArg.callee.name === 'mergeConfig' - ) { - return innerArg; - } - } - - // Handle mergeConfig(...) directly - if (resolved.callee.type === 'Identifier' && resolved.callee.name === 'mergeConfig') { - return resolved; - } - - return null; -}; - -/** - * Resolves the target's default export to the actual config object expression we can merge into. - * Handles: export default defineConfig({}), export default defineProject({}), export default {}, - * and export default config (where config is a variable holding one of those), as well as - * TypeScript type annotations on the declaration. - */ -const getTargetConfigObject = ( - target: BabelFile['ast'], - exportDefault: t.ExportDefaultDeclaration -): t.ObjectExpression | null => { - const resolved = resolveExpression(exportDefault.declaration, target); - if (!resolved) { - return null; - } - if (resolved.type === 'ObjectExpression') { - return resolved; - } - if ( - resolved.type === 'CallExpression' && - isDefineConfigLike(resolved, target) && - resolved.arguments[0]?.type === 'ObjectExpression' - ) { - return resolved.arguments[0] as t.ObjectExpression; - } - return null; -}; - /** * Merges a source Vitest configuration AST into a target configuration AST. * diff --git a/code/core/src/babel/index.ts b/code/core/src/babel/index.ts index 79e6fdd08d73..018113ec0c97 100644 --- a/code/core/src/babel/index.ts +++ b/code/core/src/babel/index.ts @@ -15,6 +15,15 @@ import * as recast from 'recast'; export * from './babelParse'; export { unwrapTSExpression, resolveExpression } from './expression-resolver'; +export { + isImportedDefineConfigLikeIdentifier, + isDefineConfigLike, + getConfigObjectFromMergeArg, + getEffectiveMergeConfigCall, + getTargetConfigObject, + canUpdateVitestConfigFile, + canUpdateVitestWorkspaceFile, +} from './vitest-config-helpers'; // @ts-expect-error (needed due to it's use of `exports.default`) const traverse = (bt.default || bt) as typeof bt; diff --git a/code/core/src/babel/vitest-config-helpers.test.ts b/code/core/src/babel/vitest-config-helpers.test.ts new file mode 100644 index 000000000000..f3d11bf3498e --- /dev/null +++ b/code/core/src/babel/vitest-config-helpers.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, it } from 'vitest'; + +import { canUpdateVitestConfigFile, canUpdateVitestWorkspaceFile } from './vitest-config-helpers'; + +describe('canUpdateVitestConfigFile', () => { + it('returns true for plain export default object literal', () => { + expect(canUpdateVitestConfigFile('export default { test: { name: "node" } }')).toBe(true); + }); + + it('returns true for bare export default {}', () => { + expect(canUpdateVitestConfigFile('export default {}')).toBe(true); + }); + + it('returns true for defineConfig({}) pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig } from 'vitest/config'; + export default defineConfig({ test: { environment: 'happy-dom' } });` + ) + ).toBe(true); + }); + + it('returns true for defineProject({}) pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineProject } from 'vitest/config'; + export default defineProject({ test: { environment: 'happy-dom' } });` + ) + ).toBe(true); + }); + + it('returns true for defineProject from vitest/config', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineProject } from 'vitest/config'; + + export default defineProject({ + test: { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }, + }) + ` + ) + ).toBe(true); + }); + + it('returns true for simple mergeConfig({}, {}) pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + export default mergeConfig(viteConfig, { test: { name: 'node' } });` + ) + ).toBe(true); + }); + + it('returns true for defineConfig(mergeConfig(...)) pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig, mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + + export default defineConfig( + mergeConfig(viteConfig, { + test: { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }, + }) + ) + ` + ) + ).toBe(true); + }); + + it('returns true for defineConfig(mergeConfig(...) satisfies ViteUserConfig) pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig, mergeConfig } from 'vitest/config'; + import type { ViteUserConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + + export default defineConfig( + mergeConfig(viteConfig, { + test: { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }, + }) satisfies ViteUserConfig + ) + ` + ) + ).toBe(true); + }); + + it('returns true for mergeConfig(...) as ViteUserConfig pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { mergeConfig } from 'vitest/config'; + import type { ViteUserConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + + export default mergeConfig(viteConfig, { + test: { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }, + }) as ViteUserConfig + ` + ) + ).toBe(true); + }); + + it('returns true for mergeConfig with shorthand test property (const test = {...})', () => { + expect( + canUpdateVitestConfigFile( + ` + import { mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + + const test = { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }; + + export default mergeConfig(viteConfig, { + test, + }) + ` + ) + ).toBe(true); + }); + + // Real-life example: xmtp-js (constant vitestConfig) + it('returns true for mergeConfig with external vitestConfig variable', () => { + expect( + canUpdateVitestConfigFile( + ` + import { mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + + const vitestConfig = { + test: { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }, + }; + + export default mergeConfig(viteConfig, vitestConfig) + ` + ) + ).toBe(true); + }); + + it('returns true for const config = mergeConfig(...); export default config pattern', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig, mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + + const config = mergeConfig( + viteConfig, + defineConfig({ + test: { + name: 'node', + environment: 'happy-dom', + include: ['**/*.test.ts'], + }, + }) + ); + + export default config + ` + ) + ).toBe(true); + }); + + it('returns true for export default config where config = defineConfig({...})', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig } from 'vitest/config'; + + const config = defineConfig({ test: { name: 'node' } }); + export default config + ` + ) + ).toBe(true); + }); + + it('returns true for defineConfig({}) as UserWorkspaceConfig', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig } from 'vitest/config'; + import type { UserWorkspaceConfig } from 'vitest/config'; + + export default defineConfig({ test: {} }) as UserWorkspaceConfig + ` + ) + ).toBe(true); + }); + + it('returns true for defineConfig({}) satisfies UserWorkspaceConfig', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig } from 'vitest/config'; + import type { UserWorkspaceConfig } from 'vitest/config'; + + export default defineConfig({ test: {} }) satisfies UserWorkspaceConfig + ` + ) + ).toBe(true); + }); + + it('returns true for defineConfig aliased to a custom name', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig as dc } from 'vitest/config'; + export default dc({ test: {} }) + ` + ) + ).toBe(true); + }); + + // ----- Unsupported patterns (should return false) ----- + + it('returns false when there is no export default', () => { + expect(canUpdateVitestConfigFile('const x = 1;')).toBe(false); + }); + + it('returns false for arrow function pattern: defineConfig(() => ({}))', () => { + expect( + canUpdateVitestConfigFile( + ` + import { defineConfig } from 'vitest/config'; + export default defineConfig(() => ({ test: {} })) + ` + ) + ).toBe(false); + }); + + it('returns false for unrecognizable export (string literal)', () => { + expect(canUpdateVitestConfigFile("export default 'something'")).toBe(false); + }); + + it('returns false for syntax errors', () => { + expect(canUpdateVitestConfigFile('this is not valid syntax !!!')).toBe(false); + }); + + it('returns false for export default function declaration', () => { + expect(canUpdateVitestConfigFile('export default function config() { return {}; }')).toBe( + false + ); + }); +}); + +// ─── canUpdateVitestWorkspaceFile ──────────────────────────────────────────── + +describe('canUpdateVitestWorkspaceFile', () => { + // ----- Supported patterns (should return true) ----- + + it('returns true for plain array workspace', () => { + expect(canUpdateVitestWorkspaceFile('export default ["project1", "project2"]')).toBe(true); + }); + + it('returns true for array with object entries', () => { + expect(canUpdateVitestWorkspaceFile('export default [{ test: {} }, "project"]')).toBe(true); + }); + + it('returns true for empty array', () => { + expect(canUpdateVitestWorkspaceFile('export default []')).toBe(true); + }); + + it('returns true for defineWorkspace([...]) pattern', () => { + expect( + canUpdateVitestWorkspaceFile( + ` + import { defineWorkspace } from 'vitest/config'; + export default defineWorkspace(['project1', 'project2']) + ` + ) + ).toBe(true); + }); + + it('returns true for defineWorkspace with object entries', () => { + expect( + canUpdateVitestWorkspaceFile( + ` + import { defineWorkspace } from 'vitest/config'; + export default defineWorkspace([{ test: { name: 'unit' } }]) + ` + ) + ).toBe(true); + }); + + // ----- Unsupported patterns (should return false) ----- + + it('returns false when there is no export default', () => { + expect(canUpdateVitestWorkspaceFile('const projects = ["a"];')).toBe(false); + }); + + it('returns false for defineWorkspace with a non-array argument', () => { + expect( + canUpdateVitestWorkspaceFile( + ` + import { defineWorkspace } from 'vitest/config'; + export default defineWorkspace('glob/**') + ` + ) + ).toBe(false); + }); + + it('returns false for syntax errors', () => { + expect(canUpdateVitestWorkspaceFile('this is not valid syntax !!!')).toBe(false); + }); +}); diff --git a/code/core/src/babel/vitest-config-helpers.ts b/code/core/src/babel/vitest-config-helpers.ts new file mode 100644 index 000000000000..b1e106a34cb9 --- /dev/null +++ b/code/core/src/babel/vitest-config-helpers.ts @@ -0,0 +1,237 @@ +/** + * Shared AST helpers for analyzing and determining whether a Vitest/Vite config file can be + * auto-updated by Storybook. Extracted from the addon-vitest postinstall logic so the same + * capability checks are used during `storybook init` (AddonVitestService) and during the actual + * config update (updateVitestFile). + */ +import type * as t from '@babel/types'; + +import { babelParse } from './babelParse'; +import { resolveExpression } from './expression-resolver'; + +type AST = t.File; + +/** + * Returns true if the identifier `localName` is a local alias for `defineConfig` or `defineProject` + * imported from either `vitest/config` or `vite`. + */ +export const isImportedDefineConfigLikeIdentifier = (localName: string, ast: AST): boolean => + ast.program.body.some( + (node): boolean => + node.type === 'ImportDeclaration' && + (node.source.value === 'vitest/config' || node.source.value === 'vite') && + node.specifiers.some( + (specifier) => + specifier.type === 'ImportSpecifier' && + specifier.local.type === 'Identifier' && + specifier.local.name === localName && + specifier.imported.type === 'Identifier' && + (specifier.imported.name === 'defineConfig' || + specifier.imported.name === 'defineProject') + ) + ); + +/** Returns true if a CallExpression is a call to defineConfig or defineProject (or an alias). */ +export const isDefineConfigLike = (node: t.CallExpression, ast: AST): boolean => + node.callee.type === 'Identifier' && + (node.callee.name === 'defineConfig' || + node.callee.name === 'defineProject' || + isImportedDefineConfigLikeIdentifier(node.callee.name, ast)); + +/** + * Resolves a `mergeConfig` argument to an ObjectExpression when possible. Handles: + * + * - Plain object literals: `{ test: {...} }` + * - Variable references: `const vitestConfig = {...}; mergeConfig(viteConfig, vitestConfig)` + * - Wrapped calls (e.g. `defineConfig({ ... })`): returns the inner ObjectExpression + * - TypeScript type annotations: `mergeConfig(...) as ViteUserConfig` + */ +export const getConfigObjectFromMergeArg = ( + arg: t.Expression, + ast: AST +): t.ObjectExpression | null => { + const resolved = resolveExpression(arg, ast); + if (!resolved) { + return null; + } + + if (resolved.type === 'ObjectExpression') { + return resolved; + } + + if (resolved.type === 'CallExpression' && resolved.arguments[0]?.type === 'ObjectExpression') { + return resolved.arguments[0] as t.ObjectExpression; + } + + return null; +}; + +/** + * Extracts the effective `mergeConfig(...)` call from a declaration, unwrapping: + * + * - TypeScript type annotations (`as X`, `satisfies X`) + * - `defineConfig(mergeConfig(...))` outer wrapper + * - Variable references (`export default config` where `config = mergeConfig(...)`) + * + * Returns null when the declaration is not or does not contain a `mergeConfig` call. + */ +export const getEffectiveMergeConfigCall = ( + decl: t.Expression | t.Declaration, + ast: AST +): t.CallExpression | null => { + const resolved = resolveExpression(decl, ast); + if (!resolved || resolved.type !== 'CallExpression') { + return null; + } + + // Handle defineConfig(mergeConfig(...)) – inner arg may itself be TS-wrapped + if (isDefineConfigLike(resolved, ast) && resolved.arguments.length > 0) { + const innerArg = resolveExpression(resolved.arguments[0] as t.Expression, ast); + if ( + innerArg?.type === 'CallExpression' && + innerArg.callee.type === 'Identifier' && + innerArg.callee.name === 'mergeConfig' + ) { + return innerArg; + } + } + + // Handle mergeConfig(...) directly + if (resolved.callee.type === 'Identifier' && resolved.callee.name === 'mergeConfig') { + return resolved; + } + + return null; +}; + +/** + * Resolves the default export to the actual config ObjectExpression we can merge into. Handles: + * + * - `export default { ... }` — plain object + * - `export default defineConfig({ ... })` — defineConfig with object + * - `export default defineProject({ ... })` — defineProject with object + * - `export default config` — variable reference to any of the above + * - Any of the above wrapped in TypeScript type annotations + * + * Returns null when a writable ObjectExpression cannot be located. + */ +export const getTargetConfigObject = ( + target: AST, + exportDefault: t.ExportDefaultDeclaration +): t.ObjectExpression | null => { + const resolved = resolveExpression(exportDefault.declaration, target); + if (!resolved) { + return null; + } + if (resolved.type === 'ObjectExpression') { + return resolved; + } + if ( + resolved.type === 'CallExpression' && + isDefineConfigLike(resolved, target) && + resolved.arguments[0]?.type === 'ObjectExpression' + ) { + return resolved.arguments[0] as t.ObjectExpression; + } + return null; +}; + +/** + * Returns `true` when a Vitest/Vite config file's default export uses a pattern that + * `updateConfigFile` (from addon-vitest) can auto-update. + * + * Supported patterns: + * + * - `export default { test: {...} }` + * - `export default defineConfig({ ... })` / `defineProject({ ... })` + * - `export default mergeConfig(viteConfig, { ... })` + * - `export default mergeConfig(viteConfig, defineConfig({ ... }))` + * - `export default defineConfig(mergeConfig(...))` + * - `export default config` (variable referencing any of the above) + * - Any of the above wrapped in `as X` or `satisfies X` TypeScript annotations + * + * Unsupported patterns (returns `false`): + * + * - `export default defineConfig(() => ({ ... }))` (arrow function) + * - Completely unrecognizable export shapes + * - No `export default` declaration at all + */ +export const canUpdateVitestConfigFile = (fileContent: string): boolean => { + let parsedAst: AST; + try { + parsedAst = babelParse(fileContent); + } catch { + return false; + } + + const exportDefault = parsedAst.program.body.find( + (n): n is t.ExportDefaultDeclaration => n.type === 'ExportDefaultDeclaration' + ); + if (!exportDefault) { + return false; + } + + // Reject arrow function pattern: defineConfig(() => ({...})) + const effectiveDecl = resolveExpression(exportDefault.declaration, parsedAst); + if ( + effectiveDecl?.type === 'CallExpression' && + isDefineConfigLike(effectiveDecl, parsedAst) && + effectiveDecl.arguments.length > 0 && + effectiveDecl.arguments[0].type === 'ArrowFunctionExpression' + ) { + return false; + } + + return ( + getTargetConfigObject(parsedAst, exportDefault) !== null || + getEffectiveMergeConfigCall(exportDefault.declaration, parsedAst) !== null + ); +}; + +/** + * Returns `true` when a Vitest workspace file's default export uses a pattern that + * `updateWorkspaceFile` (from addon-vitest) can auto-update. + * + * Supported patterns: + * + * - `export default ["project1", "project2"]` + * - `export default [{...}, "project"]` + * - `export default defineWorkspace(["project1", "project2"])` + * + * Returns `false` for any other shape (e.g. JSON files should be rejected before parsing, CommonJS + * files, unrecognizable exports). + */ +export const canUpdateVitestWorkspaceFile = (fileContent: string): boolean => { + let parsedAst: AST; + try { + parsedAst = babelParse(fileContent); + } catch { + return false; + } + + const exportDefault = parsedAst.program.body.find( + (n): n is t.ExportDefaultDeclaration => n.type === 'ExportDefaultDeclaration' + ); + if (!exportDefault) { + return false; + } + + const decl = exportDefault.declaration; + + // export default [...] + if (decl.type === 'ArrayExpression') { + return true; + } + + // export default defineWorkspace([...]) + if ( + decl.type === 'CallExpression' && + decl.callee.type === 'Identifier' && + decl.callee.name === 'defineWorkspace' && + decl.arguments[0]?.type === 'ArrayExpression' + ) { + return true; + } + + return false; +}; diff --git a/code/core/src/cli/AddonVitestService.test.ts b/code/core/src/cli/AddonVitestService.test.ts index e54f5b54f907..b19693937070 100644 --- a/code/core/src/cli/AddonVitestService.test.ts +++ b/code/core/src/cli/AddonVitestService.test.ts @@ -672,7 +672,7 @@ describe('AddonVitestService', () => { expect(result.compatible).toBe(true); }); - it('should reject invalid vitest config', async () => { + it('should accept plain export default {} (now supported)', async () => { vi.mocked(find.any) .mockReturnValueOnce(undefined) // workspace .mockReturnValueOnce('vitest.config.ts'); // config @@ -680,6 +680,20 @@ describe('AddonVitestService', () => { const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); + }); + + it('should reject arrow function vitest config (unsupported)', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + `import { defineConfig } from 'vitest/config'; +export default defineConfig(() => ({ test: {} }))` + ); + + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(false); expect(result.reasons!.some((r) => r.includes('invalid Vitest config'))).toBe(true); }); @@ -761,14 +775,124 @@ describe('AddonVitestService', () => { expect(result.compatible).toBe(true); }); - it('should reject mergeConfig with invalid object (non-object argument)', async () => { + it('should accept defineConfig(mergeConfig(...)) pattern', async () => { vi.mocked(find.any) .mockReturnValueOnce(undefined) // workspace .mockReturnValueOnce('vitest.config.ts'); // config - vi.mocked(fs.readFile).mockResolvedValue('export default mergeConfig(viteConfig, "string")'); + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { defineConfig, mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + export default defineConfig( + mergeConfig(viteConfig, { + test: { name: 'node', environment: 'happy-dom' }, + }) + )` + ); const result = await service.validateConfigFiles('.storybook'); - expect(result.compatible).toBe(false); - expect(result.reasons!.some((r) => r.includes('invalid Vitest config'))).toBe(true); + expect(result.compatible).toBe(true); + }); + + it('should accept defineConfig(mergeConfig(...) satisfies ViteUserConfig) pattern', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { defineConfig, mergeConfig } from 'vitest/config'; + import type { ViteUserConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + export default defineConfig( + mergeConfig(viteConfig, { + test: { name: 'node' }, + }) satisfies ViteUserConfig + )` + ); + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); + }); + + it('should accept mergeConfig(...) as ViteUserConfig pattern', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { mergeConfig } from 'vitest/config'; + import type { ViteUserConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + export default mergeConfig(viteConfig, { + test: { name: 'node' }, + }) as ViteUserConfig` + ); + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); + }); + + it('should accept mergeConfig with shorthand test variable', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + const test = { name: 'node', environment: 'happy-dom' }; + export default mergeConfig(viteConfig, { test })` + ); + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); + }); + + // Real-life pattern: xmtp-js — const vitestConfig = {...}; mergeConfig(vite, vitestConfig) + it('should accept mergeConfig with external vitestConfig variable', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + const vitestConfig = { test: { name: 'node' } }; + export default mergeConfig(viteConfig, vitestConfig)` + ); + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); + }); + + // Real-life pattern: lynx-stack — const config = mergeConfig(...); export default config + it('should accept const config = mergeConfig(...); export default config pattern', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { defineConfig, mergeConfig } from 'vitest/config'; + import viteConfig from './vite.config'; + const config = mergeConfig( + viteConfig, + defineConfig({ test: { name: 'node' } }) + ); + export default config` + ); + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); + }); + + // Real-life pattern: meilisearch-js-plugins — defineProject + it('should accept defineProject({}) pattern', async () => { + vi.mocked(find.any) + .mockReturnValueOnce(undefined) // workspace + .mockReturnValueOnce('vitest.config.ts'); // config + vi.mocked(fs.readFile).mockResolvedValue( + ` + import { defineProject } from 'vitest/config'; + export default defineProject({ + test: { name: 'node', environment: 'happy-dom' }, + })` + ); + const result = await service.validateConfigFiles('.storybook'); + expect(result.compatible).toBe(true); }); }); }); diff --git a/code/core/src/cli/AddonVitestService.ts b/code/core/src/cli/AddonVitestService.ts index 9fd7fc8c964f..9a49129e48d2 100644 --- a/code/core/src/cli/AddonVitestService.ts +++ b/code/core/src/cli/AddonVitestService.ts @@ -1,14 +1,13 @@ import fs from 'node:fs/promises'; import os from 'node:os'; -import * as babel from 'storybook/internal/babel'; +import { canUpdateVitestConfigFile, canUpdateVitestWorkspaceFile } from 'storybook/internal/babel'; import type { JsPackageManager } from 'storybook/internal/common'; import { getProjectRoot } from 'storybook/internal/common'; import { CLI_COLORS } from 'storybook/internal/node-logger'; import { logger, prompt } from 'storybook/internal/node-logger'; import { ErrorCollector } from 'storybook/internal/telemetry'; -import type { CallExpression } from '@babel/types'; import * as find from 'empathic/find'; import { coerce, minVersion, satisfies, validRange } from 'semver'; import { dedent } from 'ts-dedent'; @@ -299,7 +298,7 @@ export class AddonVitestService { reasons.push(`Cannot auto-update JSON workspace file: ${vitestWorkspaceFile}`); } else if (vitestWorkspaceFile) { const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8'); - if (!this.isValidWorkspaceConfigFile(fileContents)) { + if (!canUpdateVitestWorkspaceFile(fileContents)) { reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`); } } @@ -314,112 +313,11 @@ export class AddonVitestService { reasons.push(`Cannot auto-update CommonJS config file: ${vitestConfigFile}`); } else if (vitestConfigFile) { const configContent = await fs.readFile(vitestConfigFile, 'utf8'); - if (!this.isValidVitestConfig(configContent)) { + if (!canUpdateVitestConfigFile(configContent)) { reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`); } } return reasons.length > 0 ? { compatible: false, reasons } : { compatible: true }; } - - // Private helper methods for Vitest config validation - - /** Validate workspace config file structure */ - private isValidWorkspaceConfigFile(fileContents: string): boolean { - let isValid = false; - const parsedFile = babel.babelParse(fileContents); - - babel.traverse(parsedFile, { - ExportDefaultDeclaration: (path: any) => { - const declaration = path.node.declaration; - isValid = - this.isWorkspaceConfigArray(declaration) || this.isDefineWorkspaceExpression(declaration); - }, - }); - - return isValid; - } - - /** Validate Vitest config file structure */ - private isValidVitestConfig(configContent: string): boolean { - const parsedConfig = babel.babelParse(configContent); - let isValidVitestConfig = false; - - babel.traverse(parsedConfig, { - ExportDefaultDeclaration: (path: any) => { - if (this.isDefineConfigExpression(path.node.declaration)) { - isValidVitestConfig = this.isSafeToExtendWorkspace(path.node.declaration); - } else if (this.isMergeConfigExpression(path.node.declaration)) { - // the config could be anywhere in the mergeConfig call, so we need to check each argument - const mergeCall = path.node.declaration as CallExpression; - isValidVitestConfig = mergeCall.arguments.some((arg) => - this.isSafeToExtendWorkspace(arg as CallExpression) - ); - } - }, - }); - - return isValidVitestConfig; - } - - private isWorkspaceConfigArray(node: any): boolean { - return ( - babel.types.isArrayExpression(node) && - node?.elements.every( - (el: any) => babel.types.isStringLiteral(el) || babel.types.isObjectExpression(el) - ) - ); - } - - private isDefineWorkspaceExpression(node: any): boolean { - return ( - babel.types.isCallExpression(node) && - node.callee && - (node.callee as any)?.name === 'defineWorkspace' && - this.isWorkspaceConfigArray(node.arguments?.[0]) - ); - } - - private isDefineConfigExpression(node: any): boolean { - return ( - babel.types.isCallExpression(node) && - (node.callee as any)?.name === 'defineConfig' && - babel.types.isObjectExpression(node.arguments?.[0]) - ); - } - - private isMergeConfigExpression(path: babel.types.Node): boolean { - return babel.types.isCallExpression(path) && (path.callee as any)?.name === 'mergeConfig'; - } - - private isSafeToExtendWorkspace(node: babel.types.Node): boolean { - // Extract the object expression to validate - let objectToValidate: babel.types.ObjectExpression | null = null; - - if (babel.types.isCallExpression(node)) { - // Handle function calls like defineConfig({...}) - if (node.arguments.length > 0 && babel.types.isObjectExpression(node.arguments[0])) { - objectToValidate = node.arguments[0]; - } - } else if (babel.types.isObjectExpression(node)) { - // Handle plain object literals like {...} - objectToValidate = node; - } - - // If we couldn't extract a valid object, it's not safe - if (!objectToValidate) { - return false; - } - - // Check that the object doesn't have problematic test.workspace properties - return objectToValidate.properties.every( - (p: any) => - p.key?.name !== 'test' || - (babel.types.isObjectExpression(p.value) && - p.value.properties.every( - ({ key, value }: any) => - key?.name !== 'workspace' || babel.types.isArrayExpression(value) - )) - ); - } } From 4ef9a1e8ee643bcbf4c0bfb8c1d1008ac6f27faf Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Thu, 19 Mar 2026 12:50:56 +0100 Subject: [PATCH 2/2] Remove unnecessary comments --- code/addons/vitest/src/updateVitestFile.ts | 3 --- code/core/src/babel/vitest-config-helpers.test.ts | 1 - code/core/src/cli/AddonVitestService.test.ts | 5 +---- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/code/addons/vitest/src/updateVitestFile.ts b/code/addons/vitest/src/updateVitestFile.ts index 8e1a80062f38..5ba8f8efd349 100644 --- a/code/addons/vitest/src/updateVitestFile.ts +++ b/code/addons/vitest/src/updateVitestFile.ts @@ -75,9 +75,6 @@ const mergeProperties = ( } }; -// isDefineConfigLike, getConfigObjectFromMergeArg, getEffectiveMergeConfigCall, and -// getTargetConfigObject are imported from 'storybook/internal/babel' (vitest-config-helpers) - /** * Resolves the value of a `test` ObjectProperty to an ObjectExpression. Handles both inline objects * and shorthand identifier references, e.g.: `{ test: { ... } }` → returns the inline diff --git a/code/core/src/babel/vitest-config-helpers.test.ts b/code/core/src/babel/vitest-config-helpers.test.ts index f3d11bf3498e..99c21cd10cef 100644 --- a/code/core/src/babel/vitest-config-helpers.test.ts +++ b/code/core/src/babel/vitest-config-helpers.test.ts @@ -144,7 +144,6 @@ describe('canUpdateVitestConfigFile', () => { ).toBe(true); }); - // Real-life example: xmtp-js (constant vitestConfig) it('returns true for mergeConfig with external vitestConfig variable', () => { expect( canUpdateVitestConfigFile( diff --git a/code/core/src/cli/AddonVitestService.test.ts b/code/core/src/cli/AddonVitestService.test.ts index b19693937070..f16a224c06b3 100644 --- a/code/core/src/cli/AddonVitestService.test.ts +++ b/code/core/src/cli/AddonVitestService.test.ts @@ -672,7 +672,7 @@ describe('AddonVitestService', () => { expect(result.compatible).toBe(true); }); - it('should accept plain export default {} (now supported)', async () => { + it('should accept plain export default {}', async () => { vi.mocked(find.any) .mockReturnValueOnce(undefined) // workspace .mockReturnValueOnce('vitest.config.ts'); // config @@ -844,7 +844,6 @@ export default defineConfig(() => ({ test: {} }))` expect(result.compatible).toBe(true); }); - // Real-life pattern: xmtp-js — const vitestConfig = {...}; mergeConfig(vite, vitestConfig) it('should accept mergeConfig with external vitestConfig variable', async () => { vi.mocked(find.any) .mockReturnValueOnce(undefined) // workspace @@ -860,7 +859,6 @@ export default defineConfig(() => ({ test: {} }))` expect(result.compatible).toBe(true); }); - // Real-life pattern: lynx-stack — const config = mergeConfig(...); export default config it('should accept const config = mergeConfig(...); export default config pattern', async () => { vi.mocked(find.any) .mockReturnValueOnce(undefined) // workspace @@ -879,7 +877,6 @@ export default defineConfig(() => ({ test: {} }))` expect(result.compatible).toBe(true); }); - // Real-life pattern: meilisearch-js-plugins — defineProject it('should accept defineProject({}) pattern', async () => { vi.mocked(find.any) .mockReturnValueOnce(undefined) // workspace