Skip to content

Commit 4f00055

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.getPackageManifest`. - 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 fceb8a3 commit 4f00055

File tree

1 file changed

+121
-109
lines changed
  • packages/angular/cli/src/commands/add

1 file changed

+121
-109
lines changed

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

Lines changed: 121 additions & 109 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
[
@@ -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.getPackageManifest(`${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.getPackageManifest(`${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,59 +403,55 @@ 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, {
435-
registry,
436-
verbose,
437-
usingYarn: context.usingYarn,
438-
});
436+
manifest = await context.packageManager.getPackageManifest(
437+
context.packageIdentifier.toString(),
438+
{
439+
registry,
440+
},
441+
);
439442
} catch (e) {
440443
assertIsError(e);
441444
throw new CommandError(
442445
`Unable to fetch package information for '${context.packageIdentifier}': ${e.message}`,
443446
);
444447
}
445448

449+
if (!manifest) {
450+
throw new CommandError(
451+
`Unable to fetch package information for '${context.packageIdentifier}'.`,
452+
);
453+
}
454+
446455
context.hasSchematics = !!manifest.schematics;
447456
context.savePackage = manifest['ng-add']?.save;
448457
context.collectionName = manifest.name;
@@ -487,43 +496,42 @@ export default class AddCommandModule
487496
task: AddCommandTaskWrapper,
488497
options: Options<AddCommandArgs>,
489498
): Promise<void> {
490-
const { packageManager } = this.context;
491499
const { registry } = options;
500+
const { packageManager, packageIdentifier, savePackage } = context;
492501

493502
// Only show if installation will actually occur
494503
task.title = 'Installing package';
495504

496-
if (context.savePackage === false) {
505+
if (savePackage === false) {
497506
task.title += ' in temporary location';
498507

499508
// Temporary packages are located in a different directory
500509
// Hence we need to resolve them using the temp path
501-
const { success, tempNodeModules } = await packageManager.installTemp(
502-
context.packageIdentifier.toString(),
503-
registry ? [`--registry="${registry}"`] : undefined,
510+
const { workingDirectory } = await packageManager.acquireTempPackage(
511+
packageIdentifier.toString(),
512+
{
513+
registry,
514+
},
504515
);
505-
const tempRequire = createRequire(tempNodeModules + '/');
516+
517+
const tempRequire = createRequire(workingDirectory + '/');
506518
assert(context.collectionName, 'Collection name should always be available');
507519
const resolvedCollectionPath = tempRequire.resolve(
508520
join(context.collectionName, 'package.json'),
509521
);
510522

511-
if (!success) {
512-
throw new CommandError('Unable to install package');
513-
}
514-
515523
context.collectionName = dirname(resolvedCollectionPath);
516524
} else {
517-
const success = await packageManager.install(
518-
context.packageIdentifier.toString(),
519-
context.savePackage,
520-
registry ? [`--registry="${registry}"`] : undefined,
521-
undefined,
525+
await packageManager.add(
526+
packageIdentifier.toString(),
527+
'none',
528+
savePackage !== 'dependencies',
529+
false,
530+
true,
531+
{
532+
registry,
533+
},
522534
);
523-
524-
if (!success) {
525-
throw new CommandError('Unable to install package');
526-
}
527535
}
528536
}
529537

@@ -631,24 +639,28 @@ export default class AddCommandModule
631639
return cachedVersion;
632640
}
633641

634-
const { logger, root } = this.context;
635-
let installedPackage;
642+
const { root } = this.context;
643+
let installedPackagePath;
636644
try {
637-
installedPackage = this.rootRequire.resolve(join(name, 'package.json'));
645+
installedPackagePath = this.rootRequire.resolve(join(name, 'package.json'));
638646
} catch {}
639647

640-
if (installedPackage) {
648+
if (installedPackagePath) {
641649
try {
642-
const installed = await fetchPackageManifest(dirname(installedPackage), logger);
643-
this.#projectVersionCache.set(name, installed.version);
650+
const installedPackage = JSON.parse(
651+
await fs.readFile(installedPackagePath, 'utf-8'),
652+
) as PackageManifest;
653+
this.#projectVersionCache.set(name, installedPackage.version);
644654

645-
return installed.version;
655+
return installedPackage.version;
646656
} catch {}
647657
}
648658

649659
let projectManifest;
650660
try {
651-
projectManifest = await fetchPackageManifest(root, logger);
661+
projectManifest = JSON.parse(
662+
await fs.readFile(join(root, 'package.json'), 'utf-8'),
663+
) as PackageManifest;
652664
} catch {}
653665

654666
if (projectManifest) {

0 commit comments

Comments
 (0)