refactor(@angular/cli): clean up package-metadata retrieval logic

With this change we clean up the package-metadata retrieval logic and types by using public `@types/` packages.

Also, we lazily require `pacote` since this has a large set to dependencies which slows down module resolution.

Author: alan-agius4

package.json

@@ -99,6 +99,7 @@
     "@types/express": "^4.16.0",
     "@types/glob": "^7.1.1",
     "@types/http-proxy": "^1.17.4",
+    "@types/ini": "^1.3.31",
     "@types/inquirer": "^8.0.0",
     "@types/jasmine": "~4.0.0",
     "@types/karma": "^6.3.0",
@@ -107,6 +108,7 @@
     "@types/node": "^14.15.0",
     "@types/node-fetch": "^2.1.6",
     "@types/npm-package-arg": "^6.1.0",
+    "@types/pacote": "^11.1.3",
     "@types/parse5-html-rewriting-stream": "^5.1.2",
     "@types/pidusage": "^2.0.1",
     "@types/postcss-preset-env": "^6.7.1",
@@ -117,6 +119,7 @@
     "@types/uuid": "^8.0.0",
     "@types/yargs": "^17.0.8",
     "@types/yargs-parser": "^21.0.0",
+    "@types/yarnpkg__lockfile": "^1.1.5",
     "@typescript-eslint/eslint-plugin": "5.16.0",
     "@typescript-eslint/parser": "5.16.0",
     "@yarnpkg/lockfile": "1.1.0",

packages/angular/cli/BUILD.bazel

