fix(cli): add direct vitest dep for browser-mode packages on migration

Brooooooklyn · claude · Brooooooklyn · commit 03e24a2fc22a · 2026-05-21T12:08:10.000+08:00
vitest browser mode injects `optimizeDeps.include` entries like
`vitest &gt; expect-type` that Vite resolves from the config root. In a
pnpm strict (non-hoisted) layout, a `vitest` pulled in only transitively
via `vite-plus` is not resolvable from the package directory, so the
optimizer fails and the browser test page hangs forever — this broke the
`vibe-dashboard` E2E job across the whole branch.

`vp migrate` now detects vitest browser mode (recursive scan for
`@vitest/browser` / `vite-plus/test/browser` specifiers, stopping at
nested package.json boundaries) and adds `vitest` as a direct
devDependency so pnpm hoists it next to the package.

Co-Authored-By: Claude Opus 4.7 &lt;noreply@anthropic.com&gt;
diff --git a/packages/cli/src/migration/__tests__/migrator.spec.ts b/packages/cli/src/migration/__tests__/migrator.spec.ts
@@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { parse as parseYaml } from 'yaml';
 
 import { PackageManager } from '../../types/index.js';
+import { VITEST_VERSION } from '../../utils/constants.js';
 import { createMigrationReport } from '../report.js';
 
 // Mock VITE_PLUS_VERSION to a stable value for snapshot tests.
@@ -330,6 +331,54 @@ describe('rewritePackageJson', () => {
     expect(pkg.dependencies).toHaveProperty('playwright', '^1.40.0');
     expect(pkg.devDependencies).not.toHaveProperty('playwright');
   });
