Skip to content

Commit 284c47d

Browse files
committed
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.
1 parent d87f243 commit 284c47d

File tree

3 files changed

+118
-36
lines changed

3 files changed

+118
-36
lines changed

packages/angular/cli/src/package-managers/factory.ts

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import assert from 'node:assert/strict';
910
import { major } from 'semver';
1011
import { discover } from './discovery';
1112
import { Host, NodeJS_HOST } from './host';
@@ -19,26 +20,27 @@ import { PackageManagerName, SUPPORTED_PACKAGE_MANAGERS } from './package-manage
1920
const DEFAULT_PACKAGE_MANAGER: PackageManagerName = 'npm';
2021

2122
/**
22-
* Gets the version of yarn installed on the system.
23+
* Gets the version of the package manager.
2324
* @param host A `Host` instance for running commands.
2425
* @param cwd The absolute path to the working directory.
26+
* @param name The name of the package manager.
2527
* @param logger An optional logger instance.
26-
* @returns A promise that resolves to the yarn version string, or null if yarn is not installed.
28+
* @returns A promise that resolves to the version string.
2729
*/
28-
async function getYarnVersion(host: Host, cwd: string, logger?: Logger): Promise<string | null> {
29-
logger?.debug(`Getting yarn version...`);
30-
31-
try {
32-
const { stdout } = await host.runCommand('yarn', ['--version'], { cwd });
33-
const version = stdout.trim();
34-
logger?.debug(`Yarn version is '${version}'.`);
30+
async function getPackageManagerVersion(
31+
host: Host,
32+
cwd: string,
33+
name: PackageManagerName,
34+
logger?: Logger,
35+
): Promise<string> {
36+
const descriptor = SUPPORTED_PACKAGE_MANAGERS[name];
37+
logger?.debug(`Getting ${name} version...`);
3538

36-
return version;
37-
} catch (e) {
38-
logger?.debug('Failed to get yarn version.');
39+
const { stdout } = await host.runCommand(descriptor.binary, descriptor.versionCommand, { cwd });
40+
const version = stdout.trim();
41+
logger?.debug(`${name} version is '${version}'.`);
3942

40-
return null;
41-
}
43+
return version;
4244
}
4345

4446
/**
@@ -60,7 +62,11 @@ async function determinePackageManager(
6062
configured?: PackageManagerName,
6163
logger?: Logger,
6264
dryRun?: boolean,
63-
): Promise<{ name: PackageManagerName; source: 'configured' | 'discovered' | 'default' }> {
65+
): Promise<{
66+
name: PackageManagerName;
67+
source: 'configured' | 'discovered' | 'default';
68+
version?: string;
69+
}> {
6470
let name: PackageManagerName;
6571
let source: 'configured' | 'discovered' | 'default';
6672

@@ -83,17 +89,28 @@ async function determinePackageManager(
8389
}
8490
}
8591

92+
let version: string | undefined;
8693
if (name === 'yarn' && !dryRun) {
87-
const version = await getYarnVersion(host, cwd, logger);
88-
if (version && major(version) < 2) {
89-
name = 'yarn-classic';
90-
logger?.debug(`Detected yarn classic. Using 'yarn-classic'.`);
94+
assert.deepStrictEqual(
95+
SUPPORTED_PACKAGE_MANAGERS.yarn.versionCommand,
96+
SUPPORTED_PACKAGE_MANAGERS['yarn-classic'].versionCommand,
97+
'Yarn and Yarn Classic version commands must match for detection logic to be valid.',
98+
);
99+
100+
try {
101+
version = await getPackageManagerVersion(host, cwd, name, logger);
102+
if (version && major(version) < 2) {
103+
name = 'yarn-classic';
104+
logger?.debug(`Detected yarn classic. Using 'yarn-classic'.`);
105+
}
106+
} catch {
107+
logger?.debug('Failed to get yarn version.');
91108
}
92109
} else if (name === 'yarn') {
93110
logger?.debug('Skipping yarn version check due to dry run. Assuming modern yarn.');
94111
}
95112

96-
return { name, source };
113+
return { name, source, version };
97114
}
98115

99116
/**
@@ -115,29 +132,19 @@ export async function createPackageManager(options: {
115132
const { cwd, configuredPackageManager, logger, dryRun, tempDirectory } = options;
116133
const host = NodeJS_HOST;
117134

118-
const { name, source } = await determinePackageManager(
119-
host,
120-
cwd,
121-
configuredPackageManager,
122-
logger,
123-
dryRun,
124-
);
135+
const result = await determinePackageManager(host, cwd, configuredPackageManager, logger, dryRun);
136+
const { name, source } = result;
137+
let { version } = result;
125138

126139
const descriptor = SUPPORTED_PACKAGE_MANAGERS[name];
127140
if (!descriptor) {
128141
throw new Error(`Unsupported package manager: "${name}"`);
129142
}
130143

131-
const packageManager = new PackageManager(host, cwd, descriptor, {
132-
dryRun,
133-
logger,
134-
tempDirectory,
135-
});
136-
137144
// Do not verify if the package manager is installed during a dry run.
138-
if (!dryRun) {
145+
if (!dryRun && !version) {
139146
try {
140-
await packageManager.getVersion();
147+
version = await getPackageManagerVersion(host, cwd, name, logger);
141148
} catch {
142149
if (source === 'default') {
143150
throw new Error(
@@ -153,6 +160,13 @@ export async function createPackageManager(options: {
153160
}
154161
}
155162

163+
const packageManager = new PackageManager(host, cwd, descriptor, {
164+
dryRun,
165+
logger,
166+
tempDirectory,
167+
version,
168+
});
169+
156170
logger?.debug(`Successfully created PackageManager for '${name}'.`);
157171

158172
return packageManager;

packages/angular/cli/src/package-managers/package-manager.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ export interface PackageManagerOptions {
6565
* If not specified, the system's temporary directory will be used.
6666
*/
6767
tempDirectory?: string;
68+
69+
/**
70+
* The version of the package manager.
71+
* If provided, the `getVersion` method will return this version
72+
* instead of running the version command.
73+
*/
74+
version?: string;
6875
}
6976

