diff --git a/packages/cli/src/migration/__tests__/migrator.spec.ts b/packages/cli/src/migration/__tests__/migrator.spec.ts index d75bbb879f..e3afc5338c 100644 --- a/packages/cli/src/migration/__tests__/migrator.spec.ts +++ b/packages/cli/src/migration/__tests__/migrator.spec.ts @@ -9237,3 +9237,74 @@ describe('collectMigrationSetupPlan non-interactive editor conflicts', () => { expect(plan.editorConflictDecisions.get('workspace.xml')).toBe('skip'); }); }); + +// Counts how many times each of `sourceFiles` is read through `fs.readFileSync` +// while `run()` executes. Used to assert the migration's source-tree scans do +// not re-read the same file once per signal (#2420). +function countReadsPerSourceFile(sourceFiles: string[], run: () => void): Map { + const tracked = new Set(sourceFiles); + const counts = new Map(sourceFiles.map((file) => [file, 0])); + const readFileSync = fs.readFileSync.bind(fs); + vi.spyOn(fs, 'readFileSync').mockImplementation(((file: unknown, ...rest: unknown[]) => { + if (typeof file === 'string' && tracked.has(file)) { + counts.set(file, (counts.get(file) ?? 0) + 1); + } + return (readFileSync as (...args: unknown[]) => unknown)(file, ...rest); + }) as typeof fs.readFileSync); + run(); + return counts; +} + +describe('source-tree scan read amplification (#2420)', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vp-test-scan-reads-')); + }); + + afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + // Full-miss fixture: no vitest browser/provider references anywhere, no + // tsconfig retaining vitest types, and no webdriverio/provider dependency to + // short-circuit a scan. Every signal therefore takes its complete miss path, + // which is the worst case for traversal count. + function writeFullMissProject(fileCount: number): string[] { + fs.writeFileSync( + path.join(tmpDir, 'package.json'), + JSON.stringify({ + name: 'scan-reads', + devDependencies: { vite: '^7.0.0' }, + }), + ); + const srcDir = path.join(tmpDir, 'src'); + fs.mkdirSync(srcDir); + const sourceFiles: string[] = []; + for (let index = 0; index < fileCount; index++) { + const filePath = path.join(srcDir, `mod-${index}.ts`); + fs.writeFileSync(filePath, `export const value${index} = ${index};\n`); + sourceFiles.push(filePath); + } + return sourceFiles; + } + + it('reads each source file once during a standalone migration', () => { + const sourceFiles = writeFullMissProject(6); + const workspaceInfo = makeWorkspaceInfo(tmpDir, PackageManager.pnpm); + + const counts = countReadsPerSourceFile(sourceFiles, () => { + rewriteStandaloneProject(tmpDir, workspaceInfo, true, true); + }); + + // Every eligible source file must be visited, so a zero count would mean the + // fixture stopped exercising the scan rather than that the scan got faster. + for (const file of sourceFiles) { + expect(counts.get(file)).toBeGreaterThan(0); + } + // Before the single-traversal change this was 5: two provider scans, browser + // mode, the retained-vitest-module scan, and a redundant webdriverio rescan. + expect(Math.max(...counts.values())).toBe(1); + }); +}); diff --git a/packages/cli/src/migration/migrator/orchestrators.ts b/packages/cli/src/migration/migrator/orchestrators.ts index 86a6f02225..25805b766e 100644 --- a/packages/cli/src/migration/migrator/orchestrators.ts +++ b/packages/cli/src/migration/migrator/orchestrators.ts @@ -9,7 +9,7 @@ import { applyYarnWorkspaceHoistingFix, cleanupDeprecatedTsconfigOptions, collectInjectedProviderNames, - collectProviderSourceModes, + collectPackageSourceScanSignals, collectVitestEcosystemInstallDependencyNames, createCatalogDependencyResolver, dropRemovePackageOverrideKeys, @@ -45,10 +45,7 @@ import { rewriteYarnrcYml, setDirectViteEdge, setPackageManager, - sourceTreeReferencesRetainedVitestModule, takePnpmWorkspaceSettings, - usesVitestBrowserMode, - usesWebdriverioProvider, workspaceUsesVitestDirectly, workspaceUsesWebdriverio, wrapLazyPluginsInViteConfig, @@ -56,6 +53,7 @@ import { import { type MigrationReport } from '../report.ts'; import { PROVIDER_OVERRIDE_DROP_NAMES, + WEBDRIVERIO_PROVIDER, pnpmMajor, type CatalogDependencyResolver, type PnpmPackageJsonSettings, @@ -78,12 +76,14 @@ export function rewriteStandaloneProject( const vitestEcosystemPackages = collectVitestEcosystemInstallDependencyNames(projectPath); // Source-tree scan signals are computed once here and reused below (and inside // projectUsesVitestDirectly / collectInjectedProviderNames) so the source tree - // is traversed once each instead of repeatedly. They do not depend on + // is traversed once instead of repeatedly. They do not depend on // package.json contents and no scanned source files are mutated before they // are consumed, so the values match the previous lazy per-call scans exactly. - const providerSourceModes = collectProviderSourceModes(projectPath); - const browserMode = usesVitestBrowserMode(projectPath); - const retainedVitestModule = sourceTreeReferencesRetainedVitestModule(projectPath); + const { + providerSourceModes, + browserMode, + retainedModule: retainedVitestModule, + } = collectPackageSourceScanSignals(projectPath); const providerCatalogAdditions = collectInjectedProviderNames( projectPath, undefined, @@ -124,8 +124,10 @@ export function rewriteStandaloneProject( scripts?: Record; pnpm?: PnpmPackageJsonSettings; }>(packageJsonPath, (pkg) => { + // `providerSourceModes` already carries the webdriverio source-scan result + // from the single scan above, so reuse it instead of rescanning the tree. shouldAllowBrowserProviderBuilds = - hasOwnWebdriverioDependency(pkg) || usesWebdriverioProvider(projectPath); + hasOwnWebdriverioDependency(pkg) || providerSourceModes[WEBDRIVERIO_PROVIDER] === true; const requiredVitestPeer = projectListsRequiredVitestPeer(projectPath, pkg); usesVitest = projectUsesVitestDirectly(projectPath, pkg, requiredVitestPeer, true, { browserMode, @@ -535,12 +537,16 @@ export function rewriteMonorepoProject( installConfig?: { hoistingLimits?: string }; }>(packageJsonPath, (pkg) => { const requiredVitestPeer = projectListsRequiredVitestPeer(projectPath, pkg); - // Compute the browser-mode and retained-module source scans once and reuse - // them across rewritePackageJson and projectUsesVitestDirectly: the scans do - // not depend on package.json and nothing mutates the source tree between - // these reads, so this is identical to the previous per-call scans. - const browserMode = usesVitestBrowserMode(projectPath); - const retainedVitestModule = sourceTreeReferencesRetainedVitestModule(projectPath); + // Compute the browser-mode, retained-module and provider source scans in a + // single traversal and reuse them across rewritePackageJson and + // projectUsesVitestDirectly: the scans do not depend on package.json and + // nothing mutates the source tree between these reads, so this is identical + // to the previous per-call scans. + const { + browserMode, + retainedModule: retainedVitestModule, + providerSourceModes, + } = collectPackageSourceScanSignals(projectPath); // rewrite scripts in package.json extractedStagedConfig = rewritePackageJson( pkg, @@ -549,7 +555,7 @@ export function rewriteMonorepoProject( skipStagedMigration, catalogDependencyResolver, browserMode, - collectProviderSourceModes(projectPath), + providerSourceModes, projectUsesVitestDirectly(projectPath, pkg, requiredVitestPeer, true, { browserMode, retainedModule: retainedVitestModule, diff --git a/packages/cli/src/migration/migrator/source-scan.ts b/packages/cli/src/migration/migrator/source-scan.ts index 32d32877c8..c813fdbd3e 100644 --- a/packages/cli/src/migration/migrator/source-scan.ts +++ b/packages/cli/src/migration/migrator/source-scan.ts @@ -216,6 +216,32 @@ function sourceTreeMatches( projectPath: string, matchesContent: (content: string) => boolean, ): boolean { + return sourceTreeMatchesEach(projectPath, [matchesContent])[0]; +} + +/** + * Evaluate several independent content predicates in a SINGLE traversal. + * + * Each eligible source file is read exactly once and offered to every predicate + * that has not yet matched. Traversal stops as soon as every predicate has + * matched, mirroring the early return of a single-predicate scan. + * + * Semantics per predicate are identical to running `sourceTreeMatches` for it + * on its own — same traversal order, same skip directories, same nested-package + * boundary, same "unreadable file is ignored" behaviour. Only the number of + * reads changes: N predicates cost one pass instead of N. + */ +function sourceTreeMatchesEach( + projectPath: string, + predicates: readonly ((content: string) => boolean)[], +): boolean[] { + const results = Array.from({ length: predicates.length }, () => false); + let undecided = predicates.length; + if (undecided === 0) { + return results; + } + + // Returns true to unwind the whole traversal once nothing is left to decide. const scanDir = (dir: string, isRoot: boolean): boolean => { let entries: fs.Dirent[]; try { @@ -238,23 +264,54 @@ function sourceTreeMatches( return true; } } else if (entry.isFile() && VITEST_SCAN_EXTENSIONS.has(path.extname(entry.name))) { + let content: string; try { - if (matchesContent(fs.readFileSync(entryPath, 'utf8'))) { - return true; - } + content = fs.readFileSync(entryPath, 'utf8'); } catch { // Unreadable file — ignore and keep scanning. + continue; + } + for (let index = 0; index < predicates.length; index++) { + if (results[index]) { + continue; + } + if (predicates[index](content)) { + results[index] = true; + undecided--; + } + } + if (undecided === 0) { + return true; } } } return false; }; - return scanDir(projectPath, true); + scanDir(projectPath, true); + return results; +} + +// A hint list as a reusable content predicate, so the same matcher can be run +// standalone or enrolled in a combined multi-signal traversal. +function hintMatcher(hints: readonly string[]): (content: string) => boolean { + return (content) => hints.some((hint) => content.includes(hint)); } function sourceTreeReferencesAny(projectPath: string, hints: readonly string[]): boolean { - return sourceTreeMatches(projectPath, (content) => hints.some((hint) => content.includes(hint))); + return sourceTreeMatches(projectPath, hintMatcher(hints)); +} + +// Source surfaces that deliberately retain the upstream `vitest` package +// identity. Extracted so the combined scan and the standalone helper share one +// definition. See `sourceTreeReferencesRetainedVitestModule`. +function matchesRetainedVitestModule(content: string): boolean { + return ( + /\bdeclare\s+module\s+['"]vitest(?:\/[^'"]*)?['"]/.test(content) || + content.includes('vitest/package.json') || + /\brequire\.resolve\s*\(\s*['"]vitest(?:\/[^'"]*)?['"]/.test(content) || + /\bimport\.meta\.resolve\s*\(\s*['"]vitest(?:\/[^'"]*)?['"]/.test(content) + ); } function findPackageTsconfigFiles(projectPath: string): string[] { @@ -298,14 +355,7 @@ export function hasNuxtTestUtilsDependency(pkg: DependencyBag): boolean { export function sourceTreeReferencesRetainedVitestModule(projectPath: string): boolean { return ( findPackageTsconfigFiles(projectPath).some(hasVitestTypesInTsconfig) || - sourceTreeMatches(projectPath, (content) => { - return ( - /\bdeclare\s+module\s+['"]vitest(?:\/[^'"]*)?['"]/.test(content) || - content.includes('vitest/package.json') || - /\brequire\.resolve\s*\(\s*['"]vitest(?:\/[^'"]*)?['"]/.test(content) || - /\bimport\.meta\.resolve\s*\(\s*['"]vitest(?:\/[^'"]*)?['"]/.test(content) - ); - }) + sourceTreeMatches(projectPath, matchesRetainedVitestModule) ); } @@ -327,12 +377,65 @@ export function usesWebdriverioProvider(projectPath: string): boolean { // yet (e.g. a `vite.config.ts` importing the provider via a `vite-plus/test` // shim). Mirrors `usesWebdriverioProvider`'s scan for each provider. export function collectProviderSourceModes(projectPath: string): Record { + const matches = sourceTreeMatchesEach( + projectPath, + OPT_IN_BROWSER_PROVIDERS.map((provider) => + hintMatcher(BROWSER_PROVIDER_SPECIFIER_HINTS[provider]), + ), + ); const modes: Record = {}; - for (const provider of OPT_IN_BROWSER_PROVIDERS) { - modes[provider] = sourceTreeReferencesAny( - projectPath, - BROWSER_PROVIDER_SPECIFIER_HINTS[provider], - ); - } + OPT_IN_BROWSER_PROVIDERS.forEach((provider, index) => { + modes[provider] = matches[index]; + }); return modes; } + +/** All per-package source-scan signals the migration needs for one package. */ +export interface PackageSourceScanSignals { + /** @see usesVitestBrowserMode */ + browserMode: boolean; + /** @see sourceTreeReferencesRetainedVitestModule */ + retainedModule: boolean; + /** @see collectProviderSourceModes */ + providerSourceModes: Record; +} + +/** + * Collect every per-package source signal in ONE traversal of the package tree. + * + * `usesVitestBrowserMode`, `sourceTreeReferencesRetainedVitestModule` and each + * provider scan in `collectProviderSourceModes` all walk the same file set with + * the same rules, so running them separately read every eligible source file + * once per signal. Enrolling them as predicates in a single pass makes that one + * read per file while leaving each individual result unchanged. + * + * The tsconfig `types` short-circuit of `sourceTreeReferencesRetainedVitestModule` + * is preserved: when a tsconfig already settles the retained-module signal, its + * source predicate is not enrolled at all, so the tree is not scanned for it. + */ +export function collectPackageSourceScanSignals(projectPath: string): PackageSourceScanSignals { + const retainedFromTsconfig = findPackageTsconfigFiles(projectPath).some(hasVitestTypesInTsconfig); + + const predicates: ((content: string) => boolean)[] = [ + hintMatcher(VITEST_BROWSER_SPECIFIER_HINTS), + ...OPT_IN_BROWSER_PROVIDERS.map((provider) => + hintMatcher(BROWSER_PROVIDER_SPECIFIER_HINTS[provider]), + ), + ]; + if (!retainedFromTsconfig) { + predicates.push(matchesRetainedVitestModule); + } + + const matches = sourceTreeMatchesEach(projectPath, predicates); + + const providerSourceModes: Record = {}; + OPT_IN_BROWSER_PROVIDERS.forEach((provider, index) => { + providerSourceModes[provider] = matches[index + 1]; + }); + + return { + browserMode: matches[0], + retainedModule: retainedFromTsconfig || matches[1 + OPT_IN_BROWSER_PROVIDERS.length] === true, + providerSourceModes, + }; +}