refactor(ngcc): simplify `DirectoryWalkerEntryPointFinder` (#36305)

This commit simplifies the `DirectoryWalkerEntryPointFinder` inter-method
calling to make it easier to follow, and also to support controlling
walking of a directory based on its children.

PR Close #36305

packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts

@@ -5,7 +5,7 @@
  * 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
  */
-import {AbsoluteFsPath, FileSystem, join, resolve} from '../../../src/ngtsc/file_system';
+import {AbsoluteFsPath, FileSystem, PathSegment} from '../../../src/ngtsc/file_system';
 import {DependencyResolver, SortedEntryPointsInfo} from '../dependencies/dependency_resolver';
 import {Logger} from '../logging/logger';
 import {NgccConfiguration} from '../packages/configuration';
@@ -14,7 +14,7 @@ import {EntryPointManifest} from '../packages/entry_point_manifest';
 import {PathMappings} from '../utils';
 import {NGCC_DIRECTORY} from '../writing/new_entry_point_file_writer';
 import {EntryPointFinder} from './interface';
-import {getBasePaths} from './utils';
+import {getBasePaths, trackDuration} from './utils';
 
 /**
  * An EntryPointFinder that searches for all entry-points that can be found given a `basePath` and
@@ -33,141 +33,144 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder {
   findEntryPoints(): SortedEntryPointsInfo {
     const unsortedEntryPoints: EntryPoint[] = [];
     for (const basePath of this.basePaths) {
-      let entryPoints = this.entryPointManifest.readEntryPointsUsingManifest(basePath);
+      const entryPoints = this.entryPointManifest.readEntryPointsUsingManifest(basePath) ||
-      if (entryPoints === null) {
+          this.walkBasePathForPackages(basePath);
-        this.logger.debug(
-            `No manifest found for ${basePath} so walking the directories for entry-points.`);
-        const startTime = Date.now();
-        entryPoints = this.walkDirectoryForEntryPoints(basePath);
-        const duration = Math.round((Date.now() - startTime) / 100) / 10;
-        this.logger.debug(`Walking directories took ${duration}s.`);
-
-        this.entryPointManifest.writeEntryPointManifest(basePath, entryPoints);
-      }
       unsortedEntryPoints.push(...entryPoints);
     }
     return this.resolver.sortEntryPointsByDependency(unsortedEntryPoints);
   }
 
   /**
-   * Look for entry points that need to be compiled, starting at the source directory.
+   * Search the `basePath` for possible Angular packages and entry-points.
-   * The function will recurse into directories that start with `@...`, e.g. `@angular/...`.
+   *
-   * @param sourceDirectory An absolute path to the root directory where searching begins.
+   * @param basePath The path at which to start the search
+   * @returns an array of `EntryPoint`s that were found within `basePath`.
    */
-  walkDirectoryForEntryPoints(sourceDirectory: AbsoluteFsPath): EntryPoint[] {
+  walkBasePathForPackages(basePath: AbsoluteFsPath): EntryPoint[] {
-    const entryPoints = this.getEntryPointsForPackage(sourceDirectory);
+    this.logger.debug(
-    if (entryPoints === null) {
+        `No manifest found for ${basePath} so walking the directories for entry-points.`);
+    const entryPoints: EntryPoint[] = trackDuration(
+        () => this.walkDirectoryForPackages(basePath),
+        duration => this.logger.debug(`Walking ${basePath} for entry-points took ${duration}s.`));
+    this.entryPointManifest.writeEntryPointManifest(basePath, entryPoints);
+    return entryPoints;
+  }
+
+  /**
+   * Look for Angular packages that need to be compiled, starting at the source directory.
+   * The function will recurse into directories that start with `@...`, e.g. `@angular/...`.
+   *
+   * @param sourceDirectory An absolute path to the root directory where searching begins.
+   * @returns an array of `EntryPoint`s that were found within `sourceDirectory`.
+   */
+  walkDirectoryForPackages(sourceDirectory: AbsoluteFsPath): EntryPoint[] {
+    // Try to get a primary entry point from this directory
+    const primaryEntryPoint =
+        getEntryPointInfo(this.fs, this.config, this.logger, sourceDirectory, sourceDirectory);
+
+    // If there is an entry-point but it is not compatible with ngcc (it has a bad package.json or
+    // invalid typings) then exit. It is unlikely that such an entry point has a dependency on an
+    // Angular library.
+    if (primaryEntryPoint === INCOMPATIBLE_ENTRY_POINT) {
       return [];
     }
 
-    if (entryPoints.length > 0) {
+    const entryPoints: EntryPoint[] = [];
-      // The `sourceDirectory` is an entry point itself so no need to search its sub-directories.
+    if (primaryEntryPoint !== NO_ENTRY_POINT) {
-      // Also check for any nested node_modules in this package but only if it was compiled by
+      entryPoints.push(primaryEntryPoint);
-      // Angular.
+      this.collectSecondaryEntryPoints(
-      // It is unlikely that a non Angular entry point has a dependency on an Angular library.
+          entryPoints, sourceDirectory, sourceDirectory, this.fs.readdir(sourceDirectory));
+
+      // Also check for any nested node_modules in this package but only if at least one of the
+      // entry-points was compiled by Angular.
       if (entryPoints.some(e => e.compiledByAngular)) {
         const nestedNodeModulesPath = this.fs.join(sourceDirectory, 'node_modules');
         if (this.fs.exists(nestedNodeModulesPath)) {
-          entryPoints.push(...this.walkDirectoryForEntryPoints(nestedNodeModulesPath));
+          entryPoints.push(...this.walkDirectoryForPackages(nestedNodeModulesPath));
         }
       }
 
       return entryPoints;
     }
 
-    this.fs
+    // The `sourceDirectory` was not a package (i.e. there was no package.json)
-        .readdir(sourceDirectory)
+    // So search its sub-directories for Angular packages and entry-points
-        // Not interested in hidden files
+    for (const path of this.fs.readdir(sourceDirectory)) {
-        .filter(p => !p.startsWith('.'))
+      if (isIgnorablePath(path)) {
-        // Ignore node_modules
+        // Ignore hidden files, node_modules and ngcc directory
-        .filter(p => p !== 'node_modules' && p !== NGCC_DIRECTORY)
+        continue;
-        // Only interested in directories (and only those that are not symlinks)
+      }
-        .filter(p => {
+
-          const stat = this.fs.lstat(resolve(sourceDirectory, p));
+      const absolutePath = this.fs.resolve(sourceDirectory, path);
-          return stat.isDirectory() && !stat.isSymbolicLink();
+      const stat = this.fs.lstat(absolutePath);
-        })
+      if (stat.isSymbolicLink() || !stat.isDirectory()) {
-        .forEach(p => {
+        // Ignore symbolic links and non-directories
-          // Package is a potential namespace containing packages (e.g `@angular`).
+        continue;
-          const packagePath = join(sourceDirectory, p);
+      }
-          entryPoints.push(...this.walkDirectoryForEntryPoints(packagePath));
+
-        });
+      entryPoints.push(...this.walkDirectoryForPackages(this.fs.join(sourceDirectory, path)));
+    }
+
     return entryPoints;
   }
 
   /**
-   * Recurse the folder structure looking for all the entry points
+   * Search the `directory` looking for any secondary entry-points for a package, adding any that
-   * @param packagePath The absolute path to an npm package that may contain entry points
+   * are found to the `entryPoints` array.
-   * @returns An array of entry points that were discovered or null when it's not a valid entrypoint
+   *
+   * @param entryPoints An array where we will add any entry-points found in this directory
+   * @param packagePath The absolute path to the package that may contain entry-points
+   * @param directory The current directory being searched
+   * @param paths The paths contained in the current `directory`.
    */
-  private getEntryPointsForPackage(packagePath: AbsoluteFsPath): EntryPoint[]|null {
+  private collectSecondaryEntryPoints(
-    const entryPoints: EntryPoint[] = [];
+      entryPoints: EntryPoint[], packagePath: AbsoluteFsPath, directory: AbsoluteFsPath,
-
+      paths: PathSegment[]): void {
-    // Try to get an entry point from the top level package directory
+    for (const path of paths) {
-    const topLevelEntryPoint =
+      if (isIgnorablePath(path)) {
-        getEntryPointInfo(this.fs, this.config, this.logger, packagePath, packagePath);
+        // Ignore hidden files, node_modules and ngcc directory
-
+        continue;
-    // If there is no primary entry-point then exit
-    if (topLevelEntryPoint === NO_ENTRY_POINT) {
-      return [];
-    }
-
-    if (topLevelEntryPoint === INCOMPATIBLE_ENTRY_POINT) {
-      return null;
-    }
-
-    // Otherwise store it and search for secondary entry-points
-    entryPoints.push(topLevelEntryPoint);
-    this.walkDirectory(packagePath, packagePath, (path, isDirectory) => {
-      if (!path.endsWith('.js') && !isDirectory) {
-        return false;
       }
 
-      // If the path is a JS file then strip its extension and see if we can match an entry-point.
+      const absolutePath = this.fs.resolve(directory, path);
-      const possibleEntryPointPath = isDirectory ? path : stripJsExtension(path);
+      const stat = this.fs.lstat(absolutePath);
+      if (stat.isSymbolicLink()) {
+        // Ignore symbolic links
+        continue;
+      }
+
+      const isDirectory = stat.isDirectory();
+      if (!path.endsWith('.js') && !isDirectory) {
+        // Ignore files that do not end in `.js`
+        continue;
+      }
+
+      // If the path is a JS file then strip its extension and see if we can match an
+      // entry-point.
+      const possibleEntryPointPath = isDirectory ? absolutePath : stripJsExtension(absolutePath);
+      let isEntryPoint = false;
       const subEntryPoint =
           getEntryPointInfo(this.fs, this.config, this.logger, packagePath, possibleEntryPointPath);
-      if (subEntryPoint === NO_ENTRY_POINT || subEntryPoint === INCOMPATIBLE_ENTRY_POINT) {
+      if (subEntryPoint !== NO_ENTRY_POINT && subEntryPoint !== INCOMPATIBLE_ENTRY_POINT) {
-        return false;
+        entryPoints.push(subEntryPoint);
+        isEntryPoint = true;
       }
-      entryPoints.push(subEntryPoint);
-      return true;
-    });
 
-    return entryPoints;
+      if (!isDirectory || !isEntryPoint) {
-  }
+        // This path is not an entry-point directory so we are done
+        continue;
+      }
 
-  /**
+      const childPaths = this.fs.readdir(absolutePath);
-   * Recursively walk a directory and its sub-directories, applying a given
+      this.collectSecondaryEntryPoints(entryPoints, packagePath, absolutePath, childPaths);
-   * function to each directory.
+    }
-   * @param dir the directory to recursively walk.
-   * @param fn the function to apply to each directory.
-   */
-  private walkDirectory(
-      packagePath: AbsoluteFsPath, dir: AbsoluteFsPath,
-      fn: (path: AbsoluteFsPath, isDirectory: boolean) => boolean) {
-    return this.fs
-        .readdir(dir)
-        // Not interested in hidden files
-        .filter(path => !path.startsWith('.'))
-        // Ignore node_modules
-        .filter(path => path !== 'node_modules' && path !== NGCC_DIRECTORY)
-        .forEach(path => {
-          const absolutePath = resolve(dir, path);
-          const stat = this.fs.lstat(absolutePath);
-
-          if (stat.isSymbolicLink()) {
-            // We are not interested in symbolic links
-            return;
-          }
-
-          const containsEntryPoint = fn(absolutePath, stat.isDirectory());
-          if (containsEntryPoint) {
-            this.walkDirectory(packagePath, absolutePath, fn);
-          }
-        });
   }
 }
 
 function stripJsExtension<T extends string>(filePath: T): T {
   return filePath.replace(/\.js$/, '') as T;
 }
+
+function isIgnorablePath(path: PathSegment): boolean {
+  return path.startsWith('.') || path === 'node_modules' || path === NGCC_DIRECTORY;
+}

packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts

@@ -78,3 +78,19 @@ function removeContainedPaths(value: AbsoluteFsPath, index: number, array: Absol
   }
   return true;
 }
+
+/**
+ * Run a task and track how long it takes.
+ *
+ * @param task The task whose duration we are tracking
+ * @param log The function to call with the duration of the task
+ * @returns The result of calling `task`.
+ */
+export function trackDuration<T = void>(
+    task: () => T extends Promise<unknown>? never : T, log: (duration: number) => void): T {
+  const startTime = Date.now();
+  const result = task();
+  const duration = Math.round((Date.now() - startTime) / 100) / 10;
+  log(duration);
+  return result;
+}

packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts

@@ -230,7 +230,7 @@ runInEachFileSystem(() => {
         const finder = new DirectoryWalkerEntryPointFinder(
             fs, config, logger, resolver, manifest, _Abs('/nested_node_modules/node_modules'),
             undefined);
-        const spy = spyOn(finder, 'walkDirectoryForEntryPoints').and.callThrough();
+        const spy = spyOn(finder, 'walkDirectoryForPackages').and.callThrough();
         const {entryPoints} = finder.findEntryPoints();
         expect(spy.calls.allArgs()).toEqual([
           [_Abs(basePath)],
@@ -252,7 +252,7 @@ runInEachFileSystem(() => {
 
         const finder = new DirectoryWalkerEntryPointFinder(
             fs, config, logger, resolver, manifest, basePath, undefined);
-        const spy = spyOn(finder, 'walkDirectoryForEntryPoints').and.callThrough();
+        const spy = spyOn(finder, 'walkDirectoryForPackages').and.callThrough();
         const {entryPoints} = finder.findEntryPoints();
         expect(spy.calls.allArgs()).toEqual([
           [_Abs(basePath)],
@@ -276,7 +276,7 @@ runInEachFileSystem(() => {
 
         const finder = new DirectoryWalkerEntryPointFinder(
             fs, config, logger, resolver, manifest, basePath, undefined);
-        const spy = spyOn(finder, 'walkDirectoryForEntryPoints').and.callThrough();
+        const spy = spyOn(finder, 'walkDirectoryForPackages').and.callThrough();
         const {entryPoints} = finder.findEntryPoints();
         expect(spy.calls.allArgs()).toEqual([
           [_Abs(basePath)],