CLI: Fix angular-to-angular-vite migration failing to configure addon-vitest

[valentinpalkovic]

Closes #

What I did

While reproducing an angular-to-angular-vite automigration failure against a real monorepo (Bitwarden's clients), the migration reported success, but the deferred addon-vitest postinstall then failed with:

Non-Vite builder is not supported
Test feature cannot yet be used with angular

1. getStorybookData had no skipCache option. It's the canonical collector used by automigrate/doctor/add (per its own docstring), but unlike getStorybookInfo/loadMainConfig it always read the ES-module-cached, pre-migration evaluation of main.ts. Added skipCache and threaded it through; add() now passes skipCache: true, since it runs both standalone and mid-automigration right after main.ts gets rewritten.
2. The actual root cause, in code/core/src/bin/loader.ts. The custom TS loader decides whether to esbuild-transform a .ts config via url.endsWith('.ts'). skipCache's cache-busting ?<timestamp> query string breaks that check, so it silently falls back to Node's native type-stripping, which — unlike esbuild — doesn't elide a plain (non-type) named import that's only ever used as a type. Any main.ts with import { StorybookConfig } from '...' (harmless under a normal load) then hard-crashes with "does not provide an export named 'StorybookConfig'" the moment it's read with skipCache: true. Verified in isolation with a throwaway fake package before landing the fix — I initially assumed it was a difference between @storybook/angular and @storybook/angular-vite themselves, but it isn't; both packages behave identically here, and esbuild elides the type-only import regardless of whether it's marked type, so no change to the fixer's own rewrite logic was needed.

I also considered flipping the fixer's --yes default on the webpackFinal-detected prompt (it currently always cancels the migration under --yes), but decided to leave that alone: silently dropping a hook that won't carry over to Vite in unattended mode seemed worse than requiring an explicit rerun after porting it, so that stays as-is.

Fairly confident in this one — reproduced end-to-end against the real project several times, with and without the fix, before landing on this.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• unit tests
• stories
• integration tests
• end-to-end tests

Manual testing

1. Point at (or create) a Storybook project still on the old webpack-based @storybook/angular, whose .storybook/main.ts contains a non-type StorybookConfig import (no webpackFinal hook — that hits a separate, intentional --yes cancellation path unrelated to this fix), e.g.:

   import { StorybookConfig } from "@storybook/angular";
   
   const config: StorybookConfig = {
     framework: { name: "@storybook/angular", options: {} },
     // ...
   };
   export default config;

2. Build the CLI locally: yarn nx run-many -t compile --projects=core,cli,addon-vitest

3. From that project's directory, run the CLI non-interactively against it:

   node <path-to-storybook-repo>/code/lib/cli-storybook/dist/bin/index.js upgrade --yes --config-dir .storybook

4. Confirm: the migration completes, and addon-vitest gets configured without the "Non-Vite builder is not supported" error.

5. Regression check: revert just the code/core/src/bin/loader.ts change, recompile, rerun step 3 — it should fail again with SyntaxError: The requested module '@storybook/angular-vite' does not provide an export named 'StorybookConfig'.

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

• Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This pull request has been released as version 0.0.0-pr-35404-sha-848adf5e. Try it out in a new sandbox by running npx storybook@0.0.0-pr-35404-sha-848adf5e sandbox or in an existing project with npx storybook@0.0.0-pr-35404-sha-848adf5e upgrade.

More information

Published version	0.0.0-pr-35404-sha-848adf5e
Triggered by	@valentinpalkovic
Repository	storybookjs/storybook
Branch	valentin/fix-automigrate-angular-vite-skipcache
Commit	848adf5e
Datetime	Tue Jul 7 21:23:19 UTC 2026 (1783459399)
Workflow run	28899668748

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=35404

[coderabbitai]

📝 Walkthrough

Walkthrough

The loader's load function now strips query strings before checking TypeScript file extensions, fixing detection for cache-busted URLs, with accompanying test coverage. Separately, getStorybookData and getStorybookInfo gain a skipCache option, used by the add CLI command; an unrelated import reformat is also included.

Changes

ESM Loader Query String Fix

Layer / File(s)	Summary
Query string stripping in loader and updated tests code/core/src/bin/loader.ts, code/core/src/bin/loader.test.ts	load strips query strings before TS extension checks and file path resolution; new tests mock node:fs/promises and esbuild to verify transformation for plain and cache-busted .ts URLs, and delegation for non-TS URLs.

skipCache Option for Storybook Data

Layer / File(s)	Summary
skipCache option plumbing and add command usage code/core/src/cli/getStorybookData.ts, code/core/src/common/utils/get-storybook-info.ts, code/lib/cli-storybook/src/add.ts	getStorybookData accepts an optional skipCache flag forwarded to getStorybookInfo; the add command passes skipCache: true and consolidates node-logger imports; get-storybook-info.ts reorganizes type/value imports.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NodeESM
  participant load
  participant esbuild
  NodeESM->>load: url = file:///main.ts?123
  load->>load: strip query string
  load->>esbuild: transform(readFile(urlWithoutQuery))
  esbuild-->>load: transformed code
  load-->>NodeESM: return transformed source

Loading

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

code/core/src/bin/loader.test.ts (1)

34-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Avoid as any cast on mock return value.

Casting the transform mock's resolved value with as any bypasses the type contract expected of TransformResult.

As per coding guidelines, "Mock implementations should match the expected return type of the original function" and "Use type-safe mocking with vi.mocked() in Vitest tests."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/core/src/bin/loader.test.ts` around lines 34 - 38, The `transform` mock
in `loader.test.ts` is using an `as any` cast to bypass the expected
`TransformResult` type. Update the `vi.mocked(transform).mockResolvedValue(...)`
setup to return a value that matches the real `transform` return contract
without casting, using the proper typed shape for `TransformResult` so the mock
stays type-safe.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@code/core/src/bin/loader.test.ts`:
- Line 19: Add the required spy option to the new Vitest mocks in
loader.test.ts: update the vi.mock calls for node:fs/promises and esbuild to use
spy: true. Keep the existing test setup intact, and make the change wherever the
mock declarations appear so they comply with the project’s Vitest mocking
guideline.
- Line 2: The test in loader.test.ts is mocking the entire node:fs/promises
module, which should be replaced with a memfs-based filesystem for this
filesystem-touching test. Update the loader.test.ts setup to use memfs for file
reads/writes and keep the real node:fs/promises behavior otherwise, removing the
wholesale vi.mock('node:fs/promises') approach. Use the existing test setup
around readFile and any loader helpers to route filesystem access through memfs
instead of a full module mock.

---

Nitpick comments:
In `@code/core/src/bin/loader.test.ts`:
- Around line 34-38: The `transform` mock in `loader.test.ts` is using an `as
any` cast to bypass the expected `TransformResult` type. Update the
`vi.mocked(transform).mockResolvedValue(...)` setup to return a value that
matches the real `transform` return contract without casting, using the proper
typed shape for `TransformResult` so the mock stays type-safe.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8e364fb9-6f00-4866-8385-12e14847fe58

📥 Commits

Reviewing files that changed from the base of the PR and between 696e724 and 848adf5.

📒 Files selected for processing (5)

• code/core/src/bin/loader.test.ts
• code/core/src/bin/loader.ts
• code/core/src/cli/getStorybookData.ts
• code/core/src/common/utils/get-storybook-info.ts
• code/lib/cli-storybook/src/add.ts