From 372b9101e26caf463813d688e2a62f463f8846e9 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sun, 29 Mar 2020 17:13:43 +0100 Subject: [PATCH] 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 --- .../directory_walker_entry_point_finder.ts | 209 +++++++++--------- .../ngcc/src/entry_point_finder/utils.ts | 16 ++ ...irectory_walker_entry_point_finder_spec.ts | 6 +- 3 files changed, 125 insertions(+), 106 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts b/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts index a63cadf270..4ebecb69ae 100644 --- a/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts +++ b/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); - if (entryPoints === null) { - 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); - } + const entryPoints = this.entryPointManifest.readEntryPointsUsingManifest(basePath) || + this.walkBasePathForPackages(basePath); unsortedEntryPoints.push(...entryPoints); } return this.resolver.sortEntryPointsByDependency(unsortedEntryPoints); } /** - * Look for entry points 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. + * Search the `basePath` for possible Angular packages and entry-points. + * + * @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[] { - const entryPoints = this.getEntryPointsForPackage(sourceDirectory); - if (entryPoints === null) { + walkBasePathForPackages(basePath: AbsoluteFsPath): EntryPoint[] { + this.logger.debug( + `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) { - // The `sourceDirectory` is an entry point itself so no need to search its sub-directories. - // Also check for any nested node_modules in this package but only if it was compiled by - // Angular. - // It is unlikely that a non Angular entry point has a dependency on an Angular library. + const entryPoints: EntryPoint[] = []; + if (primaryEntryPoint !== NO_ENTRY_POINT) { + entryPoints.push(primaryEntryPoint); + this.collectSecondaryEntryPoints( + 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 - .readdir(sourceDirectory) - // Not interested in hidden files - .filter(p => !p.startsWith('.')) - // Ignore node_modules - .filter(p => p !== 'node_modules' && p !== NGCC_DIRECTORY) - // Only interested in directories (and only those that are not symlinks) - .filter(p => { - const stat = this.fs.lstat(resolve(sourceDirectory, p)); - return stat.isDirectory() && !stat.isSymbolicLink(); - }) - .forEach(p => { - // Package is a potential namespace containing packages (e.g `@angular`). - const packagePath = join(sourceDirectory, p); - entryPoints.push(...this.walkDirectoryForEntryPoints(packagePath)); - }); + // The `sourceDirectory` was not a package (i.e. there was no package.json) + // So search its sub-directories for Angular packages and entry-points + for (const path of this.fs.readdir(sourceDirectory)) { + if (isIgnorablePath(path)) { + // Ignore hidden files, node_modules and ngcc directory + continue; + } + + const absolutePath = this.fs.resolve(sourceDirectory, path); + const stat = this.fs.lstat(absolutePath); + if (stat.isSymbolicLink() || !stat.isDirectory()) { + // Ignore symbolic links and non-directories + continue; + } + + entryPoints.push(...this.walkDirectoryForPackages(this.fs.join(sourceDirectory, path))); + } + return entryPoints; } /** - * Recurse the folder structure looking for all the entry points - * @param packagePath The absolute path to an npm package that may contain entry points - * @returns An array of entry points that were discovered or null when it's not a valid entrypoint + * Search the `directory` looking for any secondary entry-points for a package, adding any that + * are found to the `entryPoints` array. + * + * @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 { - const entryPoints: EntryPoint[] = []; - - // Try to get an entry point from the top level package directory - const topLevelEntryPoint = - getEntryPointInfo(this.fs, this.config, this.logger, packagePath, packagePath); - - // 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; + private collectSecondaryEntryPoints( + entryPoints: EntryPoint[], packagePath: AbsoluteFsPath, directory: AbsoluteFsPath, + paths: PathSegment[]): void { + for (const path of paths) { + if (isIgnorablePath(path)) { + // Ignore hidden files, node_modules and ngcc directory + continue; } - // If the path is a JS file then strip its extension and see if we can match an entry-point. - const possibleEntryPointPath = isDirectory ? path : stripJsExtension(path); + const absolutePath = this.fs.resolve(directory, 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) { - return false; + if (subEntryPoint !== NO_ENTRY_POINT && subEntryPoint !== INCOMPATIBLE_ENTRY_POINT) { + 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; + } - /** - * Recursively walk a directory and its sub-directories, applying a given - * 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); - } - }); + const childPaths = this.fs.readdir(absolutePath); + this.collectSecondaryEntryPoints(entryPoints, packagePath, absolutePath, childPaths); + } } } function stripJsExtension(filePath: T): T { return filePath.replace(/\.js$/, '') as T; } + +function isIgnorablePath(path: PathSegment): boolean { + return path.startsWith('.') || path === 'node_modules' || path === NGCC_DIRECTORY; +} diff --git a/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts b/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts index 2f8329eae4..c81e5cc01f 100644 --- a/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts +++ b/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( + task: () => T extends Promise? 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; +} diff --git a/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts b/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts index 386da54f37..bfd297dfdd 100644 --- a/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts +++ b/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)],