diff --git a/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts index 3188d7f9b9..022fe4a284 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts @@ -9,39 +9,22 @@ import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../../src/ngtsc/file_system'; import {isRequireCall, isWildcardReexportStatement, RequireCall} from '../host/commonjs_umd_utils'; +import {isAssignment, isAssignmentStatement} from '../host/esm2015_host'; import {DependencyHostBase} from './dependency_host'; -import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; /** * Helper functions for computing dependencies. */ export class CommonJsDependencyHost extends DependencyHostBase { - /** - * Compute the dependencies of the given file. - * - * @param file An absolute path to the file whose dependencies we want to get. - * @param dependencies A set that will have the absolute paths of resolved entry points added to - * it. - * @param missing A set that will have the dependencies that could not be found added to it. - * @param deepImports A set that will have the import paths that exist but cannot be mapped to - * entry-points, i.e. deep-imports. - * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck - * in a circular dependency loop. - */ - protected recursivelyCollectDependencies( - file: AbsoluteFsPath, dependencies: Set, missing: Set, - deepImports: Set, alreadySeen: Set): void { - const fromContents = this.fs.readFile(file); - - if (!this.hasRequireCalls(fromContents)) { - // Avoid parsing the source file as there are no imports. - return; - } + protected canSkipFile(fileContents: string): boolean { + return !hasRequireCalls(fileContents); + } + protected extractImports(file: AbsoluteFsPath, fileContents: string): Set { // Parse the source into a TypeScript AST and then walk it looking for imports and re-exports. const sf = - ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); + ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); const requireCalls: RequireCall[] = []; for (const stmt of sf.statements) { @@ -92,37 +75,20 @@ export class CommonJsDependencyHost extends DependencyHostBase { } } - const importPaths = new Set(requireCalls.map(call => call.arguments[0].text)); - for (const importPath of importPaths) { - const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, file); - if (resolvedModule === null) { - missing.add(importPath); - } else if (resolvedModule instanceof ResolvedRelativeModule) { - const internalDependency = resolvedModule.modulePath; - if (!alreadySeen.has(internalDependency)) { - alreadySeen.add(internalDependency); - this.recursivelyCollectDependencies( - internalDependency, dependencies, missing, deepImports, alreadySeen); - } - } else if (resolvedModule instanceof ResolvedDeepImport) { - deepImports.add(resolvedModule.importPath); - } else { - dependencies.add(resolvedModule.entryPointPath); - } - } - } - - /** - * Check whether a source file needs to be parsed for imports. - * This is a performance short-circuit, which saves us from creating - * a TypeScript AST unnecessarily. - * - * @param source The content of the source file to check. - * - * @returns false if there are definitely no require calls - * in this file, true otherwise. - */ - private hasRequireCalls(source: string): boolean { - return /require\(['"]/.test(source); + return new Set(requireCalls.map(call => call.arguments[0].text)); } } + +/** + * Check whether a source file needs to be parsed for imports. + * This is a performance short-circuit, which saves us from creating + * a TypeScript AST unnecessarily. + * + * @param source The content of the source file to check. + * + * @returns false if there are definitely no require calls + * in this file, true otherwise. + */ +export function hasRequireCalls(source: string): boolean { + return /require\(['"]/.test(source); +} diff --git a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts index 9b175ade0b..e251973e0e 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts @@ -9,7 +9,7 @@ import {AbsoluteFsPath, FileSystem, PathSegment} from '../../../src/ngtsc/file_s import {EntryPoint} from '../packages/entry_point'; import {resolveFileWithPostfixes} from '../utils'; -import {ModuleResolver} from './module_resolver'; +import {ModuleResolver, ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; export interface DependencyHost { collectDependencies( @@ -65,7 +65,54 @@ export abstract class DependencyHostBase implements DependencyHost { * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck * in a circular dependency loop. */ - protected abstract recursivelyCollectDependencies( + protected recursivelyCollectDependencies( file: AbsoluteFsPath, dependencies: Set, missing: Set, - deepImports: Set, alreadySeen: Set): void; + deepImports: Set, alreadySeen: Set): void { + const fromContents = this.fs.readFile(file); + if (this.canSkipFile(fromContents)) { + return; + } + const imports = this.extractImports(file, fromContents); + for (const importPath of imports) { + const resolved = + this.processImport(importPath, file, dependencies, missing, deepImports, alreadySeen); + if (!resolved) { + missing.add(importPath); + } + } + } + + protected abstract canSkipFile(fileContents: string): boolean; + protected abstract extractImports(file: AbsoluteFsPath, fileContents: string): Set; + + /** + * Resolve the given `importPath` from `file` and add it to the appropriate set. + * + * If the import is local to this package then follow it by calling + * `recursivelyCollectDependencies()`. + * + * @returns `true` if the import was resolved (to an entry-point, a local import, or a + * deep-import), `false` otherwise. + */ + protected processImport( + importPath: string, file: AbsoluteFsPath, dependencies: Set, + missing: Set, deepImports: Set, alreadySeen: Set): boolean { + const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, file); + if (resolvedModule === null) { + return false; + } + if (resolvedModule instanceof ResolvedRelativeModule) { + const internalDependency = resolvedModule.modulePath; + if (!alreadySeen.has(internalDependency)) { + alreadySeen.add(internalDependency); + this.recursivelyCollectDependencies( + internalDependency, dependencies, missing, deepImports, alreadySeen); + } + } else if (resolvedModule instanceof ResolvedDeepImport) { + deepImports.add(resolvedModule.importPath); + } else { + dependencies.add(resolvedModule.entryPointPath); + } + return true; + } } diff --git a/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts index bbb4ef5156..71e8aa2923 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts @@ -8,77 +8,25 @@ import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../../src/ngtsc/file_system'; import {DependencyHostBase} from './dependency_host'; -import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; /** * Helper functions for computing dependencies. */ export class EsmDependencyHost extends DependencyHostBase { - /** - * Compute the dependencies of the given file. - * - * @param file An absolute path to the file whose dependencies we want to get. - * @param dependencies A set that will have the absolute paths of resolved entry points added to - * it. - * @param missing A set that will have the dependencies that could not be found added to it. - * @param deepImports A set that will have the import paths that exist but cannot be mapped to - * entry-points, i.e. deep-imports. - * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck - * in a circular dependency loop. - */ - protected recursivelyCollectDependencies( - file: AbsoluteFsPath, dependencies: Set, missing: Set, - deepImports: Set, alreadySeen: Set): void { - const fromContents = this.fs.readFile(file); - - if (!hasImportOrReexportStatements(fromContents)) { - // Avoid parsing the source file as there are no imports. - return; - } - - // Parse the source into a TypeScript AST and then walk it looking for imports and re-exports. - const sf = - ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); - sf.statements - // filter out statements that are not imports or reexports - .filter(isStringImportOrReexport) - // Grab the id of the module that is being imported - .map(stmt => stmt.moduleSpecifier.text) - .forEach(importPath => { - const resolved = - this.processImport(importPath, file, dependencies, missing, deepImports, alreadySeen); - if (!resolved) { - missing.add(importPath); - } - }); + protected canSkipFile(fileContents: string): boolean { + return !hasImportOrReexportStatements(fileContents); } - /** - * Resolve the given `importPath` from `file` and add it to the appropriate set. - * - * @returns `true` if the import was resolved (to an entry-point, a local import, or a - * deep-import). - */ - protected processImport( - importPath: string, file: AbsoluteFsPath, dependencies: Set, - missing: Set, deepImports: Set, alreadySeen: Set): boolean { - const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, file); - if (resolvedModule === null) { - return false; - } - if (resolvedModule instanceof ResolvedRelativeModule) { - const internalDependency = resolvedModule.modulePath; - if (!alreadySeen.has(internalDependency)) { - alreadySeen.add(internalDependency); - this.recursivelyCollectDependencies( - internalDependency, dependencies, missing, deepImports, alreadySeen); - } - } else if (resolvedModule instanceof ResolvedDeepImport) { - deepImports.add(resolvedModule.importPath); - } else { - dependencies.add(resolvedModule.entryPointPath); - } - return true; + protected extractImports(file: AbsoluteFsPath, fileContents: string): Set { + const imports: string[] = []; + // Parse the source into a TypeScript AST and then walk it looking for imports and re-exports. + const sf = + ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); + return new Set(sf.statements + // filter out statements that are not imports or reexports + .filter(isStringImportOrReexport) + // Grab the id of the module that is being imported + .map(stmt => stmt.moduleSpecifier.text)); } } diff --git a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts index 38700bbc4b..d3d97baf85 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts @@ -6,85 +6,36 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; + import {AbsoluteFsPath} from '../../../src/ngtsc/file_system'; import {getImportsOfUmdModule, parseStatementForUmdModule} from '../host/umd_host'; + +import {hasRequireCalls} from './commonjs_dependency_host'; import {DependencyHostBase} from './dependency_host'; -import {ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; /** * Helper functions for computing dependencies. */ export class UmdDependencyHost extends DependencyHostBase { - /** - * Compute the dependencies of the given file. - * - * @param file An absolute path to the file whose dependencies we want to get. - * @param dependencies A set that will have the absolute paths of resolved entry points added to - * it. - * @param missing A set that will have the dependencies that could not be found added to it. - * @param deepImports A set that will have the import paths that exist but cannot be mapped to - * entry-points, i.e. deep-imports. - * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck - * in a circular dependency loop. - */ - protected recursivelyCollectDependencies( - file: AbsoluteFsPath, dependencies: Set, missing: Set, - deepImports: Set, alreadySeen: Set): void { - const fromContents = this.fs.readFile(file); - - if (!this.hasRequireCalls(fromContents)) { - // Avoid parsing the source file as there are no imports. - return; - } + protected canSkipFile(fileContents: string): boolean { + return !hasRequireCalls(fileContents); + } + protected extractImports(file: AbsoluteFsPath, fileContents: string): Set { // Parse the source into a TypeScript AST and then walk it looking for imports and re-exports. const sf = - ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); + ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS); if (sf.statements.length !== 1) { - return; + return new Set(); } const umdModule = parseStatementForUmdModule(sf.statements[0]); const umdImports = umdModule && getImportsOfUmdModule(umdModule); if (umdImports === null) { - return; + return new Set(); } - umdImports.forEach(umdImport => { - const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, file); - if (resolvedModule) { - if (resolvedModule instanceof ResolvedRelativeModule) { - const internalDependency = resolvedModule.modulePath; - if (!alreadySeen.has(internalDependency)) { - alreadySeen.add(internalDependency); - this.recursivelyCollectDependencies( - internalDependency, dependencies, missing, deepImports, alreadySeen); - } - } else { - if (resolvedModule instanceof ResolvedDeepImport) { - deepImports.add(resolvedModule.importPath); - } else { - dependencies.add(resolvedModule.entryPointPath); - } - } - } else { - missing.add(umdImport.path); - } - }); - } - - /** - * Check whether a source file needs to be parsed for imports. - * This is a performance short-circuit, which saves us from creating - * a TypeScript AST unnecessarily. - * - * @param source The content of the source file to check. - * - * @returns false if there are definitely no require calls - * in this file, true otherwise. - */ - private hasRequireCalls(source: string): boolean { - return /require\(['"]/.test(source); + return new Set(umdImports.map(i => i.path)); } } diff --git a/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts index 1932167c07..f20b5036d1 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts @@ -27,12 +27,12 @@ runInEachFileSystem(() => { }); describe('collectDependencies()', () => { - it('should not generate a TS AST if the source does not contain any imports or re-exports', + it('should not try to extract import paths if the source does not contain any imports or re-exports', () => { - spyOn(ts, 'createSourceFile'); + const extractImportsSpy = spyOn(host as any, 'extractImports'); host.collectDependencies( _('/no/imports/or/re-exports/index.js'), createDependencyInfo()); - expect(ts.createSourceFile).not.toHaveBeenCalled(); + expect(extractImportsSpy).not.toHaveBeenCalled(); }); it('should resolve all the external imports of the source file', () => {