+
+  it('adds a direct vitest devDependency when the package uses browser mode', async () => {
+    // A package that drives vitest browser mode but has no direct vitest dep
+    // (e.g. it only imports `vite-plus/test/browser-playwright`). `@vitest/browser`
+    // needs `vitest` resolvable from the package root, so the migration must
+    // pin it as a direct devDependency.
+    const pkg = {
+      devDependencies: {
+        playwright: '^1.58.0',
+      },
+    };
+    rewritePackageJson(pkg, PackageManager.pnpm, true, undefined, undefined, true);
+    expect(pkg.devDependencies).toHaveProperty('vitest', 'catalog:');
+    expect(pkg.devDependencies).toHaveProperty('vite-plus', 'catalog:');
+  });
+
+  it('uses a concrete vitest version for browser mode in non-catalog package managers', async () => {
+    const pkg = {
+      devDependencies: {
+        playwright: '^1.58.0',
+      },
+    };
+    rewritePackageJson(pkg, PackageManager.npm, false, undefined, undefined, true);
+    expect((pkg as { devDependencies?: Record<string, string> }).devDependencies?.vitest).toBe(
+      VITEST_VERSION,
+    );
+  });
+
+  it('does not overwrite an existing direct vitest dep in browser mode', async () => {
+    const pkg = {
+      devDependencies: {
+        vitest: '^4.0.0',
+      },
+    };
+    rewritePackageJson(pkg, PackageManager.pnpm, true, undefined, undefined, true);
+    // existing direct dep is normalized through the override path, not replaced
+    expect(pkg.devDependencies.vitest).toBe('catalog:');
+  });
+
+  it('does not add vitest when browser mode is not detected', async () => {
+    const pkg = {
+      devDependencies: {
+        vite: '^7.0.0',
+      },
+    };
+    rewritePackageJson(pkg, PackageManager.pnpm, true, undefined, undefined, false);
+    expect(pkg.devDependencies).not.toHaveProperty('vitest');
+  });
 });
 
 describe('parseNvmrcVersion', () => {
@@ -779,6 +828,72 @@ describe('rewriteStandaloneProject pnpm workspace yaml', () => {
     expect(pkg.peerDependencies).not.toHaveProperty('tsdown');
   });
 
+  it('adds a direct vitest dep when a vite config enables browser mode', () => {
+    // A package whose vite config imports a browser provider but has no direct
+    // vitest dep — `@vitest/browser` needs `vitest` resolvable from the package
+    // root, so the migration must pin it. Mirrors the vibe-dashboard regression.
+    fs.writeFileSync(
+      path.join(tmpDir, 'package.json'),
+      JSON.stringify({ name: 'test', devDependencies: { playwright: '^1.58.0' } }),
+    );
+    fs.writeFileSync(
+      path.join(tmpDir, 'vite.config.ts'),
+      [
+        "import { playwright } from 'vite-plus/test/browser-playwright';",
+        "import { defineConfig } from 'vite-plus';",
+        'export default defineConfig({',
+        '  test: { browser: { enabled: true, provider: playwright() } },',
+        '});',
+        '',
+      ].join('\n'),
+    );
+    rewriteStandaloneProject(tmpDir, makeWorkspaceInfo(tmpDir, PackageManager.pnpm), true, true);
+
+    const pkg = readJson(path.join(tmpDir, 'package.json'));
+    const devDeps = pkg.devDependencies as Record<string, string>;
+    expect(devDeps.vitest).toBe('catalog:');
+    expect(devDeps['vite-plus']).toBe('catalog:');
+  });
+
+  it('detects browser mode from a test file when the config has no hint', () => {
+    // Browser config can live in a workspace-referenced config under any name;
+    // the source scan also catches `@vitest/browser*` imports in test files.
+    fs.writeFileSync(
+      path.join(tmpDir, 'package.json'),
+      JSON.stringify({ name: 'test', devDependencies: { vite: '^7.0.0' } }),
+    );
+    fs.mkdirSync(path.join(tmpDir, 'src', '__tests__'), { recursive: true });
+    fs.writeFileSync(
+      path.join(tmpDir, 'src', '__tests__', 'app.test.ts'),
+      "import { page } from '@vitest/browser/context';\n",
+    );
+    rewriteStandaloneProject(tmpDir, makeWorkspaceInfo(tmpDir, PackageManager.pnpm), true, true);
+
+    const devDeps = readJson(path.join(tmpDir, 'package.json')).devDependencies as Record<
+      string,
+      string
+    >;
+    expect(devDeps.vitest).toBe('catalog:');
+  });
+
+  it('does not add vitest for a package without browser mode', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, 'package.json'),
+      JSON.stringify({ name: 'test', devDependencies: { vite: '^7.0.0' } }),
+    );
+    fs.writeFileSync(
+      path.join(tmpDir, 'vite.config.ts'),
+      "import { defineConfig } from 'vite';\nexport default defineConfig({});\n",
+    );
+    rewriteStandaloneProject(tmpDir, makeWorkspaceInfo(tmpDir, PackageManager.pnpm), true, true);
+
+    const devDeps = readJson(path.join(tmpDir, 'package.json')).devDependencies as Record<
+      string,
+      string
+    >;
+    expect(devDeps).not.toHaveProperty('vitest');
+  });
+
   it('preserves named pnpm overrides when moving root overrides to pnpm-workspace.yaml', () => {
     fs.writeFileSync(
       path.join(tmpDir, 'package.json'),
@@ -888,6 +1003,51 @@ describe('rewriteStandaloneProject pnpm workspace yaml', () => {
     // resolve to the override target are coerced to '*'.
     expect(pkg.peerDependencies.vitest).toBe('*');
   });
+
+  it('adds vitest only to the monorepo package that uses browser mode', () => {
+    // Root has no browser config; only `apps/dashboard` does. The browser-mode
+    // scan must stop at the nested package.json boundary so the root package
+    // does not inherit the sub-package's signal.
+    fs.writeFileSync(
+      path.join(tmpDir, 'package.json'),
+      JSON.stringify({ name: 'root', devDependencies: {} }),
+    );
+    fs.writeFileSync(path.join(tmpDir, 'pnpm-workspace.yaml'), 'packages:\n  - apps/*\n');
+    const appDir = path.join(tmpDir, 'apps', 'dashboard');
+    fs.mkdirSync(appDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(appDir, 'package.json'),
+      JSON.stringify({ name: '@vibe/dashboard', devDependencies: { playwright: '^1.58.0' } }),
+    );
+    fs.writeFileSync(
+      path.join(appDir, 'vite.config.ts'),
+      [
+        "import { playwright } from 'vite-plus/test/browser-playwright';",
+        "import { defineConfig } from 'vite-plus';",
+        'export default defineConfig({ test: { browser: { provider: playwright() } } });',
+        '',
+      ].join('\n'),
+    );
+
+    const workspaceInfo = makeWorkspaceInfo(tmpDir, PackageManager.pnpm);
+    workspaceInfo.isMonorepo = true;
+    workspaceInfo.packages = [
+      { name: '@vibe/dashboard', path: 'apps/dashboard', isTemplatePackage: false },
+    ];
+    rewriteMonorepo(workspaceInfo, true);
+
+    const rootDeps = (readJson(path.join(tmpDir, 'package.json')).devDependencies ?? {}) as Record<
+      string,
+      string
+    >;
+    expect(rootDeps).not.toHaveProperty('vitest');
+
+    const appDeps = readJson(path.join(appDir, 'package.json')).devDependencies as Record<
+      string,
+      string
+    >;
+    expect(appDeps.vitest).toBe('catalog:');
+  });
 });
 
 describe('rewriteMonorepo yarn catalog', () => {
diff --git a/packages/cli/src/migration/migrator.ts b/packages/cli/src/migration/migrator.ts
@@ -992,6 +992,7 @@ export function rewriteStandaloneProject(
       usePnpmWorkspaceYaml,
       skipStagedMigration,
       catalogDependencyResolver,
+      usesVitestBrowserMode(projectPath),
     );
 
     // ensure vite-plus is in devDependencies
@@ -1144,6 +1145,7 @@ export function rewriteMonorepoProject(
       true,
       skipStagedMigration,
       catalogDependencyResolver,
+      usesVitestBrowserMode(projectPath),
     );
     return pkg;
   });
@@ -1853,6 +1855,105 @@ function readPrepareRulesYaml(): string {
   return cachedPrepareRulesYaml;
 }
 
+// Specifier fragments that signal vitest browser mode. `@vitest/browser`
+// (upstream, pre-migration) and `vite-plus/test/browser` (post-migration, e.g.
+// re-migrating an already-migrated project) both indicate the package drives
+// vitest's browser runner.
+const VITEST_BROWSER_SPECIFIER_HINTS = ['@vitest/browser', 'vite-plus/test/browser'] as const;
+
+// TypeScript/JavaScript source extensions scanned for browser-mode hints.
+const VITEST_SCAN_EXTENSIONS = new Set([
+  '.ts',
+  '.mts',
+  '.cts',
+  '.tsx',
+  '.js',
+  '.mjs',
+  '.cjs',
+  '.jsx',
+]);
+
+// Directories never worth scanning for browser-mode hints — generated output,
+// installed deps, VCS metadata. Skipped at every recursion level.
+const VITEST_SCAN_SKIP_DIRS = new Set([
+  'node_modules',
+  'dist',
+  'build',
+  'out',
+  'coverage',
+  '.git',
+  '.next',
+  '.nuxt',
+  '.svelte-kit',
+  '.vite',
+  '.cache',
+]);
+
+/**
+ * Detect whether a package uses vitest's browser mode.
+ *
+ * Upstream `@vitest/browser` injects `optimizeDeps.include` entries of the form
+ * `vitest > expect-type` (and `vitest > @vitest/snapshot > magic-string`,
+ * `vitest > @vitest/expect > chai`). Vite resolves the leading `vitest` segment
+ * from the Vite config root, so `vitest` MUST be resolvable as a package from
+ * the consuming package's directory. In a pnpm strict (non-hoisted) layout,
+ * `vitest` pulled in only transitively via `vite-plus` is NOT reachable from the
+ * package root — the optimizer then fails with `Failed to resolve dependency`
+ * and the browser test page hangs forever.
+ *
+ * When this returns true the migration adds `vitest` as a direct
+ * devDependency so it is hoisted next to the package and the optimizer chain
+ * resolves. The signal is any of the package's TS/JS files (config, workspace
+ * config under any name, or test file) referencing `@vitest/browser*` or
+ * `vite-plus/test/browser*`. The scan recurses through the package directory
+ * (skipping `node_modules`, build output, VCS metadata) so browser config in a
+ * non-standard filename or browser imports in test files are all caught.
+ *
+ * Recursion stops at nested `package.json` boundaries: a workspace sub-package
+ * is a separate package that the migration scans on its own pass, so the root
+ * package must not inherit a browser-mode signal from a sub-package.
+ */
+function usesVitestBrowserMode(projectPath: string): boolean {
+  const matchesBrowserHint = (content: string): boolean =>
+    VITEST_BROWSER_SPECIFIER_HINTS.some((hint) => content.includes(hint));
+
+  const scanDir = (dir: string, isRoot: boolean): boolean => {
+    let entries: fs.Dirent[];
+    try {
+      entries = fs.readdirSync(dir, { withFileTypes: true });
+    } catch {
+      return false;
+    }
+    // A nested package.json marks a separate workspace package — it is migrated
+    // (and scanned) on its own pass, so don't let its files leak into this one.
+    if (!isRoot && entries.some((e) => e.isFile() && e.name === 'package.json')) {
+      return false;
+    }
+    for (const entry of entries) {
+      const entryPath = path.join(dir, entry.name);
+      if (entry.isDirectory()) {
+        if (VITEST_SCAN_SKIP_DIRS.has(entry.name)) {
+          continue;
+        }
+        if (scanDir(entryPath, false)) {
+          return true;
+        }
+      } else if (entry.isFile() && VITEST_SCAN_EXTENSIONS.has(path.extname(entry.name))) {
+        try {
+          if (matchesBrowserHint(fs.readFileSync(entryPath, 'utf8'))) {
+            return true;
+          }
+        } catch {
+          // Unreadable file — ignore and keep scanning.
+        }
+      }
+    }
+    return false;
+  };
+
+  return scanDir(projectPath, true);
+}
+
 export function rewritePackageJson(
   pkg: {
     scripts?: Record<string, string>;
@@ -1866,6 +1967,7 @@ export function rewritePackageJson(
   isMonorepo?: boolean,
   skipStagedMigration?: boolean,
   catalogDependencyResolver?: CatalogDependencyResolver,
+  vitestBrowserMode?: boolean,
 ): Record<string, string | string[]> | null {
   if (pkg.scripts) {
     const updated = rewriteScripts(
@@ -1973,6 +2075,15 @@ export function rewritePackageJson(
   if (isVitestAdjacent) {
     needVitePlus = true;
   }
+  // A package running vitest browser mode must have `vitest` directly
+  // resolvable from its own directory — `@vitest/browser` injects
+  // `optimizeDeps.include` entries (`vitest > expect-type`, etc.) that Vite
+  // resolves from the config root. A `vitest` pulled in only transitively via
+  // `vite-plus` is invisible in a pnpm strict layout, so the optimizer fails
+  // and the browser test page hangs. See `usesVitestBrowserMode`.
+  if (vitestBrowserMode && !installableNames.includes('vitest')) {
+    needVitePlus = true;
+  }
   if (needVitePlus) {
     // add vite-plus to devDependencies
     const version =
@@ -1993,9 +2104,18 @@ export function rewritePackageJson(
       ...pkg.devDependencies,
       ...pkg.optionalDependencies,
     };
+    // Add `vitest` as a direct devDependency when:
+    //  - a remaining dependency likely peer-depends on vitest (e.g.
+    //    vitest-browser-svelte), OR
+    //  - the package runs vitest browser mode (`@vitest/browser` needs
+    //    `vitest` resolvable from the package root — see usesVitestBrowserMode).
+    // Vite-plus already bundles upstream vitest 4.1.7 as a direct dep, but a
+    // strict pnpm / yarn PnP layout will not expose that transitive `vitest`
+    // to the package. Pinning it here points the dep at the same upstream
+    // version vite-plus ships with.
     if (
       !installableDeps.vitest &&
-      Object.keys(installableDeps).some((name) => name.includes('vitest'))
+      (vitestBrowserMode || Object.keys(installableDeps).some((name) => name.includes('vitest')))
     ) {
       pkg.devDependencies.vitest = getCatalogDependencySpec(
         undefined,
