From f812967cb207cf1bc32a739c1c53c75c3c163cc3 Mon Sep 17 00:00:00 2001 From: JongKyung Lee Date: Fri, 23 Oct 2026 16:35:15 +0900 Subject: [PATCH] perf(migrate): avoid repeated lockfile parsing during preflight Reuse parsed lockfiles within each Vitest migration plan across pnpm, npm, Yarn and Bun. Keep version resolution package-specific and create fresh readers for subsequent plans. Cover parsing reuse, mixed runner versions, changed inputs, malformed lockfiles and npm shrinkwrap precedence. --- .../__tests__/vitest-v5-lockfile.spec.ts | 181 +++++++++++++++++- .../cli/src/migration/migrator/vitest-v5.ts | 43 +++-- .../cli/src/migration/vitest-v5/lockfile.ts | 90 ++++++--- 3 files changed, 268 insertions(+), 46 deletions(-) diff --git a/packages/cli/src/migration/__tests__/vitest-v5-lockfile.spec.ts b/packages/cli/src/migration/__tests__/vitest-v5-lockfile.spec.ts index 8ce6932299..f4d4b36d76 100644 --- a/packages/cli/src/migration/__tests__/vitest-v5-lockfile.spec.ts +++ b/packages/cli/src/migration/__tests__/vitest-v5-lockfile.spec.ts @@ -2,19 +2,33 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; +import { parse as parseJsonc } from 'jsonc-parser'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { parseDocument } from 'yaml'; import { PackageManager } from '../../types/index.ts'; import { applyVitestV5Migration, planVitestV5Migration } from '../migrator.ts'; +vi.mock('yaml', async (importOriginal) => { + const yaml = await importOriginal(); + return { ...yaml, parseDocument: vi.fn(yaml.parseDocument) }; +}); + +vi.mock('jsonc-parser', async (importOriginal) => { + const jsonc = await importOriginal(); + return { ...jsonc, parse: vi.fn(jsonc.parse) }; +}); + const directories: string[] = []; afterEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); for (const directory of directories.splice(0)) { fs.rmSync(directory, { recursive: true, force: true }); } }); -const formats = ['npm', 'yarn', 'yarn-classic', 'bun'] as const; +const formats = ['pnpm', 'npm', 'npm-shrinkwrap', 'yarn', 'yarn-classic', 'bun'] as const; function project( format: (typeof formats)[number], spec = '^4 || ^5', @@ -31,8 +45,16 @@ function project( const manifest = { devDependencies: { vitest: lockedSpec } }; let filename: string; let content: string; - if (format === 'npm') { - filename = 'package-lock.json'; + if (format === 'pnpm') { + filename = 'pnpm-lock.yaml'; + content = JSON.stringify({ + lockfileVersion: '9.0', + importers: { + '.': { devDependencies: { vitest: { specifier: lockedSpec, version } } }, + }, + }); + } else if (format === 'npm' || format === 'npm-shrinkwrap') { + filename = format === 'npm' ? 'package-lock.json' : 'npm-shrinkwrap.json'; content = JSON.stringify({ lockfileVersion: 3, packages: { '': manifest, 'node_modules/vitest': { version } }, @@ -52,15 +74,21 @@ function project( content = `# yarn lockfile v1\n\n${JSON.stringify(`vitest@${lockedSpec}`)}:\n version "${version}"\n resolved "https://registry.npmjs.org/vitest/-/vitest-${version}.tgz"\n`; } fs.writeFileSync(path.join(root, filename), content); - const manager = format === 'yarn-classic' ? PackageManager.yarn : PackageManager[format]; + const manager = + format === 'yarn-classic' + ? PackageManager.yarn + : format === 'npm-shrinkwrap' + ? PackageManager.npm + : PackageManager[format]; return { root, + manager, file: path.join(root, filename), plan: () => planVitestV5Migration({ rootDir: root, packageManager: manager }), }; } -describe('Vitest source versions from non-pnpm lockfiles', () => { +describe('Vitest source versions from lockfiles', () => { it.each( formats.flatMap((format) => ['4.1.11', '5.0.0'].map((version) => [format, version] as const)), )('uses the original %s lockfile runner %s without node_modules', (format, version) => { @@ -118,6 +146,147 @@ describe('Vitest source versions from non-pnpm lockfiles', () => { expect(plan.projects.map(({ sourceVersion }) => sourceVersion)).toEqual(['5.0.0', '4.1.11']); }); + it('reuses a pnpm lockfile within a plan without sharing versions between importers or plans', () => { + const fixture = project('pnpm'); + const packages = [ + { name: 'unit', path: 'packages/unit' }, + { name: 'inherited', path: 'packages/inherited' }, + ]; + for (const pkg of packages) { + fs.mkdirSync(path.join(fixture.root, pkg.path), { recursive: true }); + fs.writeFileSync( + path.join(fixture.root, pkg.path, 'package.json'), + JSON.stringify(pkg.name === 'unit' ? { devDependencies: { vitest: '^4 || ^5' } } : {}), + ); + } + const lock = JSON.parse(fs.readFileSync(fixture.file, 'utf8')); + lock.importers['packages/unit'] = { + devDependencies: { vitest: { specifier: '^4 || ^5', version: '5.0.0' } }, + }; + lock.importers['packages/inherited'] = {}; + fs.writeFileSync(fixture.file, JSON.stringify(lock)); + const workspace = { rootDir: fixture.root, packageManager: PackageManager.pnpm, packages }; + const plan = planVitestV5Migration(workspace); + expect(plan.projects.map(({ sourceVersion }) => sourceVersion)).toEqual([ + '4.1.11', + '5.0.0', + '4.1.11', + ]); + expect(parseDocument).toHaveBeenCalledTimes(1); + + lock.importers['.'].devDependencies.vitest.version = '5.0.0'; + fs.writeFileSync(fixture.file, JSON.stringify(lock)); + expect(() => applyVitestV5Migration(plan)).toThrow('Migration input changed'); + expect( + planVitestV5Migration(workspace).projects.map(({ sourceVersion }) => sourceVersion), + ).toEqual(['5.0.0', '5.0.0', '5.0.0']); + expect(parseDocument).toHaveBeenCalledTimes(2); + + fs.writeFileSync(fixture.file, '{ invalid'); + expect( + planVitestV5Migration(workspace).projects.map(({ sourceVersion }) => sourceVersion), + ).toEqual([undefined, undefined, undefined]); + expect(parseDocument).toHaveBeenCalledTimes(3); + fs.writeFileSync(fixture.file, JSON.stringify(lock)); + expect( + planVitestV5Migration(workspace).projects.map(({ sourceVersion }) => sourceVersion), + ).toEqual(['5.0.0', '5.0.0', '5.0.0']); + expect(parseDocument).toHaveBeenCalledTimes(4); + }); + + it.each(formats)('reads and parses the %s lockfile once per plan', (format) => { + const fixture = project(format); + const member = 'packages/unit'; + fs.mkdirSync(path.join(fixture.root, member), { recursive: true }); + fs.writeFileSync( + path.join(fixture.root, member, 'package.json'), + JSON.stringify({ devDependencies: { vitest: '^4 || ^5' } }), + ); + if (format === 'pnpm' || fixture.manager === PackageManager.npm || format === 'bun') { + const lock = JSON.parse(fs.readFileSync(fixture.file, 'utf8')); + const importers = + format === 'pnpm' + ? lock.importers + : fixture.manager === PackageManager.npm + ? lock.packages + : lock.workspaces; + importers[member] = importers[format === 'pnpm' ? '.' : '']; + fs.writeFileSync(fixture.file, JSON.stringify(lock)); + } + const before = fs.readFileSync(fixture.file, 'utf8'); + const reads = vi.spyOn(fs, 'readFileSync'); + const json = vi.spyOn(JSON, 'parse'); + const workspace = { + rootDir: fixture.root, + packageManager: fixture.manager, + packages: [{ name: 'unit', path: member }], + }; + const plan = planVitestV5Migration(workspace); + expect(plan.projects.map(({ sourceVersion }) => sourceVersion)).toEqual(['4.1.11', '4.1.11']); + // One read captures the input snapshot and one parses the lockfile. + expect(reads.mock.calls.filter(([file]) => file === fixture.file)).toHaveLength(2); + if (fixture.manager === PackageManager.npm) { + expect(json.mock.calls.filter(([source]) => source === before)).toHaveLength(1); + } else if (format === 'bun') { + expect(vi.mocked(parseJsonc).mock.calls.filter(([source]) => source === before)).toHaveLength( + 1, + ); + } else { + expect(parseDocument).toHaveBeenCalledTimes(1); + } + + const after = before.replaceAll('4.1.11', '5.0.0'); + fs.writeFileSync(fixture.file, after); + expect(() => applyVitestV5Migration(plan)).toThrow('Migration input changed'); + expect( + planVitestV5Migration(workspace).projects.map(({ sourceVersion }) => sourceVersion), + ).toEqual(['5.0.0', '5.0.0']); + + fs.writeFileSync(fixture.file, '{ invalid'); + reads.mockClear(); + expect( + planVitestV5Migration(workspace).projects.map(({ sourceVersion }) => sourceVersion), + ).toEqual([undefined, undefined]); + expect(reads.mock.calls.filter(([file]) => file === fixture.file)).toHaveLength(2); + fs.writeFileSync(fixture.file, before); + expect( + planVitestV5Migration(workspace).projects.map(({ sourceVersion }) => sourceVersion), + ).toEqual(['4.1.11', '4.1.11']); + }); + + it.each(['yarn', 'yarn-classic'] as const)( + 'keeps %s descriptor resolution package-specific when sharing a lockfile', + (format) => { + const fixture = project(format, '^4.1.0'); + fs.mkdirSync(path.join(fixture.root, 'unit')); + fs.writeFileSync( + path.join(fixture.root, 'unit/package.json'), + JSON.stringify({ devDependencies: { vitest: '^5.0.0' } }), + ); + fs.appendFileSync( + fixture.file, + format === 'yarn' + ? '\n"vitest@npm:^5.0.0":\n resolution: "vitest@npm:5.0.0"\n' + : '\n"vitest@^5.0.0":\n version "5.0.0"\n', + ); + const plan = planVitestV5Migration({ + rootDir: fixture.root, + packageManager: PackageManager.yarn, + packages: [{ name: 'unit', path: 'unit' }], + }); + expect(plan.projects.map(({ sourceVersion }) => sourceVersion)).toEqual(['4.1.11', '5.0.0']); + expect(parseDocument).toHaveBeenCalledTimes(format === 'yarn' ? 1 : 2); + }, + ); + + it('prefers npm shrinkwrap over package-lock.json', () => { + const fixture = project('npm-shrinkwrap'); + const otherLock = JSON.parse(fs.readFileSync(fixture.file, 'utf8')); + otherLock.packages['node_modules/vitest'].version = '5.0.0'; + fs.writeFileSync(path.join(fixture.root, 'package-lock.json'), JSON.stringify(otherLock)); + expect(fixture.plan().projects[0].sourceVersion).toBe('4.1.11'); + }); + it('handles grouped Yarn classic descriptors', () => { const fixture = project('yarn-classic'); fs.writeFileSync( diff --git a/packages/cli/src/migration/migrator/vitest-v5.ts b/packages/cli/src/migration/migrator/vitest-v5.ts index f9c91117ec..3113afaf90 100644 --- a/packages/cli/src/migration/migrator/vitest-v5.ts +++ b/packages/cli/src/migration/migrator/vitest-v5.ts @@ -31,7 +31,11 @@ import { migrateVitestV5Config, } from '../vitest-v5/config.ts'; import { vitestV5Documentation } from '../vitest-v5/documentation.ts'; -import { lockedVitestVersion } from '../vitest-v5/lockfile.ts'; +import { + createLockfileReader, + lockedVitestVersion, + type LockfileReader, +} from '../vitest-v5/lockfile.ts'; import { findVitestV5ConfigEntries, resolveVitestV5TestModes, @@ -180,25 +184,21 @@ function lockedSourceVersion( directory: string, root: string, pkg: Record, + readLockfile: LockfileReader, manager?: PackageManager, expectedRunnerPackage?: string, ): string | undefined { if (manager !== undefined && manager !== PackageManager.pnpm) { const spec = dependency(pkg, 'vitest'); return spec && !expectedRunnerPackage - ? lockedVitestVersion(root, directory, manager, spec) + ? lockedVitestVersion(root, directory, manager, spec, readLockfile) : undefined; } - const lockfile = path.join(root, 'pnpm-lock.yaml'); - if (!fs.existsSync(lockfile)) { - return undefined; - } try { - const document = parseDocument(fs.readFileSync(lockfile, 'utf8')); - if (document.errors.length) { - return undefined; - } - const lock = record(document.toJS()); + const lock = readLockfile(path.join(root, 'pnpm-lock.yaml'), (source) => { + const document = parseDocument(source); + return document.errors.length ? undefined : record(document.toJS()); + }); const importer = record( record(lock?.importers)?.[path.relative(root, directory).replaceAll('\\', '/') || '.'], ); @@ -327,6 +327,7 @@ function sourceVersion( directory: string, root: string, pkg: Record, + readLockfile: LockfileReader, manager?: PackageManager, ): string | undefined { const catalog = createCatalogDependencyResolver(root, manager ?? PackageManager.pnpm); @@ -357,7 +358,14 @@ function sourceVersion( // A stale install or catalog lock entry can still resolve upstream Vitest // under this name. Its version is not evidence for the declared wrapper. const expectedRunnerPackage = legacyAlias ? LEGACY_RUNNER : undefined; - const locked = lockedSourceVersion(directory, root, pkg, manager, expectedRunnerPackage); + const locked = lockedSourceVersion( + directory, + root, + pkg, + readLockfile, + manager, + expectedRunnerPackage, + ); if ( locked && (!spec || @@ -390,7 +398,13 @@ function sourceVersion( } // A workspace member commonly uses the root's test command and dependencies. if (directory !== root) { - return sourceVersion(root, root, readJson(path.join(root, 'package.json')), manager); + return sourceVersion( + root, + root, + readJson(path.join(root, 'package.json')), + readLockfile, + manager, + ); } return undefined; } @@ -685,6 +699,7 @@ export function planVitestV5Migration( workspace: MigrationWorkspace, originalProjects?: ReadonlyMap, ): VitestV5MigrationPlan { + const readLockfile = createLockfileReader(); const findings: VitestV5Finding[] = []; const directories = [ workspace.rootDir, @@ -726,7 +741,7 @@ export function planVitestV5Migration( const original = originalProjects?.get(directory); const version = original ? original.sourceVersion - : sourceVersion(directory, workspace.rootDir, pkg, workspace.packageManager); + : sourceVersion(directory, workspace.rootDir, pkg, readLockfile, workspace.packageManager); const active = original?.active ?? (!!dependency(pkg, 'vitest') || diff --git a/packages/cli/src/migration/vitest-v5/lockfile.ts b/packages/cli/src/migration/vitest-v5/lockfile.ts index 85e3b3a3cb..ee07925bc6 100644 --- a/packages/cli/src/migration/vitest-v5/lockfile.ts +++ b/packages/cli/src/migration/vitest-v5/lockfile.ts @@ -13,6 +13,50 @@ function record(value: unknown): Record | undefined { : undefined; } +export type LockfileReader = (file: string, parse: (source: string) => T) => T | undefined; + +/** Create one reader per preflight, with a single parser for each lockfile path. */ +export function createLockfileReader(): LockfileReader { + const locks = new Map(); + return (file: string, parse: (source: string) => T): T | undefined => { + if (!locks.has(file)) { + // Cache failed reads and parses too, until the next preflight. + locks.set(file, undefined); + try { + locks.set(file, parse(fs.readFileSync(file, 'utf8').replace(/^\uFEFF/, ''))); + } catch { + // Missing or malformed locks are not evidence of a runner version. + } + } + return locks.get(file) as T | undefined; + }; +} + +function parseYarnLockfile(text: string) { + if (/^__metadata:/m.test(text)) { + const document = parseDocument(text); + if (document.errors.length) { + return undefined; + } + return { modern: true as const, entries: record(document.toJS()) ?? {} }; + } + // Classic's quoted descriptor lists are YAML flow scalars, but its entry + // bodies are not YAML. Read only the exact descriptor and version field. + const entries: { keys: unknown[]; version: string | undefined }[] = []; + for (const block of text.split(/\r?\n(?=[^\s#])/)) { + const [header] = block.split(/\r?\n/); + if (!header.endsWith(':')) { + continue; + } + const document = parseDocument(`[${header.slice(0, -1)}]`); + const keys: unknown = document.errors.length ? undefined : document.toJS(); + if (Array.isArray(keys)) { + entries.push({ keys, version: /^ version "([^"]+)"\r?$/m.exec(block)?.[1] }); + } + } + return { modern: false as const, entries }; +} + function declaredVitest(pkg: unknown): unknown { const manifest = record(pkg); for (const field of [ @@ -37,6 +81,7 @@ export function lockedVitestVersion( directory: string, manager: PackageManager, spec: string, + readLockfile: LockfileReader, ): string | undefined { const range = spec.replace(/^npm:vitest@/, ''); const validRange = normalizeRange(range); @@ -52,18 +97,18 @@ export function lockedVitestVersion( : undefined; } const relative = path.relative(root, directory).replaceAll('\\', '/'); - const read = (name: string) => - fs.readFileSync(path.join(root, name), 'utf8').replace(/^\uFEFF/, ''); try { if (manager === PackageManager.npm) { - const lock = JSON.parse( - read( + const lock = readLockfile( + path.join( + root, fs.existsSync(path.join(root, 'npm-shrinkwrap.json')) ? 'npm-shrinkwrap.json' : 'package-lock.json', ), + (source) => record(JSON.parse(source)), ); - const packages = record(lock.packages); + const packages = record(lock?.packages); if (!packages) { // npm v1 locks have no importer specifiers to prove this direct edge. return undefined; @@ -87,14 +132,13 @@ export function lockedVitestVersion( } } if (manager === PackageManager.yarn) { - const text = read('yarn.lock'); - if (/^__metadata:/m.test(text)) { - const document = parseDocument(text); - if (document.errors.length) { - return undefined; - } + const lock = readLockfile(path.join(root, 'yarn.lock'), parseYarnLockfile); + if (!lock) { + return undefined; + } + if (lock.modern) { const descriptors = new Set([`vitest@${spec}`, `vitest@npm:${range}`]); - const entries = Object.entries(record(document.toJS()) ?? {}).filter(([keys]) => + const entries = Object.entries(lock.entries).filter(([keys]) => keys.split(/,\s+/).some((key) => descriptors.has(key)), ); if (entries.length !== 1) { @@ -105,18 +149,9 @@ export function lockedVitestVersion( ? valid(entry.resolution.slice('vitest@npm:'.length)) : undefined; } - // Classic's quoted descriptor lists are YAML flow scalars, but its entry - // bodies are not YAML. Read only the exact descriptor and version field. const matches: string[] = []; - for (const block of text.split(/\r?\n(?=[^\s#])/)) { - const [header] = block.split(/\r?\n/); - if (!header.endsWith(':')) { - continue; - } - const document = parseDocument(`[${header.slice(0, -1)}]`); - const keys: unknown = document.errors.length ? undefined : document.toJS(); - if (Array.isArray(keys) && keys.includes(`vitest@${spec}`)) { - const version = /^ version "([^"]+)"\r?$/m.exec(block)?.[1]; + for (const { keys, version } of lock.entries) { + if (keys.includes(`vitest@${spec}`)) { if (!version) { return undefined; } @@ -126,9 +161,12 @@ export function lockedVitestVersion( return matches.length === 1 ? valid(matches[0]) : undefined; } if (manager === PackageManager.bun) { - const errors: ParseError[] = []; - const lock = record(parseJsonc(read('bun.lock'), errors, { allowTrailingComma: true })); - if (errors.length || declaredVitest(record(lock?.workspaces)?.[relative]) !== spec) { + const lock = readLockfile(path.join(root, 'bun.lock'), (source) => { + const errors: ParseError[] = []; + const parsed = record(parseJsonc(source, errors, { allowTrailingComma: true })); + return errors.length ? undefined : parsed; + }); + if (declaredVitest(record(lock?.workspaces)?.[relative]) !== spec) { return undefined; } const packages = record(lock?.packages);