@@ -55,13 +55,16 @@ ts_library(
         "//packages/angular_devkit/schematics/tools",
         "@npm//@angular/core",
         "@npm//@types/debug",
+        "@npm//@types/ini",
         "@npm//@types/inquirer",
         "@npm//@types/node",
         "@npm//@types/npm-package-arg",
+        "@npm//@types/pacote",
         "@npm//@types/resolve",
         "@npm//@types/semver",
         "@npm//@types/uuid",
         "@npm//@types/yargs",
+        "@npm//@types/yarnpkg__lockfile",
         "@npm//@yarnpkg/lockfile",
         "@npm//ansi-colors",
         "@npm//ini",

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

@@ -27,7 +27,7 @@ import { colors } from '../../utilities/color';
 import { installPackage, installTempPackage } from '../../utilities/install-package';
 import { ensureCompatibleNpm } from '../../utilities/package-manager';
 import {
-  NgAddSaveDepedency,
+  NgAddSaveDependency,
   PackageManifest,
   fetchPackageManifest,
   fetchPackageMetadata,
@@ -165,7 +165,10 @@ export class AddCommandModule
       }
 
       // Adjust the version based on name and peer dependencies
-      if (latestManifest && Object.keys(latestManifest.peerDependencies).length === 0) {
+      if (
+        latestManifest?.peerDependencies &&
+        Object.keys(latestManifest.peerDependencies).length === 0
+      ) {
         spinner.succeed(
           `Found compatible package version: ${colors.grey(packageIdentifier.toString())}.`,
         );
@@ -217,7 +220,7 @@ export class AddCommandModule
     }
 
     let collectionName = packageIdentifier.name;
-    let savePackage: NgAddSaveDepedency | undefined;
+    let savePackage: NgAddSaveDependency | undefined;
 
     try {
       spinner.start('Loading package information from registry...');
@@ -415,7 +418,8 @@ export class AddCommandModule
     } catch {}
 
     if (projectManifest) {
-      const version = projectManifest.dependencies[name] || projectManifest.devDependencies[name];
+      const version =
+        projectManifest.dependencies?.[name] || projectManifest.devDependencies?.[name];
       if (version) {
         return version;
       }

packages/angular/cli/src/commands/update/schematic/index.ts

@@ -9,11 +9,19 @@
 import { logging, tags } from '@angular-devkit/core';
 import { Rule, SchematicContext, SchematicsException, Tree } from '@angular-devkit/schematics';
 import * as npa from 'npm-package-arg';
+import type { Manifest } from 'pacote';
 import * as semver from 'semver';
-import { Dependency, JsonSchemaForNpmPackageJsonFiles } from '../../../utilities/package-json';
-import { NpmRepositoryPackageJson, getNpmPackageJson } from '../../../utilities/package-metadata';
+import {
+  NgPackageManifestProperties,
+  NpmRepositoryPackageJson,
+  getNpmPackageJson,
+} from '../../../utilities/package-metadata';
 import { Schema as UpdateSchema } from './schema';
 
+interface JsonSchemaForNpmPackageJsonFiles extends Manifest, NgPackageManifestProperties {
+  peerDependenciesMeta?: Record<string, { optional?: boolean }>;
+}
+
 type VersionRange = string & { __VERSION_RANGE: void };
 type PeerVersionTransform = string | ((range: string) => string);
 
@@ -264,7 +272,7 @@ function _performUpdate(
     throw new SchematicsException('package.json could not be parsed: ' + e.message);
   }
 
-  const updateDependency = (deps: Dependency, name: string, newVersion: string) => {
+  const updateDependency = (deps: Record<string, string>, name: string, newVersion: string) => {
     const oldVersion = deps[name];
     // We only respect caret and tilde ranges on update.
     const execResult = /^[\^~]/.exec(oldVersion);
@@ -367,6 +375,7 @@ function _getUpdateMetadata(
     } else if (
       typeof packageGroup == 'object' &&
       packageGroup &&
+      !Array.isArray(packageGroup) &&
       Object.values(packageGroup).every((x) => typeof x == 'string')
     ) {
       result.packageGroup = packageGroup;
@@ -463,15 +472,19 @@ function _usageMessage(
     )
     .map(({ name, info, version, tag, target }) => {
       // Look for packageGroup.
-      const packageGroup = target['ng-update']['packageGroup'];
+      const packageGroup = target['ng-update']?.['packageGroup'];
       if (packageGroup) {
-        const packageGroupName = target['ng-update']['packageGroupName'] || packageGroup[0];
+        const packageGroupNames = Array.isArray(packageGroup)
+          ? packageGroup
+          : Object.keys(packageGroup);
+
+        const packageGroupName = target['ng-update']?.['packageGroupName'] || packageGroupNames[0];
         if (packageGroupName) {
           if (packageGroups.has(name)) {
             return null;
           }
 
-          packageGroup.forEach((x: string) => packageGroups.set(x, packageGroupName));
+          packageGroupNames.forEach((x: string) => packageGroups.set(x, packageGroupName));
           packageGroups.set(packageGroupName, packageGroupName);
           name = packageGroupName;
         }
@@ -663,35 +676,37 @@ function _addPackageGroup(
     return;
   }
 
-  let packageGroup = ngUpdateMetadata['packageGroup'];
+  const packageGroup = ngUpdateMetadata['packageGroup'];
   if (!packageGroup) {
     return;
   }
+  let packageGroupNormalized: Record<string, string> = {};
   if (Array.isArray(packageGroup) && !packageGroup.some((x) => typeof x != 'string')) {
-    packageGroup = packageGroup.reduce((acc, curr) => {
+    packageGroupNormalized = packageGroup.reduce((acc, curr) => {
       acc[curr] = maybePackage;
 
       return acc;
     }, {} as { [name: string]: string });
-  }
-
-  // Only need to check if it's an object because we set it right the time before.
-  if (
-    typeof packageGroup != 'object' ||
-    packageGroup === null ||
-    Object.values(packageGroup).some((v) => typeof v != 'string')
+  } else if (
+    typeof packageGroup == 'object' &&
+    packageGroup &&
+    !Array.isArray(packageGroup) &&
+    Object.values(packageGroup).every((x) => typeof x == 'string')
   ) {
-    logger.warn(`packageGroup metadata of package ${npmPackageJson.name} is malformed.`);
+    packageGroupNormalized = packageGroup;
+  } else {
+    logger.warn(`packageGroup metadata of package ${npmPackageJson.name} is malformed. Ignoring.`);
 
     return;
   }
 
-  Object.keys(packageGroup)
-    .filter((name) => !packages.has(name)) // Don't override names from the command line.
-    .filter((name) => allDependencies.has(name)) // Remove packages that aren't installed.
-    .forEach((name) => {
-      packages.set(name, packageGroup[name]);
-    });
+  for (const [name, value] of Object.entries(packageGroupNormalized)) {
+    // Don't override names from the command line.
+    // Remove packages that aren't installed.
+    if (!packages.has(name) && allDependencies.has(name)) {
+      packages.set(name, value as VersionRange);
+    }
+  }
 }
 
 /**

packages/angular/cli/src/typings-bazel.d.ts

@@ -0,0 +1,14 @@
+/**
+ * @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.io/license
+ */
+
+/* eslint-disable import/no-extraneous-dependencies */
+// Workaround for https://github.com/bazelbuild/rules_nodejs/issues/1033
+// Alternative approach instead of https://github.com/angular/angular/pull/33226
+declare module '@yarnpkg/lockfile' {
+  export * from '@types/yarnpkg__lockfile';
+}

packages/angular/cli/src/typings.ts

@@ -6,32 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-declare module '@yarnpkg/lockfile' {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  export function parse(data: string): Record<string, any>;
-}
-
-declare module 'ini' {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  export function parse(data: string): Record<string, any>;
-}
-
 declare module 'npm-pick-manifest' {
   function pickManifest(
     metadata: import('./utilities/package-metadata').PackageMetadata,
     selector: string,
   ): import('./utilities/package-metadata').PackageManifest;
   export = pickManifest;
 }
-
-declare module 'pacote' {
-  export function manifest(
-    specifier: string,
-    options: Record<string, unknown>,
-  ): Promise<{ name: string; version: string }>;
-
-  export function packument(
-    specifier: string,
-    options: Record<string, unknown>,
-  ): Promise<import('./utilities/package-metadata').NpmRepositoryPackageJson>;
-}

packages/angular/cli/src/utilities/install-package.ts

@@ -11,7 +11,7 @@ import { existsSync, mkdtempSync, readFileSync, realpathSync, rmdirSync, writeFi
 import { tmpdir } from 'os';
 import { join, resolve } from 'path';
 import { PackageManager } from '../../lib/config/workspace-schema';
-import { NgAddSaveDepedency } from './package-metadata';
+import { NgAddSaveDependency } from './package-metadata';
 import { Spinner } from './spinner';
 
 interface PackageManagerOptions {
@@ -70,7 +70,7 @@ export async function installAllPackages(
 export async function installPackage(
   packageName: string,
   packageManager: PackageManager = PackageManager.Npm,
-  save: Exclude<NgAddSaveDepedency, false> = true,
+  save: Exclude<NgAddSaveDependency, false> = true,
   extraArgs: string[] = [],
   cwd = process.cwd(),
 ): Promise<1 | 0> {