From fceb8a31c41848864705601b52d6bd6fb1e0bdd5 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 21 Oct 2025 10:38:25 -0400 Subject: [PATCH 1/3] refactor(@angular/cli): support non-registry sources in getPackageManifest The `getPackageManifest` method in the package manager abstraction was previously limited to fetching manifests from an NPM registry. This commit refactors the method to accept a generic `packageSpecifier` string. This allows the abstraction to leverage the underlying package manager's native ability to read manifests from various sources, including local tarballs (`.tgz`), local directories (`file:`), and git URLs, in addition to registry specifiers. This makes the `PackageManager` class more robust and capable of handling all package sources supported by commands like `ng add`. --- .../src/package-managers/package-manager.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/angular/cli/src/package-managers/package-manager.ts b/packages/angular/cli/src/package-managers/package-manager.ts index 4f4620994769..0a52ffd8578d 100644 --- a/packages/angular/cli/src/package-managers/package-manager.ts +++ b/packages/angular/cli/src/package-managers/package-manager.ts @@ -343,29 +343,30 @@ export class PackageManager { } /** - * Fetches the registry manifest for a specific version of a package. + * Fetches the manifest for a package. * The manifest is similar to the package's `package.json` file. - * @param packageName The name of the package to fetch the manifest for. - * @param version The version of the package to fetch the manifest for. + * This method can handle various package sources, including registry specifiers + * (`name@version`), local tarballs (`./package.tgz`), and local directories (`file:.`). + * @param packageSpecifier The identifier of the package to fetch the manifest for. * @param options Options for the fetch. * @param options.timeout The maximum time in milliseconds to wait for the command to complete. - * @param options.registry The registry to use for the fetch. + * @param options.registry The registry to use for the fetch (for registry specifiers). * @param options.bypassCache If true, ignores the in-memory cache and fetches fresh data. * @returns A promise that resolves to the `PackageManifest` object, or `null` if the package is not found. */ async getPackageManifest( - packageName: string, - version: string, + packageSpecifier: string, options: { timeout?: number; registry?: string; bypassCache?: boolean } = {}, ): Promise { - const specifier = `${packageName}@${version}`; - const commandArgs = [...this.descriptor.getManifestCommand, specifier]; + const commandArgs = [...this.descriptor.getManifestCommand, packageSpecifier]; const formatter = this.descriptor.viewCommandFieldArgFormatter; if (formatter) { commandArgs.push(...formatter(MANIFEST_FIELDS)); } - const cacheKey = options.registry ? `${specifier}|${options.registry}` : specifier; + const cacheKey = options.registry + ? `${packageSpecifier}|${options.registry}` + : packageSpecifier; return this.#fetchAndParse( commandArgs, From 4f000559db14db3900b7c3c6dcc151a8e1052fef Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 20 Oct 2025 18:14:23 -0400 Subject: [PATCH 2/3] 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. --- packages/angular/cli/src/commands/add/cli.ts | 230 ++++++++++--------- 1 file changed, 121 insertions(+), 109 deletions(-) diff --git a/packages/angular/cli/src/commands/add/cli.ts b/packages/angular/cli/src/commands/add/cli.ts index 1ea2fff699c6..7adaac1c5067 100644 --- a/packages/angular/cli/src/commands/add/cli.ts +++ b/packages/angular/cli/src/commands/add/cli.ts @@ -8,12 +8,12 @@ import { Listr, ListrRenderer, ListrTaskWrapper, color, figures } from 'listr2'; import assert from 'node:assert'; +import { promises as fs } from 'node:fs'; import { createRequire } from 'node:module'; import { dirname, join } from 'node:path'; import npa from 'npm-package-arg'; import { Range, compare, intersects, prerelease, satisfies, valid } from 'semver'; import { Argv } from 'yargs'; -import { PackageManager } from '../../../lib/config/workspace-schema'; import { CommandModuleImplementation, Options, @@ -23,14 +23,14 @@ import { SchematicsCommandArgs, SchematicsCommandModule, } from '../../command-builder/schematics-command-module'; -import { assertIsError } from '../../utilities/error'; import { NgAddSaveDependency, + PackageManager, PackageManifest, PackageMetadata, - fetchPackageManifest, - fetchPackageMetadata, -} from '../../utilities/package-metadata'; + createPackageManager, +} from '../../package-managers'; +import { assertIsError } from '../../utilities/error'; import { isTTY } from '../../utilities/tty'; import { VERSION } from '../../utilities/version'; @@ -44,8 +44,8 @@ interface AddCommandArgs extends SchematicsCommandArgs { } interface AddCommandTaskContext { + packageManager: PackageManager; packageIdentifier: npa.Result; - usingYarn?: boolean; savePackage?: NgAddSaveDependency; collectionName?: string; executeSchematic: AddCommandModule['executeSchematic']; @@ -173,12 +173,12 @@ export default class AddCommandModule } } - const taskContext: AddCommandTaskContext = { + const taskContext = { packageIdentifier, executeSchematic: this.executeSchematic.bind(this), getPeerDependencyConflicts: this.getPeerDependencyConflicts.bind(this), dryRun: options.dryRun, - }; + } as AddCommandTaskContext; const tasks = new Listr( [ @@ -294,13 +294,16 @@ export default class AddCommandModule } } - private determinePackageManagerTask( + private async determinePackageManagerTask( context: AddCommandTaskContext, task: AddCommandTaskWrapper, - ): void { - const { packageManager } = this.context; - context.usingYarn = packageManager.name === PackageManager.Yarn; - task.output = `Using package manager: ${color.dim(packageManager.name)}`; + ): Promise { + context.packageManager = await createPackageManager({ + cwd: this.context.root, + logger: this.context.logger, + dryRun: context.dryRun, + }); + task.output = `Using package manager: ${color.dim(context.packageManager.name)}`; } private async findCompatiblePackageVersionTask( @@ -308,77 +311,87 @@ export default class AddCommandModule task: AddCommandTaskWrapper, options: Options, ): Promise { - const { logger } = this.context; - const { verbose, registry } = options; + const { registry, verbose } = options; + const { packageManager, packageIdentifier } = context; + const packageName = packageIdentifier.name; - assert( - context.packageIdentifier.name, - 'Registry package identifiers should always have a name.', - ); + assert(packageName, 'Registry package identifiers should always have a name.'); - // only package name provided; search for viable version - // plus special cases for packages that did not have peer deps setup - let packageMetadata; + const rejectionReasons: string[] = []; + + // Attempt to use the 'latest' tag from the registry. try { - packageMetadata = await fetchPackageMetadata(context.packageIdentifier.name, logger, { + const latestManifest = await packageManager.getPackageManifest(`${packageName}@latest`, { registry, - usingYarn: context.usingYarn, - verbose, }); + + if (latestManifest) { + const conflicts = await this.getPeerDependencyConflicts(latestManifest); + if (!conflicts) { + context.packageIdentifier = npa.resolve(latestManifest.name, latestManifest.version); + task.output = `Found compatible package version: ${color.blue(latestManifest.version)}.`; + + return; + } + rejectionReasons.push(...conflicts); + } } catch (e) { assertIsError(e); throw new CommandError(`Unable to load package information from registry: ${e.message}`); } - const rejectionReasons: string[] = []; + // 'latest' is invalid or not found, search for most recent matching package. + task.output = + 'Could not find a compatible version with `latest`. Searching for a compatible version.'; - // Start with the version tagged as `latest` if it exists - const latestManifest = packageMetadata.tags['latest']; - if (latestManifest) { - const latestConflicts = await this.getPeerDependencyConflicts(latestManifest); - if (latestConflicts) { - // 'latest' is invalid so search for most recent matching package - rejectionReasons.push(...latestConflicts); - } else { - context.packageIdentifier = npa.resolve(latestManifest.name, latestManifest.version); - task.output = `Found compatible package version: ${color.blue(latestManifest.version)}.`; + let packageMetadata; + try { + packageMetadata = await packageManager.getRegistryMetadata(packageName, { + registry, + }); + } catch (e) { + assertIsError(e); + throw new CommandError(`Unable to load package information from registry: ${e.message}`); + } - return; - } + if (!packageMetadata) { + throw new CommandError('Unable to load package information from registry.'); } - // Allow prelease versions if the CLI itself is a prerelease + // Allow prelease versions if the CLI itself is a prerelease. const allowPrereleases = !!prerelease(VERSION.full); - const versionManifests = this.#getPotentialVersionManifests(packageMetadata, allowPrereleases); + const potentialVersions = this.#getPotentialVersions(packageMetadata, allowPrereleases); - let found = false; - for (const versionManifest of versionManifests) { - // Already checked the 'latest' version - if (latestManifest?.version === versionManifest.version) { + let found; + for (const version of potentialVersions) { + const manifest = await packageManager.getPackageManifest(`${packageName}@${version}`, { + registry, + }); + if (!manifest) { continue; } - const conflicts = await this.getPeerDependencyConflicts(versionManifest); + const conflicts = await this.getPeerDependencyConflicts(manifest); if (conflicts) { - if (options.verbose || rejectionReasons.length < DEFAULT_CONFLICT_DISPLAY_LIMIT) { + if (verbose || rejectionReasons.length < DEFAULT_CONFLICT_DISPLAY_LIMIT) { rejectionReasons.push(...conflicts); } continue; } - context.packageIdentifier = npa.resolve(versionManifest.name, versionManifest.version); - found = true; + context.packageIdentifier = npa.resolve(manifest.name, manifest.version); + found = manifest; break; } if (!found) { - let message = `Unable to find compatible package. Using 'latest' tag.`; + let message = `Unable to find compatible package.`; if (rejectionReasons.length > 0) { message += '\nThis is often because of incompatible peer dependencies.\n' + 'These versions were rejected due to the following conflicts:\n' + rejectionReasons - .slice(0, options.verbose ? undefined : DEFAULT_CONFLICT_DISPLAY_LIMIT) + .slice(0, verbose ? undefined : DEFAULT_CONFLICT_DISPLAY_LIMIT) .map((r) => ` - ${r}`) .join('\n'); } @@ -390,35 +403,25 @@ export default class AddCommandModule } } - #getPotentialVersionManifests( - packageMetadata: PackageMetadata, - allowPrereleases: boolean, - ): PackageManifest[] { + #getPotentialVersions(packageMetadata: PackageMetadata, allowPrereleases: boolean): string[] { const versionExclusions = packageVersionExclusions[packageMetadata.name]; - const versionManifests = Object.values(packageMetadata.versions).filter( - (value: PackageManifest) => { - // Prerelease versions are not stable and should not be considered by default - if (!allowPrereleases && prerelease(value.version)) { - return false; - } - // Deprecated versions should not be used or considered - if (value.deprecated) { - return false; - } - // Excluded package versions should not be considered - if ( - versionExclusions && - satisfies(value.version, versionExclusions, { includePrerelease: true }) - ) { - return false; - } - return true; - }, - ); + const versions = Object.values(packageMetadata.versions).filter((version) => { + // Prerelease versions are not stable and should not be considered by default + if (!allowPrereleases && prerelease(version)) { + return false; + } + + // Excluded package versions should not be considered + if (versionExclusions && satisfies(version, versionExclusions, { includePrerelease: true })) { + return false; + } + + return true; + }); // Sort in reverse SemVer order so that the newest compatible version is chosen - return versionManifests.sort((a, b) => compare(b.version, a.version, true)); + return versions.sort((a, b) => compare(b, a, true)); } private async loadPackageInfoTask( @@ -426,16 +429,16 @@ export default class AddCommandModule task: AddCommandTaskWrapper, options: Options, ): Promise { - const { logger } = this.context; - const { verbose, registry } = options; + const { registry } = options; let manifest; try { - manifest = await fetchPackageManifest(context.packageIdentifier.toString(), logger, { - registry, - verbose, - usingYarn: context.usingYarn, - }); + manifest = await context.packageManager.getPackageManifest( + context.packageIdentifier.toString(), + { + registry, + }, + ); } catch (e) { assertIsError(e); throw new CommandError( @@ -443,6 +446,12 @@ export default class AddCommandModule ); } + if (!manifest) { + throw new CommandError( + `Unable to fetch package information for '${context.packageIdentifier}'.`, + ); + } + context.hasSchematics = !!manifest.schematics; context.savePackage = manifest['ng-add']?.save; context.collectionName = manifest.name; @@ -487,43 +496,42 @@ export default class AddCommandModule task: AddCommandTaskWrapper, options: Options, ): Promise { - const { packageManager } = this.context; const { registry } = options; + const { packageManager, packageIdentifier, savePackage } = context; // Only show if installation will actually occur task.title = 'Installing package'; - if (context.savePackage === false) { + if (savePackage === false) { task.title += ' in temporary location'; // Temporary packages are located in a different directory // Hence we need to resolve them using the temp path - const { success, tempNodeModules } = await packageManager.installTemp( - context.packageIdentifier.toString(), - registry ? [`--registry="${registry}"`] : undefined, + const { workingDirectory } = await packageManager.acquireTempPackage( + packageIdentifier.toString(), + { + registry, + }, ); - const tempRequire = createRequire(tempNodeModules + '/'); + + const tempRequire = createRequire(workingDirectory + '/'); assert(context.collectionName, 'Collection name should always be available'); const resolvedCollectionPath = tempRequire.resolve( join(context.collectionName, 'package.json'), ); - if (!success) { - throw new CommandError('Unable to install package'); - } - context.collectionName = dirname(resolvedCollectionPath); } else { - const success = await packageManager.install( - context.packageIdentifier.toString(), - context.savePackage, - registry ? [`--registry="${registry}"`] : undefined, - undefined, + await packageManager.add( + packageIdentifier.toString(), + 'none', + savePackage !== 'dependencies', + false, + true, + { + registry, + }, ); - - if (!success) { - throw new CommandError('Unable to install package'); - } } } @@ -631,24 +639,28 @@ export default class AddCommandModule return cachedVersion; } - const { logger, root } = this.context; - let installedPackage; + const { root } = this.context; + let installedPackagePath; try { - installedPackage = this.rootRequire.resolve(join(name, 'package.json')); + installedPackagePath = this.rootRequire.resolve(join(name, 'package.json')); } catch {} - if (installedPackage) { + if (installedPackagePath) { try { - const installed = await fetchPackageManifest(dirname(installedPackage), logger); - this.#projectVersionCache.set(name, installed.version); + const installedPackage = JSON.parse( + await fs.readFile(installedPackagePath, 'utf-8'), + ) as PackageManifest; + this.#projectVersionCache.set(name, installedPackage.version); - return installed.version; + return installedPackage.version; } catch {} } let projectManifest; try { - projectManifest = await fetchPackageManifest(root, logger); + projectManifest = JSON.parse( + await fs.readFile(join(root, 'package.json'), 'utf-8'), + ) as PackageManifest; } catch {} if (projectManifest) { From a512fb3388d44c63c03afa1aea07e72002e1fc2e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 20 Oct 2025 18:16:25 -0400 Subject: [PATCH 3/3] perf(@angular/cli): optimize `ng add` version discovery The `ng add` command's version discovery mechanism could be slow when the `latest` tag of a package was incompatible, as it would fall back to exhaustively fetching the manifest for every available version. This commit introduces a performance optimization that uses a heuristic-based search. The new logic first identifies the latest release within each major version line and checks only those for compatibility. This dramatically reduces the number of network requests in the common case where peer dependency conflicts align with major versions. The exhaustive, version-by-version search is retained as a fallback to ensure correctness in edge cases. --- packages/angular/cli/src/commands/add/cli.ts | 98 +++++++++++++++----- 1 file changed, 76 insertions(+), 22 deletions(-) diff --git a/packages/angular/cli/src/commands/add/cli.ts b/packages/angular/cli/src/commands/add/cli.ts index 7adaac1c5067..bd17fee6cd78 100644 --- a/packages/angular/cli/src/commands/add/cli.ts +++ b/packages/angular/cli/src/commands/add/cli.ts @@ -8,11 +8,11 @@ import { Listr, ListrRenderer, ListrTaskWrapper, color, figures } from 'listr2'; import assert from 'node:assert'; -import { promises as fs } from 'node:fs'; +import fs from 'node:fs/promises'; import { createRequire } from 'node:module'; import { dirname, join } from 'node:path'; import npa from 'npm-package-arg'; -import { Range, compare, intersects, prerelease, satisfies, valid } from 'semver'; +import semver, { Range, compare, intersects, prerelease, satisfies, valid } from 'semver'; import { Argv } from 'yargs'; import { CommandModuleImplementation, @@ -358,30 +358,27 @@ export default class AddCommandModule throw new CommandError('Unable to load package information from registry.'); } - // Allow prelease versions if the CLI itself is a prerelease. - const allowPrereleases = !!prerelease(VERSION.full); + // Allow prelease versions if the CLI itself is a prerelease or locally built. + const allowPrereleases = !!prerelease(VERSION.full) || VERSION.full === '0.0.0'; const potentialVersions = this.#getPotentialVersions(packageMetadata, allowPrereleases); - let found; - for (const version of potentialVersions) { - const manifest = await packageManager.getPackageManifest(`${packageName}@${version}`, { + // Heuristic-based search: Check the latest release of each major version first. + const majorVersions = this.#getMajorVersions(potentialVersions); + let found = await this.#findCompatibleVersion(context, majorVersions, { + registry, + verbose, + rejectionReasons, + }); + + // Exhaustive search: If no compatible major version is found, fall back to checking all versions. + if (!found) { + const checkedVersions = new Set(majorVersions); + const remainingVersions = potentialVersions.filter((v) => !checkedVersions.has(v)); + found = await this.#findCompatibleVersion(context, remainingVersions, { registry, + verbose, + rejectionReasons, }); - if (!manifest) { - continue; - } - - const conflicts = await this.getPeerDependencyConflicts(manifest); - if (conflicts) { - if (verbose || rejectionReasons.length < DEFAULT_CONFLICT_DISPLAY_LIMIT) { - rejectionReasons.push(...conflicts); - } - continue; - } - - context.packageIdentifier = npa.resolve(manifest.name, manifest.version); - found = manifest; - break; } if (!found) { @@ -403,10 +400,54 @@ export default class AddCommandModule } } + async #findCompatibleVersion( + context: AddCommandTaskContext, + versions: string[], + options: { + registry?: string; + verbose?: boolean; + rejectionReasons: string[]; + }, + ): Promise { + const { packageManager, packageIdentifier } = context; + const { registry, verbose, rejectionReasons } = options; + const packageName = packageIdentifier.name; + assert(packageName, 'Package name must be defined.'); + + for (const version of versions) { + const manifest = await packageManager.getPackageManifest(`${packageName}@${version}`, { + registry, + }); + if (!manifest) { + continue; + } + + const conflicts = await this.getPeerDependencyConflicts(manifest); + if (conflicts) { + if (verbose || rejectionReasons.length < DEFAULT_CONFLICT_DISPLAY_LIMIT) { + rejectionReasons.push(...conflicts); + } + continue; + } + + context.packageIdentifier = npa.resolve(manifest.name, manifest.version); + + return manifest; + } + + return null; + } + #getPotentialVersions(packageMetadata: PackageMetadata, allowPrereleases: boolean): string[] { const versionExclusions = packageVersionExclusions[packageMetadata.name]; + const latestVersion = packageMetadata['dist-tags']['latest']; const versions = Object.values(packageMetadata.versions).filter((version) => { + // Latest tag has already been checked + if (latestVersion && version === latestVersion) { + return false; + } + // Prerelease versions are not stable and should not be considered by default if (!allowPrereleases && prerelease(version)) { return false; @@ -424,6 +465,19 @@ export default class AddCommandModule return versions.sort((a, b) => compare(b, a, true)); } + #getMajorVersions(versions: string[]): string[] { + const majorVersions = new Map(); + for (const version of versions) { + const major = semver.major(version); + const existing = majorVersions.get(major); + if (!existing || semver.gt(version, existing)) { + majorVersions.set(major, version); + } + } + + return [...majorVersions.values()].sort((a, b) => compare(b, a, true)); + } + private async loadPackageInfoTask( context: AddCommandTaskContext, task: AddCommandTaskWrapper,