perf(@angular/cli): optimize package manager discovery with stat-based probing

This refactors the package manager discovery logic to use 'stat' instead of 'readdir', significantly improving performance in directories with many files. It also simplifies the internal host abstraction by removing the now unused 'readdir' method.

Author: clydin

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

@@ -21,18 +21,6 @@ import {
   SUPPORTED_PACKAGE_MANAGERS,
 } from './package-manager-descriptor';
 
-/**
- * A map from lockfile names to their corresponding package manager.
- * This is a performance optimization to avoid iterating over all possible
- * lockfiles in every directory.
- */
-const LOCKFILE_TO_PACKAGE_MANAGER = new Map<string, PackageManagerName>();
-for (const [name, descriptor] of Object.entries(SUPPORTED_PACKAGE_MANAGERS)) {
-  for (const lockfile of descriptor.lockfiles) {
-    LOCKFILE_TO_PACKAGE_MANAGER.set(lockfile, name as PackageManagerName);
-  }
-}
-
 /**
  * Searches a directory for lockfiles and returns a set of package managers that correspond to them.
  * @param host A `Host` instance for interacting with the file system.
@@ -47,25 +35,32 @@ async function findLockfiles(
 ): Promise<Set<PackageManagerName>> {
   logger?.debug(`Searching for lockfiles in '${directory}'...`);
 
-  try {
-    const files = await host.readdir(directory);
-    const foundPackageManagers = new Set<PackageManagerName>();
-
-    for (const file of files) {
-      const packageManager = LOCKFILE_TO_PACKAGE_MANAGER.get(file);
-      if (packageManager) {
-        logger?.debug(`  Found '${file}'.`);
-        foundPackageManagers.add(packageManager);
-      }
+  const foundPackageManagers = new Set<PackageManagerName>();
+  const checks: Promise<void>[] = [];
+
+  for (const [name, descriptor] of Object.entries(SUPPORTED_PACKAGE_MANAGERS)) {
+    const manager = name as PackageManagerName;
+    for (const lockfile of descriptor.lockfiles) {
+      checks.push(
+        (async () => {
+          try {
+            const path = join(directory, lockfile);
+            const stats = await host.stat(path);
+            if (stats.isFile()) {
+              logger?.debug(`  Found '${lockfile}'.`);
+              foundPackageManagers.add(manager);
+            }
+          } catch {
+            // File does not exist or cannot be accessed.
+          }
+        })(),
+      );
     }
+  }
 
-    return foundPackageManagers;
-  } catch (e) {
-    logger?.debug(`  Failed to read directory: ${e}`);
+  await Promise.all(checks);
 
-    // Ignore directories that don't exist or can't be read.
-    return new Set();
-  }
+  return foundPackageManagers;
 }
 
 /**

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

@@ -64,14 +64,6 @@ describe('discover', () => {
     expect(result).toBeNull();
   });
 
-  it('should handle file system errors during readdir gracefully', async () => {
-    const host = new MockHost({});
-    host.readdir = () => Promise.reject(new Error('Permission denied'));
-
-    const result = await discover(host, '/project');
-    expect(result).toBeNull();
-  });
-
   it('should handle file system errors during stat gracefully', async () => {
     const host = new MockHost({ '/project': ['.git'] });
     host.stat = () => Promise.reject(new Error('Permission denied'));

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

@@ -39,13 +39,6 @@ export interface Host {
    */
   stat(path: string): Promise<Stats>;
 
-  /**
-   * Reads the contents of a directory.
-   * @param path The path to the directory.
-   * @returns A promise that resolves to an array of file and directory names.
-   */
-  readdir(path: string): Promise<string[]>;
-
   /**
    * Reads the content of a file.
    * @param path The path to the file.
@@ -108,7 +101,6 @@ export interface Host {
  */
 export const NodeJS_HOST: Host = {
   stat,
-  readdir,
   mkdir,
   readFile: (path: string) => readFile(path, { encoding: 'utf8' }),
   copyFile: (src, dest) => copyFile(src, dest, constants.COPYFILE_FICLONE),

packages/angular/cli/src/package-managers/testing/mock-host.ts

@@ -19,7 +19,16 @@ export class MockHost implements Host {
   constructor(files: Record<string, string[] | true> = {}) {
     // Normalize paths to use forward slashes for consistency in tests.
     for (const [path, content] of Object.entries(files)) {
-      this.fs.set(path.replace(/\\/g, '/'), content);
+      const normalizedPath = path.replace(/\\/g, '/');
+      this.fs.set(normalizedPath, content);
+
+      // If the content is an array (directory listing), create entries for the files in it.
+      if (Array.isArray(content)) {
+        for (const file of content) {
+          const filePath = normalizedPath === '/' ? `/${file}` : `${normalizedPath}/${file}`;
+          this.fs.set(filePath, []); // Use empty array to represent a file (not `true` which is a dir)
+        }
+      }
     }
   }
 
@@ -34,17 +43,11 @@ export class MockHost implements Host {
     }
 
     // A `true` value signifies a directory in our mock file system.
-    return Promise.resolve({ isDirectory: () => content === true } as Stats);
-  }
-
-  readdir(path: string): Promise<string[]> {
-    const content = this.fs.get(path.replace(/\\/g, '/'));
-    if (content === true || content === undefined) {
-      // This should be a directory with a file list.
-      return Promise.reject(new Error(`Directory not found or not a directory: ${path}`));
-    }
-
-    return Promise.resolve(content);
+    // Anything else is considered a file for the purpose of this mock.
+    return Promise.resolve({
+      isDirectory: () => content === true,
+      isFile: () => content !== true,
+    } as Stats);
   }
 
   runCommand(): Promise<{ stdout: string; stderr: string }> {