Skip to content

Commit 42629c0

Browse files
committed
refactor(@angular/cli): migrate ng add to new package manager abstraction
This commit fully refactors the `ng add` command to use the new, centralized package manager abstraction. All package installation and metadata fetching logic now goes through the new API. This includes: - Using `createPackageManager` to detect the project's package manager. - Replacing `fetchPackageMetadata` with `packageManager.getRegistryMetadata`. - Replacing `fetchPackageManifest` with `packageManager.getManifest`. - Replacing the `install` and `installTemp` methods with the `packageManager.add` and `packageManager.acquireTempPackage` methods. This change improves security by eliminating `shell: true` execution as well as enhances reliability with better error handling and caching.
1 parent 69b2be8 commit 42629c0

File tree

1 file changed

+117
-110
lines changed
  • packages/angular/cli/src/commands/add

1 file changed

+117
-110
lines changed

packages/angular/cli/src/commands/add/cli.ts

Lines changed: 117 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88

99
import { Listr, ListrRenderer, ListrTaskWrapper, color, figures } from 'listr2';
1010
import assert from 'node:assert';
11+
import { promises as fs } from 'node:fs';
1112
import { createRequire } from 'node:module';
1213
import { dirname, join } from 'node:path';
1314
import npa from 'npm-package-arg';
1415
import { Range, compare, intersects, prerelease, satisfies, valid } from 'semver';
1516
import { Argv } from 'yargs';
16-
import { PackageManager } from '../../../lib/config/workspace-schema';
1717
import {
1818
CommandModuleImplementation,
1919
Options,
@@ -23,14 +23,14 @@ import {
2323
SchematicsCommandArgs,
2424
SchematicsCommandModule,
2525
} from '../../command-builder/schematics-command-module';
26-
import { assertIsError } from '../../utilities/error';
2726
import {
2827
NgAddSaveDependency,
28+
PackageManager,
2929
PackageManifest,
3030
PackageMetadata,
31-
fetchPackageManifest,
32-
fetchPackageMetadata,
33-
} from '../../utilities/package-metadata';
31+
createPackageManager,
32+
} from '../../package-managers';
33+
import { assertIsError } from '../../utilities/error';
3434
import { isTTY } from '../../utilities/tty';
3535
import { VERSION } from '../../utilities/version';
3636

@@ -44,8 +44,8 @@ interface AddCommandArgs extends SchematicsCommandArgs {
4444
}
4545

4646
interface AddCommandTaskContext {
47+
packageManager: PackageManager;
4748
packageIdentifier: npa.Result;
48-
usingYarn?: boolean;
4949
savePackage?: NgAddSaveDependency;
5050
collectionName?: string;
5151
executeSchematic: AddCommandModule['executeSchematic'];
@@ -173,12 +173,12 @@ export default class AddCommandModule
173173
}
174174
}
175175

176-
const taskContext: AddCommandTaskContext = {
176+
const taskContext = {
177177
packageIdentifier,
178178
executeSchematic: this.executeSchematic.bind(this),
179179
getPeerDependencyConflicts: this.getPeerDependencyConflicts.bind(this),
180180
dryRun: options.dryRun,
181-
};
181+
} as AddCommandTaskContext;
182182

183183
const tasks = new Listr<AddCommandTaskContext>(
184184
[
@@ -194,7 +194,7 @@ export default class AddCommandModule
194194
rendererOptions: { persistentOutput: true },
195195
},
196196
{
197-
title: 'Loading package information from registry',
197+
title: 'Loading package information',
198198
task: (context, task) => this.loadPackageInfoTask(context, task, options),
199199
rendererOptions: { persistentOutput: true },
200200
},
@@ -294,91 +294,104 @@ export default class AddCommandModule
294294
}
295295
}
296296

