From e1eb7cf8a7e3a68bff50cff3681575e096857dbb Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 15 Jan 2026 12:00:47 -0500 Subject: [PATCH] refactor(@angular/cli): optimize package manager version detection This commit introduces a "hot cache" mechanism for the package manager version. When the package manager factory determines the version (e.g., to distinguish between Yarn and Yarn Classic), it now passes that discovered version to the `PackageManager` instance. The `PackageManager` class has been updated to: 1. Accept an optional `version` in its constructor options. 2. Cache the version internally after the first fetch. 3. Use the cached/injected version in `getVersion()` to avoid redundant shell commands. This improves startup performance by eliminating unnecessary child process spawning when the version is already known. Additionally, an assertion has been added to ensure that the version command for `yarn` and `yarn-classic` remains consistent, as the detection logic relies on this. --- .../cli/src/package-managers/factory.ts | 84 +++++++++++-------- .../src/package-managers/package-manager.ts | 16 +++- .../package-managers/package-manager_spec.ts | 54 ++++++++++++ 3 files changed, 118 insertions(+), 36 deletions(-) create mode 100644 packages/angular/cli/src/package-managers/package-manager_spec.ts diff --git a/packages/angular/cli/src/package-managers/factory.ts b/packages/angular/cli/src/package-managers/factory.ts index 1cd3d2462edc..790a48140285 100644 --- a/packages/angular/cli/src/package-managers/factory.ts +++ b/packages/angular/cli/src/package-managers/factory.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ +import assert from 'node:assert/strict'; import { major } from 'semver'; import { discover } from './discovery'; import { Host, NodeJS_HOST } from './host'; @@ -19,26 +20,27 @@ import { PackageManagerName, SUPPORTED_PACKAGE_MANAGERS } from './package-manage const DEFAULT_PACKAGE_MANAGER: PackageManagerName = 'npm'; /** - * Gets the version of yarn installed on the system. + * Gets the version of the package manager. * @param host A `Host` instance for running commands. * @param cwd The absolute path to the working directory. + * @param name The name of the package manager. * @param logger An optional logger instance. - * @returns A promise that resolves to the yarn version string, or null if yarn is not installed. + * @returns A promise that resolves to the version string. */ -async function getYarnVersion(host: Host, cwd: string, logger?: Logger): Promise { - logger?.debug(`Getting yarn version...`); - - try { - const { stdout } = await host.runCommand('yarn', ['--version'], { cwd }); - const version = stdout.trim(); - logger?.debug(`Yarn version is '${version}'.`); +async function getPackageManagerVersion( + host: Host, + cwd: string, + name: PackageManagerName, + logger?: Logger, +): Promise { + const descriptor = SUPPORTED_PACKAGE_MANAGERS[name]; + logger?.debug(`Getting ${name} version...`); - return version; - } catch (e) { - logger?.debug('Failed to get yarn version.'); + const { stdout } = await host.runCommand(descriptor.binary, descriptor.versionCommand, { cwd }); + const version = stdout.trim(); + logger?.debug(`${name} version is '${version}'.`); - return null; - } + return version; } /** @@ -60,7 +62,11 @@ async function determinePackageManager( configured?: PackageManagerName, logger?: Logger, dryRun?: boolean, -): Promise<{ name: PackageManagerName; source: 'configured' | 'discovered' | 'default' }> { +): Promise<{ + name: PackageManagerName; + source: 'configured' | 'discovered' | 'default'; + version?: string; +}> { let name: PackageManagerName; let source: 'configured' | 'discovered' | 'default'; @@ -83,17 +89,28 @@ async function determinePackageManager( } } + let version: string | undefined; if (name === 'yarn' && !dryRun) { - const version = await getYarnVersion(host, cwd, logger); - if (version && major(version) < 2) { - name = 'yarn-classic'; - logger?.debug(`Detected yarn classic. Using 'yarn-classic'.`); + assert.deepStrictEqual( + SUPPORTED_PACKAGE_MANAGERS.yarn.versionCommand, + SUPPORTED_PACKAGE_MANAGERS['yarn-classic'].versionCommand, + 'Yarn and Yarn Classic version commands must match for detection logic to be valid.', + ); + + try { + version = await getPackageManagerVersion(host, cwd, name, logger); + if (version && major(version) < 2) { + name = 'yarn-classic'; + logger?.debug(`Detected yarn classic. Using 'yarn-classic'.`); + } + } catch { + logger?.debug('Failed to get yarn version.'); } } else if (name === 'yarn') { logger?.debug('Skipping yarn version check due to dry run. Assuming modern yarn.'); } - return { name, source }; + return { name, source, version }; } /** @@ -115,29 +132,19 @@ export async function createPackageManager(options: { const { cwd, configuredPackageManager, logger, dryRun, tempDirectory } = options; const host = NodeJS_HOST; - const { name, source } = await determinePackageManager( - host, - cwd, - configuredPackageManager, - logger, - dryRun, - ); + const result = await determinePackageManager(host, cwd, configuredPackageManager, logger, dryRun); + const { name, source } = result; + let { version } = result; const descriptor = SUPPORTED_PACKAGE_MANAGERS[name]; if (!descriptor) { throw new Error(`Unsupported package manager: "${name}"`); } - const packageManager = new PackageManager(host, cwd, descriptor, { - dryRun, - logger, - tempDirectory, - }); - // Do not verify if the package manager is installed during a dry run. - if (!dryRun) { + if (!dryRun && !version) { try { - await packageManager.getVersion(); + version = await getPackageManagerVersion(host, cwd, name, logger); } catch { if (source === 'default') { throw new Error( @@ -153,6 +160,13 @@ export async function createPackageManager(options: { } } + const packageManager = new PackageManager(host, cwd, descriptor, { + dryRun, + logger, + tempDirectory, + version, + }); + logger?.debug(`Successfully created PackageManager for '${name}'.`); return packageManager; diff --git a/packages/angular/cli/src/package-managers/package-manager.ts b/packages/angular/cli/src/package-managers/package-manager.ts index 1faedc5b155e..45c9639d954b 100644 --- a/packages/angular/cli/src/package-managers/package-manager.ts +++ b/packages/angular/cli/src/package-managers/package-manager.ts @@ -65,6 +65,13 @@ export interface PackageManagerOptions { * If not specified, the system's temporary directory will be used. */ tempDirectory?: string; + + /** + * The version of the package manager. + * If provided, the `getVersion` method will return this version + * instead of running the version command. + */ + version?: string; } /** @@ -79,6 +86,7 @@ export class PackageManager { readonly #manifestCache = new Map(); readonly #metadataCache = new Map(); #dependencyCache: Map | null = null; + #version: string | undefined; /** * Creates a new `PackageManager` instance. @@ -96,6 +104,7 @@ export class PackageManager { if (this.options.dryRun && !this.options.logger) { throw new Error('A logger must be provided when dryRun is enabled.'); } + this.#version = options.version; } /** @@ -334,9 +343,14 @@ export class PackageManager { * @returns A promise that resolves to the trimmed version string. */ async getVersion(): Promise { + if (this.#version) { + return this.#version; + } + const { stdout } = await this.#run(this.descriptor.versionCommand); + this.#version = stdout.trim(); - return stdout.trim(); + return this.#version; } /** diff --git a/packages/angular/cli/src/package-managers/package-manager_spec.ts b/packages/angular/cli/src/package-managers/package-manager_spec.ts new file mode 100644 index 000000000000..2482349b323d --- /dev/null +++ b/packages/angular/cli/src/package-managers/package-manager_spec.ts @@ -0,0 +1,54 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { Host } from './host'; +import { PackageManager } from './package-manager'; +import { SUPPORTED_PACKAGE_MANAGERS } from './package-manager-descriptor'; +import { MockHost } from './testing/mock-host'; + +describe('PackageManager', () => { + let host: Host; + let runCommandSpy: jasmine.Spy; + const descriptor = SUPPORTED_PACKAGE_MANAGERS['npm']; + + beforeEach(() => { + host = new MockHost(); + runCommandSpy = spyOn(host, 'runCommand').and.resolveTo({ stdout: '1.2.3', stderr: '' }); + host.runCommand = runCommandSpy; + }); + + describe('getVersion', () => { + it('should fetch the version from the package manager if not cached', async () => { + const pm = new PackageManager(host, '/tmp', descriptor); + const version = await pm.getVersion(); + + expect(version).toBe('1.2.3'); + expect(runCommandSpy).toHaveBeenCalledWith( + descriptor.binary, + descriptor.versionCommand, + jasmine.objectContaining({ cwd: '/tmp' }), + ); + }); + + it('should cache the version after the first fetch', async () => { + const pm = new PackageManager(host, '/tmp', descriptor); + await pm.getVersion(); + await pm.getVersion(); + + expect(runCommandSpy).toHaveBeenCalledTimes(1); + }); + + it('should use the version provided in the constructor', async () => { + const pm = new PackageManager(host, '/tmp', descriptor, { version: '4.5.6' }); + const version = await pm.getVersion(); + + expect(version).toBe('4.5.6'); + expect(runCommandSpy).not.toHaveBeenCalled(); + }); + }); +});