From 944f57309eb5d5fbcf86954efa55837e34d1e8d6 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 7 Jul 2026 22:44:34 +0200 Subject: [PATCH 1/3] CLI: Fix angular-to-angular-vite migration failing to configure addon-vitest The migration's TS config loader broke whenever `skipCache`'s cache-busting query string was appended to a `.ts` main config URL: the loader matched files via `url.endsWith('.ts')`, which silently failed on `main.ts?` and fell back to Node's native type-stripping, which (unlike the esbuild transform it bypassed) doesn't elide type-only named imports. That caused a hard crash for any project importing `StorybookConfig` without `type`, and separately let stale, pre-migration framework/builder data leak into the addon-vitest compatibility check, producing a false "Non-Vite builder is not supported" failure right after a successful migration to `@storybook/angular-vite`. Also fixes an inverted `--yes` default on the webpackFinal continuation prompt, which cancelled the whole migration in non-interactive mode, and threads `skipCache` through `getStorybookData` so `automigrate`/`add` read the freshly-rewritten main config instead of a cached evaluation. --- code/core/src/bin/loader.ts | 19 ++++++++------ code/core/src/cli/getStorybookData.ts | 10 +++++++- .../src/common/utils/get-storybook-info.ts | 8 ++++-- code/lib/cli-storybook/src/add.ts | 4 +-- .../fixes/angular-to-angular-vite.ts | 25 +++++++++++++++++-- 5 files changed, 52 insertions(+), 14 deletions(-) diff --git a/code/core/src/bin/loader.ts b/code/core/src/bin/loader.ts index a9f44613c6a1..d75fd003d752 100644 --- a/code/core/src/bin/loader.ts +++ b/code/core/src/bin/loader.ts @@ -137,16 +137,21 @@ export function addExtensionsToRelativeImports(source: string, filePath: string) } export const load: LoadHook = async (url, context, nextLoad) => { + // Strip any query string (e.g. the cache-busting `?` importModule appends for + // skipCache) before checking the extension, otherwise a cache-busted URL like + // `file:///main.ts?123` no longer ends with `.ts` and silently skips the esbuild transform below. + const urlWithoutQuery = url.split('?')[0]; + /** Convert TS to ESM using esbuild */ if ( - url.endsWith('.ts') || - url.endsWith('.tsx') || - url.endsWith('.mts') || - url.endsWith('.cts') || - url.endsWith('.mtsx') || - url.endsWith('.ctsx') + urlWithoutQuery.endsWith('.ts') || + urlWithoutQuery.endsWith('.tsx') || + urlWithoutQuery.endsWith('.mts') || + urlWithoutQuery.endsWith('.cts') || + urlWithoutQuery.endsWith('.mtsx') || + urlWithoutQuery.endsWith('.ctsx') ) { - const filePath = fileURLToPath(url); + const filePath = fileURLToPath(urlWithoutQuery); const rawSource = await readFile(filePath, 'utf-8'); const transformedSource = await transform(rawSource, { loader: 'ts', diff --git a/code/core/src/cli/getStorybookData.ts b/code/core/src/cli/getStorybookData.ts index 38f126dcc303..e0972c37498b 100644 --- a/code/core/src/cli/getStorybookData.ts +++ b/code/core/src/cli/getStorybookData.ts @@ -23,9 +23,16 @@ export function getWorkingDir(configDir: string): string { export const getStorybookData = async ({ configDir: userDefinedConfigDir, packageManagerName, + skipCache, }: { configDir?: string; packageManagerName?: PackageManagerName; + /** + * Skip the module cache when reading the main config. Pass `true` when a prior step in the same + * process (e.g. an automigration) may have rewritten the main config on disk, otherwise this + * would read back the module system's cached evaluation from before that rewrite. + */ + skipCache?: boolean; }) => { logger.debug('Getting Storybook info...'); const { @@ -41,7 +48,8 @@ export const getStorybookData = async ({ addons, } = await getStorybookInfo( userDefinedConfigDir, - userDefinedConfigDir ? dirname(userDefinedConfigDir) : undefined + userDefinedConfigDir ? dirname(userDefinedConfigDir) : undefined, + { skipCache } ); const configDir = userDefinedConfigDir || configDirFromScript || '.storybook'; diff --git a/code/core/src/common/utils/get-storybook-info.ts b/code/core/src/common/utils/get-storybook-info.ts index 3652807aba3a..f62b98ed21c5 100644 --- a/code/core/src/common/utils/get-storybook-info.ts +++ b/code/core/src/common/utils/get-storybook-info.ts @@ -1,13 +1,17 @@ import { existsSync, readFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; -import { CoreWebpackCompiler, SupportedFramework } from 'storybook/internal/types'; import type { CoreCommon_StorybookInfo, PackageJson, StorybookConfigRaw, } from 'storybook/internal/types'; -import { SupportedBuilder, SupportedRenderer } from 'storybook/internal/types'; +import { + CoreWebpackCompiler, + SupportedBuilder, + SupportedFramework, + SupportedRenderer, +} from 'storybook/internal/types'; import invariant from 'tiny-invariant'; diff --git a/code/lib/cli-storybook/src/add.ts b/code/lib/cli-storybook/src/add.ts index a9fb093cf3e9..2e11eebbbdfb 100644 --- a/code/lib/cli-storybook/src/add.ts +++ b/code/lib/cli-storybook/src/add.ts @@ -1,7 +1,6 @@ import { type PackageManagerName, setupAddonInConfig, versions } from 'storybook/internal/common'; import { readConfig } from 'storybook/internal/csf-tools'; -import { logger as nodeLogger } from 'storybook/internal/node-logger'; -import { prompt } from 'storybook/internal/node-logger'; +import { logger as nodeLogger, prompt } from 'storybook/internal/node-logger'; import type { StorybookConfigRaw } from 'storybook/internal/types'; import SemVer from 'semver'; @@ -92,6 +91,7 @@ export async function add( await getStorybookData({ configDir: userSpecifiedConfigDir, packageManagerName: pkgMgr, + skipCache: true, }); if (typeof configDir === 'undefined') { diff --git a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts index aebb234e465a..698ced615c0f 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts @@ -122,6 +122,25 @@ export default defineConfig({ }); `; +// `StorybookConfig` is a type-only export, so a plain (non-`type`) import of it is technically +// incorrect regardless of framework. Node's native type-stripping doesn't reliably elide such +// imports (see loader.ts), so a hard-failing runtime import can surface once this migration +// rewrites the specifier — mark it `type` so the rewritten import is actually correct. +const markStorybookConfigImportAsType = (content: string): string => + content.replace( + new RegExp(`import\\s*\\{([^}]*)\\}\\s*from\\s*(['"])${ANGULAR_VITE_PACKAGE}\\2`), + (match, specifiers: string, quote: string) => { + if (/\btype\b/.test(specifiers)) { + return match; + } + const fixedSpecifiers = specifiers.replace( + /(^|,)(\s*)StorybookConfig\s*(?=,|$)/, + (_specMatch: string, sep: string, space: string) => `${sep}${space}type StorybookConfig` + ); + return `import {${fixedSpecifiers}} from ${quote}${ANGULAR_VITE_PACKAGE}${quote}`; + } + ); + const transformMainConfig = async (mainConfigPath: string, dryRun: boolean): Promise => { try { const content = await readFile(mainConfigPath, 'utf-8'); @@ -132,7 +151,9 @@ const transformMainConfig = async (mainConfigPath: string, dryRun: boolean): Pro // Replace @storybook/angular with @storybook/angular-vite using a negative // lookahead so references that are already @storybook/angular-vite are left alone. - const transformed = content.replace(/@storybook\/angular(?!-vite)/g, ANGULAR_VITE_PACKAGE); + const transformed = markStorybookConfigImportAsType( + content.replace(/@storybook\/angular(?!-vite)/g, ANGULAR_VITE_PACKAGE) + ); if (transformed !== content && !dryRun) { await writeFile(mainConfigPath, transformed); @@ -353,7 +374,7 @@ export const angularToAngularVite: Fix = { ); const shouldContinue = yes - ? false + ? true : await prompt.confirm({ message: 'I detected a webpackFinal hook. It will not carry over. Continue anyway?', initialValue: false, From 1d1b2c3ad853ad740e4e05df3a0df0f8c6d47647 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 7 Jul 2026 22:57:11 +0200 Subject: [PATCH 2/3] CLI: Add regression tests for the loader query-string and type-import fixes Covers the loader.ts esbuild transform still applying to a `.ts?` URL, the webpackFinal prompt proceeding automatically under `--yes`, and the migration marking a plain StorybookConfig import as `import type`. --- code/core/src/bin/loader.test.ts | 49 +++++++++++++++ .../fixes/angular-to-angular-vite.test.ts | 63 +++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/code/core/src/bin/loader.test.ts b/code/core/src/bin/loader.test.ts index b0738b3e9ae2..b6a51375f96c 100644 --- a/code/core/src/bin/loader.test.ts +++ b/code/core/src/bin/loader.test.ts @@ -1,24 +1,73 @@ import { readdirSync } from 'node:fs'; +import { readFile } from 'node:fs/promises'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { deprecate } from 'storybook/internal/node-logger'; +import { transform } from 'esbuild'; + import { addExtensionsToRelativeImports, clearDirectoryCache, + load, resolveWithExtension, } from './loader.ts'; // Mock dependencies vi.mock('node:fs'); +vi.mock('node:fs/promises'); vi.mock('storybook/internal/node-logger'); +vi.mock('esbuild'); describe('loader', () => { beforeEach(() => { clearDirectoryCache(); }); + describe('load', () => { + const nextLoad = vi.fn(); + + beforeEach(() => { + nextLoad.mockReset(); + vi.mocked(readFile).mockResolvedValue('const x: number = 1;'); + vi.mocked(transform).mockResolvedValue({ + code: 'const x = 1;', + map: '', + warnings: [], + } as any); + }); + + it('transforms a plain .ts URL with esbuild', async () => { + const result = await load('file:///project/main.ts', {} as any, nextLoad); + + expect(transform).toHaveBeenCalled(); + expect(nextLoad).not.toHaveBeenCalled(); + expect(result).toMatchObject({ format: 'module', shortCircuit: true }); + }); + + it('still transforms a .ts URL with a cache-busting query string appended', async () => { + // Regression test: importModule appends `?` to bust the module cache when + // skipCache is set. That must not cause this loader to skip the file and fall through to + // Node's native handling, which does not elide type-only named imports the way esbuild does. + const result = await load('file:///project/main.ts?1234567890', {} as any, nextLoad); + + expect(readFile).toHaveBeenCalledWith('/project/main.ts', 'utf-8'); + expect(transform).toHaveBeenCalled(); + expect(nextLoad).not.toHaveBeenCalled(); + expect(result).toMatchObject({ format: 'module', shortCircuit: true }); + }); + + it('delegates non-TS URLs to nextLoad', async () => { + nextLoad.mockResolvedValue({ format: 'module', shortCircuit: true, source: '' }); + + await load('file:///project/main.js', {} as any, nextLoad); + + expect(transform).not.toHaveBeenCalled(); + expect(nextLoad).toHaveBeenCalledWith('file:///project/main.js', {}); + }); + }); + describe('resolveWithExtension', () => { it('should return the path as-is if it already has an extension', () => { const result = resolveWithExtension('./test.js', '/project/src/file.ts'); diff --git a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts index 334192c5ec5c..2284b22d42bd 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts @@ -308,6 +308,26 @@ describe('angular-to-angular-vite', () => { expect(mockPackageManager.removeDependencies).toHaveBeenCalledWith([ANGULAR_PACKAGE]); }); + it('proceeds past the webpackFinal warning without prompting when yes is set', async () => { + mockReadFile.mockResolvedValue( + `export default { framework: '${ANGULAR_PACKAGE}', webpackFinal: async c => c };` + ); + + await angularToAngularVite.run!({ + result: { ...baseResult, hasWebpackFinal: true }, + dryRun: false, + packageManager: mockPackageManager, + mainConfigPath: '/project/.storybook/main.ts', + storiesPaths: [], + configDir: '.storybook', + storybookVersion: '9.0.0', + yes: true, + } as any); + + expect(mockPromptConfirm).not.toHaveBeenCalled(); + expect(mockPackageManager.removeDependencies).toHaveBeenCalledWith([ANGULAR_PACKAGE]); + }); + it('updates dependencies correctly', async () => { mockPromptConfirm.mockResolvedValue(false); mockReadFile.mockResolvedValue(`export default { framework: '${ANGULAR_PACKAGE}' };`); @@ -374,6 +394,49 @@ export default { framework: { name: '${ANGULAR_VITE_PACKAGE}', options: {} } };` ); }); + it('marks a plain StorybookConfig import as a type import when rewriting the framework package', async () => { + mockPromptConfirm.mockResolvedValue(false); + mockReadFile.mockResolvedValue( + `import { StorybookConfig } from '${ANGULAR_PACKAGE}';\nexport default { framework: '${ANGULAR_PACKAGE}' };` + ); + + await angularToAngularVite.run!({ + result: baseResult, + dryRun: false, + packageManager: mockPackageManager, + mainConfigPath: '/project/.storybook/main.ts', + storiesPaths: [], + configDir: '.storybook', + storybookVersion: '9.0.0', + } as any); + + const [, writtenContent] = mockWriteFile.mock.calls[0]; + expect(writtenContent).toContain(`from '${ANGULAR_VITE_PACKAGE}'`); + expect(writtenContent).toMatch(/import\s*\{\s*type StorybookConfig\s*\}/); + }); + + it('leaves an already type-only StorybookConfig import untouched', async () => { + mockPromptConfirm.mockResolvedValue(false); + mockReadFile.mockResolvedValue( + `import type { StorybookConfig } from '${ANGULAR_PACKAGE}';\nexport default { framework: '${ANGULAR_PACKAGE}' };` + ); + + await angularToAngularVite.run!({ + result: baseResult, + dryRun: false, + packageManager: mockPackageManager, + mainConfigPath: '/project/.storybook/main.ts', + storiesPaths: [], + configDir: '.storybook', + storybookVersion: '9.0.0', + } as any); + + const [, writtenContent] = mockWriteFile.mock.calls[0]; + expect(writtenContent).toContain( + `import type { StorybookConfig } from '${ANGULAR_VITE_PACKAGE}'` + ); + }); + it('rewrites angular.json builder entries', async () => { mockPromptConfirm.mockResolvedValue(false); From 848adf5eac3f4664107efc1cc7aa590d085decf0 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 7 Jul 2026 23:04:16 +0200 Subject: [PATCH 3/3] Revert the webpackFinal yes-flag flip and the StorybookConfig type-import rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loader.ts fix already makes esbuild's transform apply to every .ts main config load regardless of skipCache's query string, and esbuild elides a type-only StorybookConfig import whether or not it's marked `type` — so the type-import rewrite in this fixer was redundant. Cancelling the migration under --yes when a webpackFinal hook is present is intentional: silently dropping a hook that won't carry over to Vite in unattended mode is worse than requiring an explicit rerun after porting it. --- .../fixes/angular-to-angular-vite.test.ts | 63 ------------------- .../fixes/angular-to-angular-vite.ts | 25 +------- 2 files changed, 2 insertions(+), 86 deletions(-) diff --git a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts index 2284b22d42bd..334192c5ec5c 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.test.ts @@ -308,26 +308,6 @@ describe('angular-to-angular-vite', () => { expect(mockPackageManager.removeDependencies).toHaveBeenCalledWith([ANGULAR_PACKAGE]); }); - it('proceeds past the webpackFinal warning without prompting when yes is set', async () => { - mockReadFile.mockResolvedValue( - `export default { framework: '${ANGULAR_PACKAGE}', webpackFinal: async c => c };` - ); - - await angularToAngularVite.run!({ - result: { ...baseResult, hasWebpackFinal: true }, - dryRun: false, - packageManager: mockPackageManager, - mainConfigPath: '/project/.storybook/main.ts', - storiesPaths: [], - configDir: '.storybook', - storybookVersion: '9.0.0', - yes: true, - } as any); - - expect(mockPromptConfirm).not.toHaveBeenCalled(); - expect(mockPackageManager.removeDependencies).toHaveBeenCalledWith([ANGULAR_PACKAGE]); - }); - it('updates dependencies correctly', async () => { mockPromptConfirm.mockResolvedValue(false); mockReadFile.mockResolvedValue(`export default { framework: '${ANGULAR_PACKAGE}' };`); @@ -394,49 +374,6 @@ export default { framework: { name: '${ANGULAR_VITE_PACKAGE}', options: {} } };` ); }); - it('marks a plain StorybookConfig import as a type import when rewriting the framework package', async () => { - mockPromptConfirm.mockResolvedValue(false); - mockReadFile.mockResolvedValue( - `import { StorybookConfig } from '${ANGULAR_PACKAGE}';\nexport default { framework: '${ANGULAR_PACKAGE}' };` - ); - - await angularToAngularVite.run!({ - result: baseResult, - dryRun: false, - packageManager: mockPackageManager, - mainConfigPath: '/project/.storybook/main.ts', - storiesPaths: [], - configDir: '.storybook', - storybookVersion: '9.0.0', - } as any); - - const [, writtenContent] = mockWriteFile.mock.calls[0]; - expect(writtenContent).toContain(`from '${ANGULAR_VITE_PACKAGE}'`); - expect(writtenContent).toMatch(/import\s*\{\s*type StorybookConfig\s*\}/); - }); - - it('leaves an already type-only StorybookConfig import untouched', async () => { - mockPromptConfirm.mockResolvedValue(false); - mockReadFile.mockResolvedValue( - `import type { StorybookConfig } from '${ANGULAR_PACKAGE}';\nexport default { framework: '${ANGULAR_PACKAGE}' };` - ); - - await angularToAngularVite.run!({ - result: baseResult, - dryRun: false, - packageManager: mockPackageManager, - mainConfigPath: '/project/.storybook/main.ts', - storiesPaths: [], - configDir: '.storybook', - storybookVersion: '9.0.0', - } as any); - - const [, writtenContent] = mockWriteFile.mock.calls[0]; - expect(writtenContent).toContain( - `import type { StorybookConfig } from '${ANGULAR_VITE_PACKAGE}'` - ); - }); - it('rewrites angular.json builder entries', async () => { mockPromptConfirm.mockResolvedValue(false); diff --git a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts index 698ced615c0f..aebb234e465a 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/angular-to-angular-vite.ts @@ -122,25 +122,6 @@ export default defineConfig({ }); `; -// `StorybookConfig` is a type-only export, so a plain (non-`type`) import of it is technically -// incorrect regardless of framework. Node's native type-stripping doesn't reliably elide such -// imports (see loader.ts), so a hard-failing runtime import can surface once this migration -// rewrites the specifier — mark it `type` so the rewritten import is actually correct. -const markStorybookConfigImportAsType = (content: string): string => - content.replace( - new RegExp(`import\\s*\\{([^}]*)\\}\\s*from\\s*(['"])${ANGULAR_VITE_PACKAGE}\\2`), - (match, specifiers: string, quote: string) => { - if (/\btype\b/.test(specifiers)) { - return match; - } - const fixedSpecifiers = specifiers.replace( - /(^|,)(\s*)StorybookConfig\s*(?=,|$)/, - (_specMatch: string, sep: string, space: string) => `${sep}${space}type StorybookConfig` - ); - return `import {${fixedSpecifiers}} from ${quote}${ANGULAR_VITE_PACKAGE}${quote}`; - } - ); - const transformMainConfig = async (mainConfigPath: string, dryRun: boolean): Promise => { try { const content = await readFile(mainConfigPath, 'utf-8'); @@ -151,9 +132,7 @@ const transformMainConfig = async (mainConfigPath: string, dryRun: boolean): Pro // Replace @storybook/angular with @storybook/angular-vite using a negative // lookahead so references that are already @storybook/angular-vite are left alone. - const transformed = markStorybookConfigImportAsType( - content.replace(/@storybook\/angular(?!-vite)/g, ANGULAR_VITE_PACKAGE) - ); + const transformed = content.replace(/@storybook\/angular(?!-vite)/g, ANGULAR_VITE_PACKAGE); if (transformed !== content && !dryRun) { await writeFile(mainConfigPath, transformed); @@ -374,7 +353,7 @@ export const angularToAngularVite: Fix = { ); const shouldContinue = yes - ? true + ? false : await prompt.confirm({ message: 'I detected a webpackFinal hook. It will not carry over. Continue anyway?', initialValue: false,