297-
private determinePackageManagerTask(
297+
private async determinePackageManagerTask(
298298
context: AddCommandTaskContext,
299299
task: AddCommandTaskWrapper,
300-
): void {
301-
const { packageManager } = this.context;
302-
context.usingYarn = packageManager.name === PackageManager.Yarn;
303-
task.output = `Using package manager: ${color.dim(packageManager.name)}`;
300+
): Promise<void> {
301+
context.packageManager = await createPackageManager({
302+
cwd: this.context.root,
303+
logger: this.context.logger,
304+
dryRun: context.dryRun,
305+
});
306+
task.output = `Using package manager: ${color.dim(context.packageManager.name)}`;
304307
}
305308

306309
private async findCompatiblePackageVersionTask(
307310
context: AddCommandTaskContext,
308311
task: AddCommandTaskWrapper,
309312
options: Options<AddCommandArgs>,
310313
): Promise<void> {
311-
const { logger } = this.context;
312-
const { verbose, registry } = options;
314+
const { registry, verbose } = options;
315+
const { packageManager, packageIdentifier } = context;
316+
const packageName = packageIdentifier.name;
313317

314-
assert(
315-
context.packageIdentifier.name,
316-
'Registry package identifiers should always have a name.',
317-
);
318+
assert(packageName, 'Registry package identifiers should always have a name.');
318319

319-
// only package name provided; search for viable version
320-
// plus special cases for packages that did not have peer deps setup
321-
let packageMetadata;
320+
const rejectionReasons: string[] = [];
321+
322+
// Attempt to use the 'latest' tag from the registry.
322323
try {
323-
packageMetadata = await fetchPackageMetadata(context.packageIdentifier.name, logger, {
324+
const latestManifest = await packageManager.getManifest(`${packageName}@latest`, {
324325
registry,
325-
usingYarn: context.usingYarn,
326-
verbose,
327326
});
327+
328+
if (latestManifest) {
329+
const conflicts = await this.getPeerDependencyConflicts(latestManifest);
330+
if (!conflicts) {
331+
context.packageIdentifier = npa.resolve(latestManifest.name, latestManifest.version);
332+
task.output = `Found compatible package version: ${color.blue(latestManifest.version)}.`;
333+
334+
return;
335+
}
336+
rejectionReasons.push(...conflicts);
337+
}
328338
} catch (e) {
329339
assertIsError(e);
330340
throw new CommandError(`Unable to load package information from registry: ${e.message}`);
331341
}
332342

333-
const rejectionReasons: string[] = [];
343+
// 'latest' is invalid or not found, search for most recent matching package.
344+
task.output =
345+
'Could not find a compatible version with `latest`. Searching for a compatible version.';
334346

335-
// Start with the version tagged as `latest` if it exists
336-
const latestManifest = packageMetadata.tags['latest'];
337-
if (latestManifest) {
338-
const latestConflicts = await this.getPeerDependencyConflicts(latestManifest);
339-
if (latestConflicts) {
340-
// 'latest' is invalid so search for most recent matching package
341-
rejectionReasons.push(...latestConflicts);
342-
} else {
343-
context.packageIdentifier = npa.resolve(latestManifest.name, latestManifest.version);
344-
task.output = `Found compatible package version: ${color.blue(latestManifest.version)}.`;
347+
let packageMetadata;
348+
try {
349+
packageMetadata = await packageManager.getRegistryMetadata(packageName, {
350+
registry,
351+
});
352+
} catch (e) {
353+
assertIsError(e);
354+
throw new CommandError(`Unable to load package information from registry: ${e.message}`);
355+
}
345356

346-
return;
347-
}
357+
if (!packageMetadata) {
358+
throw new CommandError('Unable to load package information from registry.');
348359
}
349360

350-
// Allow prelease versions if the CLI itself is a prerelease
361+
// Allow prelease versions if the CLI itself is a prerelease.
351362
const allowPrereleases = !!prerelease(VERSION.full);
352-
const versionManifests = this.#getPotentialVersionManifests(packageMetadata, allowPrereleases);
363+
const potentialVersions = this.#getPotentialVersions(packageMetadata, allowPrereleases);
353364

354-
let found = false;
355-
for (const versionManifest of versionManifests) {
356-
// Already checked the 'latest' version
357-
if (latestManifest?.version === versionManifest.version) {
365+
let found;
366+
for (const version of potentialVersions) {
367+
const manifest = await packageManager.getManifest(`${packageName}@${version}`, {
368+
registry,
369+
});
370+
if (!manifest) {
358371
continue;
359372
}
360373

361-
const conflicts = await this.getPeerDependencyConflicts(versionManifest);
374+
const conflicts = await this.getPeerDependencyConflicts(manifest);
362375
if (conflicts) {
363-
if (options.verbose || rejectionReasons.length < DEFAULT_CONFLICT_DISPLAY_LIMIT) {
376+
if (verbose || rejectionReasons.length < DEFAULT_CONFLICT_DISPLAY_LIMIT) {
364377
rejectionReasons.push(...conflicts);
365378
}
366379
continue;
367380
}
368381

369-
context.packageIdentifier = npa.resolve(versionManifest.name, versionManifest.version);
370-
found = true;
382+
context.packageIdentifier = npa.resolve(manifest.name, manifest.version);
383+
found = manifest;
371384
break;
372385
}
373386

374387
if (!found) {
375-
let message = `Unable to find compatible package. Using 'latest' tag.`;
388+
let message = `Unable to find compatible package.`;
376389
if (rejectionReasons.length > 0) {
377390
message +=
378391
'\nThis is often because of incompatible peer dependencies.\n' +
379392
'These versions were rejected due to the following conflicts:\n' +
380393
rejectionReasons
381-
.slice(0, options.verbose ? undefined : DEFAULT_CONFLICT_DISPLAY_LIMIT)
394+
.slice(0, verbose ? undefined : DEFAULT_CONFLICT_DISPLAY_LIMIT)
382395
.map((r) => ` - ${r}`)
383396
.join('\n');
384397
}
@@ -390,51 +403,38 @@ export default class AddCommandModule
390403
}
391404
}
392405

393-
#getPotentialVersionManifests(
394-
packageMetadata: PackageMetadata,
395-
allowPrereleases: boolean,
396-
): PackageManifest[] {
406+
#getPotentialVersions(packageMetadata: PackageMetadata, allowPrereleases: boolean): string[] {
397407
const versionExclusions = packageVersionExclusions[packageMetadata.name];
398-
const versionManifests = Object.values(packageMetadata.versions).filter(
399-
(value: PackageManifest) => {
400-
// Prerelease versions are not stable and should not be considered by default
401-
if (!allowPrereleases && prerelease(value.version)) {
402-
return false;
403-
}
404-
// Deprecated versions should not be used or considered
405-
if (value.deprecated) {
406-
return false;
407-
}
408-
// Excluded package versions should not be considered
409-
if (
410-
versionExclusions &&
411-
satisfies(value.version, versionExclusions, { includePrerelease: true })
412-
) {
413-
return false;
414-
}
415408

416-
return true;
417-
},
418-
);
409+
const versions = Object.values(packageMetadata.versions).filter((version) => {
410+
// Prerelease versions are not stable and should not be considered by default
411+
if (!allowPrereleases && prerelease(version)) {
412+
return false;
413+
}
414+
415+
// Excluded package versions should not be considered
416+
if (versionExclusions && satisfies(version, versionExclusions, { includePrerelease: true })) {
417+
return false;
418+
}
419+
420+
return true;
421+
});
419422

420423
// Sort in reverse SemVer order so that the newest compatible version is chosen
421-
return versionManifests.sort((a, b) => compare(b.version, a.version, true));
424+
return versions.sort((a, b) => compare(b, a, true));
422425
}
423426

424427
private async loadPackageInfoTask(
425428
context: AddCommandTaskContext,
426429
task: AddCommandTaskWrapper,
427430
options: Options<AddCommandArgs>,
428431
): Promise<void> {
429-
const { logger } = this.context;
430-
const { verbose, registry } = options;
432+
const { registry } = options;
431433

432434
let manifest;
433435
try {
434-
manifest = await fetchPackageManifest(context.packageIdentifier.toString(), logger, {
436+
manifest = await context.packageManager.getManifest(context.packageIdentifier.toString(), {
435437
registry,
436-
verbose,
437-
usingYarn: context.usingYarn,
438438
});
439439
} catch (e) {
440440
assertIsError(e);
@@ -443,6 +443,12 @@ export default class AddCommandModule
443443
);
444444
}
445445

446+
if (!manifest) {
447+
throw new CommandError(
448+
`Unable to fetch package information for '${context.packageIdentifier}'.`,
449+
);
450+
}
451+
446452
context.hasSchematics = !!manifest.schematics;
447453
context.savePackage = manifest['ng-add']?.save;
448454
context.collectionName = manifest.name;
@@ -487,45 +493,42 @@ export default class AddCommandModule
487493
task: AddCommandTaskWrapper,
488494
options: Options<AddCommandArgs>,
489495
): Promise<void> {
490-
const { packageManager } = this.context;
491496
const { registry } = options;
497+
const { packageManager, packageIdentifier, savePackage } = context;
492498

493499
// Only show if installation will actually occur
494500
task.title = 'Installing package';
495501

496-
if (context.savePackage === false && packageManager.name !== PackageManager.Bun) {
497-
// Bun has a `--no-save` option which we are using to
498-
// install the package and not update the package.json and the lock file.
502+
if (context.savePackage === false) {
499503
task.title += ' in temporary location';
500504

501505
// Temporary packages are located in a different directory
502506
// Hence we need to resolve them using the temp path
503-
const { success, tempNodeModules } = await packageManager.installTemp(
504-
context.packageIdentifier.toString(),
505-
registry ? [`--registry="${registry}"`] : undefined,
507+
const { workingDirectory } = await packageManager.acquireTempPackage(
508+
packageIdentifier.toString(),
509+
{
510+
registry,
511+
},
506512
);
507-
const tempRequire = createRequire(tempNodeModules + '/');
513+
514+
const tempRequire = createRequire(workingDirectory + '/');
508515
assert(context.collectionName, 'Collection name should always be available');
509516
const resolvedCollectionPath = tempRequire.resolve(
510517
join(context.collectionName, 'package.json'),
511518
);
512519

513-
if (!success) {
514-
throw new CommandError('Unable to install package');
515-
}
516-
517520
context.collectionName = dirname(resolvedCollectionPath);
518521
} else {
519-
const success = await packageManager.install(
520-
context.packageIdentifier.toString(),
521-
context.savePackage,
522-
registry ? [`--registry="${registry}"`] : undefined,
523-
undefined,
522+
await packageManager.add(
523+
packageIdentifier.toString(),
524+
'none',
525+
savePackage !== 'dependencies',
526+
false,
527+
true,
528+
{
529+
registry,
530+
},
524531
);
525-
526-
if (!success) {
527-
throw new CommandError('Unable to install package');
528-
}
529532
}
530533
}
531534

@@ -633,24 +636,28 @@ export default class AddCommandModule
633636
return cachedVersion;
634637
}
635638

636-
const { logger, root } = this.context;
637-
let installedPackage;
639+
const { root } = this.context;
640+
let installedPackagePath;
638641
try {
639-
installedPackage = this.rootRequire.resolve(join(name, 'package.json'));
642+
installedPackagePath = this.rootRequire.resolve(join(name, 'package.json'));
640643
} catch {}
641644

642-
if (installedPackage) {
645+
if (installedPackagePath) {
643646
try {
644-
const installed = await fetchPackageManifest(dirname(installedPackage), logger);
645-
this.#projectVersionCache.set(name, installed.version);
647+
const installedPackage = JSON.parse(
648+
await fs.readFile(installedPackagePath, 'utf-8'),
649+
) as PackageManifest;
650+
this.#projectVersionCache.set(name, installedPackage.version);
646651

647-
return installed.version;
652+
return installedPackage.version;
648653
} catch {}
649654
}
650655

651656
let projectManifest;
652657
try {
653-
projectManifest = await fetchPackageManifest(root, logger);
658+
projectManifest = JSON.parse(
659+
await fs.readFile(join(root, 'package.json'), 'utf-8'),
660+
) as PackageManifest;
654661
} catch {}
655662

656663
if (projectManifest) {

0 commit comments

Comments
 (0)