From 5b9405fe8cb02d779af206f0d743792ce50cb77f Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 12 Sep 2026 19:11:08 -0400 Subject: [PATCH 1/2] fix: force the Storybook dep pre-bundle on every run instead of trusting the cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vite keys the Storybook project's dep cache on the lockfile and the config, never on what the story graph imports. After a story or component gains an import of an already-installed package, the cached bundle is still "valid", Vite skips the scan, and the first request for the new package re-runs the optimizer mid-run: the browser hash is bumped, a full-reload is sent that the Vitest tester iframes ignore, and the `@storybook/react` renderer already loaded in a live iframe keeps the old `?v=` for its lazy `import("@storybook/react-dom-shim")`, so every story that iframe renders from then on fails with "Failed to fetch dynamically imported module". The next run is green because the cache has caught up. `optimizeDeps.force: true` on the storybook project discards the cache up front so the scanner crawls every story entry before the browser opens — the cold path CI already takes on every PR. Measured: stale-cache 6/6 red on the baseline, 6/6 green with the fix, 3/3 red reverted; ~0.8s per run. Closes #2340 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall --- clients/web/server/vite-base-config.ts | 7 +++++++ clients/web/vite.config.ts | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/clients/web/server/vite-base-config.ts b/clients/web/server/vite-base-config.ts index 3d9c1a90..5d4345a3 100644 --- a/clients/web/server/vite-base-config.ts +++ b/clients/web/server/vite-base-config.ts @@ -55,6 +55,13 @@ export function getViteBaseConfig() { // components import it at runtime (e.g. `protocolUtils`'s // `isInputRequiredResult` / `SUBSCRIPTION_ID_META_KEY`), so listing it // here keeps the dep graph stable across story files. + // + // That was one instance of a class, not the class: any package a story + // or component starts importing after the cache was written is + // discovered the same way, and pinning each one here would chase the + // symptom. The `storybook` project in `vite.config.ts` now sets + // `optimizeDeps.force` so every run re-scans the story entries up front + // (#2340); this entry stays as the explicit include it always was. include: ["@modelcontextprotocol/client"], }, }; diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index d6fea376..85ec909f 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -391,6 +391,24 @@ export default defineConfig(({ command }) => { configDir: path.join(dirname, ".storybook"), }), ], + optimizeDeps: { + // Re-bundle the browser dep pre-bundle on every run instead of + // trusting the cached one (#2340). Vite keys that cache on the + // lockfile and on this config, never on what the story graph + // imports, so a cached bundle stays "valid" after a story or + // component gains an import of a package that is already + // installed. The first request for it then re-runs the optimizer + // mid-run, which bumps the `?v=` browser hash and rewrites the + // shared chunks; the `@storybook/react` renderer already loaded in + // a live test iframe still holds the old hash for its lazy + // `import("@storybook/react-dom-shim")`, so every story that + // iframe renders from then on fails with "Failed to fetch + // dynamically imported module", and the next run is green because + // the cache has caught up. `force` discards the cache up front, so + // the scanner crawls every story entry before the browser opens — + // the same cold path CI takes on every PR. Cost: ~0.8s per run. + force: true, + }, test: { name: "storybook", // Vitest's default is 5000ms, which is the whole budget a play From 86e6774a9aab9c72ac174fcdddae43c90f0ceac9 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 12 Sep 2026 19:24:44 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20Copilot=20review=20round?= =?UTF-8?q?=201=20=E2=80=94=20move=20the=20Storybook=20optimizeDeps=20into?= =?UTF-8?q?=20a=20tested=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The storybook project's optimizeDeps now comes from `getStorybookOptimizeDeps()` in `vite-base-config.ts`, beside the dev-server helper it mirrors, so the `force` flag and the base include/exclude it carries are asserted by `vite-base-config.test.ts` rather than living only in an inline object a config refactor could drop. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp Signed-off-by: cliffhall --- clients/web/server/vite-base-config.ts | 35 +++++++++++++++++-- .../server/vite-base-config.test.ts | 25 +++++++++++++ clients/web/vite.config.ts | 23 +++--------- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/clients/web/server/vite-base-config.ts b/clients/web/server/vite-base-config.ts index 5d4345a3..9a0a4609 100644 --- a/clients/web/server/vite-base-config.ts +++ b/clients/web/server/vite-base-config.ts @@ -59,14 +59,43 @@ export function getViteBaseConfig() { // That was one instance of a class, not the class: any package a story // or component starts importing after the cache was written is // discovered the same way, and pinning each one here would chase the - // symptom. The `storybook` project in `vite.config.ts` now sets - // `optimizeDeps.force` so every run re-scans the story entries up front - // (#2340); this entry stays as the explicit include it always was. + // symptom. `getStorybookOptimizeDeps` below adds `force` so every run + // re-scans the story entries up front (#2340); this entry stays as the + // explicit include it always was. include: ["@modelcontextprotocol/client"], }, }; } +/** + * Storybook (browser Vitest project) optimizeDeps: the base set, re-bundled + * on every run instead of read from the cache (#2340). + * + * Vite keys that cache on the lockfile and on the config, never on what the + * story graph imports, so a cached bundle stays "valid" after a story or + * component gains an import of a package that is already installed. With a + * valid cache Vite skips the scan, and the first request for the new package + * re-runs the optimizer mid-run: the shared chunks are rewritten, the `?v=` + * browser hash is bumped, and Vite sends a `full-reload` that the Vitest + * tester iframes do not act on. The `@storybook/react` renderer already loaded + * in a live iframe still holds the old hash for its lazy + * `import("@storybook/react-dom-shim")`, so every story that iframe renders + * from then on fails with "Failed to fetch dynamically imported module" — and + * the next run is green, because the cache has caught up. `force` discards the + * cache up front so the scanner crawls every story entry before the browser + * opens: the same cold path CI takes on every PR. Cost: ~0.8s per run. + * + * Spread from `getViteBaseConfig()` rather than left to Vitest's project merge + * so the include/exclude the base set carries are the helper's own claim, and + * tested as such. + */ +export function getStorybookOptimizeDeps() { + return { + ...getViteBaseConfig().optimizeDeps, + force: true, + }; +} + /** * Dev-server optimizeDeps: rebuild from scratch each launch — no stale * pre-bundles carried across restarts (avoids 504 Outdated Optimize Dep). diff --git a/clients/web/src/test/integration/server/vite-base-config.test.ts b/clients/web/src/test/integration/server/vite-base-config.test.ts index 90c39abd..2fcb88d9 100644 --- a/clients/web/src/test/integration/server/vite-base-config.test.ts +++ b/clients/web/src/test/integration/server/vite-base-config.test.ts @@ -10,6 +10,7 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { clearViteDepsCache, + getStorybookOptimizeDeps, getViteBaseConfig, getViteDevOptimizeDeps, } from "../../../../server/vite-base-config.js"; @@ -54,6 +55,30 @@ describe("getViteDevOptimizeDeps", () => { }); }); +describe("getStorybookOptimizeDeps", () => { + it("forces a re-bundle on every run so a stale cache cannot re-optimize mid-run (#2340)", () => { + const config = getStorybookOptimizeDeps(); + expect(config.force).toBe(true); + }); + + it("keeps the base include and exclude sets", () => { + const config = getStorybookOptimizeDeps(); + const base = getViteBaseConfig().optimizeDeps; + expect(config.include).toEqual(base.include); + expect(config.exclude).toEqual(base.exclude); + expect(config.include).toContain("@modelcontextprotocol/client"); + }); + + it("does not carry the dev server's stale-request tolerance", () => { + // `ignoreOutdatedRequests` belongs to `vite dev`, where a full page reload + // follows a re-optimization; under Vitest a request for an outdated dep + // must fail loudly, since nothing reloads the tester iframe. + expect(getStorybookOptimizeDeps()).not.toHaveProperty( + "ignoreOutdatedRequests", + ); + }); +}); + describe("clearViteDepsCache", () => { let tempRoot: string; diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index 85ec909f..ac7f541b 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -9,6 +9,7 @@ import { storybookTest } from "@storybook/addon-vitest/vitest-plugin"; import { playwright } from "@vitest/browser-playwright"; import { honoMiddlewarePlugin } from "./server/vite-hono-plugin"; import { + getStorybookOptimizeDeps, getViteBaseConfig, getViteDevOptimizeDeps, } from "./server/vite-base-config"; @@ -391,24 +392,10 @@ export default defineConfig(({ command }) => { configDir: path.join(dirname, ".storybook"), }), ], - optimizeDeps: { - // Re-bundle the browser dep pre-bundle on every run instead of - // trusting the cached one (#2340). Vite keys that cache on the - // lockfile and on this config, never on what the story graph - // imports, so a cached bundle stays "valid" after a story or - // component gains an import of a package that is already - // installed. The first request for it then re-runs the optimizer - // mid-run, which bumps the `?v=` browser hash and rewrites the - // shared chunks; the `@storybook/react` renderer already loaded in - // a live test iframe still holds the old hash for its lazy - // `import("@storybook/react-dom-shim")`, so every story that - // iframe renders from then on fails with "Failed to fetch - // dynamically imported module", and the next run is green because - // the cache has caught up. `force` discards the cache up front, so - // the scanner crawls every story entry before the browser opens — - // the same cold path CI takes on every PR. Cost: ~0.8s per run. - force: true, - }, + // Re-bundled on every run rather than read from the cache — the + // stale-cache failure it prevents, and why `force` is the lever, are + // on the helper (#2340). + optimizeDeps: getStorybookOptimizeDeps(), test: { name: "storybook", // Vitest's default is 5000ms, which is the whole budget a play