7077
/**
@@ -79,6 +86,7 @@ export class PackageManager {
7986
readonly #manifestCache = new Map<string, PackageManifest | null>();
8087
readonly #metadataCache = new Map<string, PackageMetadata | null>();
8188
#dependencyCache: Map<string, InstalledPackage> | null = null;
89+
#version: string | undefined;
8290

8391
/**
8492
* Creates a new `PackageManager` instance.
@@ -96,6 +104,7 @@ export class PackageManager {
96104
if (this.options.dryRun && !this.options.logger) {
97105
throw new Error('A logger must be provided when dryRun is enabled.');
98106
}
107+
this.#version = options.version;
99108
}
100109

101110
/**
@@ -334,9 +343,14 @@ export class PackageManager {
334343
* @returns A promise that resolves to the trimmed version string.
335344
*/
336345
async getVersion(): Promise<string> {
346+
if (this.#version) {
347+
return this.#version;
348+
}
349+
337350
const { stdout } = await this.#run(this.descriptor.versionCommand);
351+
this.#version = stdout.trim();
338352

339-
return stdout.trim();
353+
return this.#version;
340354
}
341355

342356
/**
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { Host } from './host';
10+
import { PackageManager } from './package-manager';
11+
import { SUPPORTED_PACKAGE_MANAGERS } from './package-manager-descriptor';
12+
import { MockHost } from './testing/mock-host';
13+
14+
describe('PackageManager', () => {
15+
let host: Host;
16+
let runCommandSpy: jasmine.Spy;
17+
const descriptor = SUPPORTED_PACKAGE_MANAGERS['npm'];
18+
19+
beforeEach(() => {
20+
host = new MockHost();
21+
runCommandSpy = spyOn(host, 'runCommand').and.resolveTo({ stdout: '1.2.3', stderr: '' });
22+
host.runCommand = runCommandSpy;
23+
});
24+
25+
describe('getVersion', () => {
26+
it('should fetch the version from the package manager if not cached', async () => {
27+
const pm = new PackageManager(host, '/tmp', descriptor);
28+
const version = await pm.getVersion();
29+
30+
expect(version).toBe('1.2.3');
31+
expect(runCommandSpy).toHaveBeenCalledWith(
32+
descriptor.binary,
33+
descriptor.versionCommand,
34+
jasmine.objectContaining({ cwd: '/tmp' }),
35+
);
36+
});
37+
38+
it('should cache the version after the first fetch', async () => {
39+
const pm = new PackageManager(host, '/tmp', descriptor);
40+
await pm.getVersion();
41+
await pm.getVersion();
42+
43+
expect(runCommandSpy).toHaveBeenCalledTimes(1);
44+
});
45+
46+
it('should use the version provided in the constructor', async () => {
47+
const pm = new PackageManager(host, '/tmp', descriptor, { version: '4.5.6' });
48+
const version = await pm.getVersion();
49+
50+
expect(version).toBe('4.5.6');
51+
expect(runCommandSpy).not.toHaveBeenCalled();
52+
});
53+
});
54+
});

0 commit comments

Comments
 (0)