From 1bdec3ed6a2dee25530d8957104fa49a0fb5e94b Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 17 May 2019 20:35:16 +0100 Subject: [PATCH] feat(ivy): ngcc - implement CommonJsDependencyHost (#30200) PR Close #30200 --- .../dependencies/commonjs_dependency_host.ts | 107 ++++++++++ .../ngcc/src/dependencies/dependency_host.ts | 6 +- .../src/dependencies/esm_dependency_host.ts | 6 +- .../src/dependencies/umd_dependency_host.ts | 6 +- .../commonjs_dependency_host_spec.ts | 185 ++++++++++++++++++ .../dependencies/esm_dependency_host_spec.ts | 4 +- .../dependencies/umd_dependency_host_spec.ts | 4 +- 7 files changed, 305 insertions(+), 13 deletions(-) create mode 100644 packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts create mode 100644 packages/compiler-cli/ngcc/test/dependencies/commonjs_dependency_host_spec.ts diff --git a/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts new file mode 100644 index 0000000000..352844d282 --- /dev/null +++ b/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts @@ -0,0 +1,107 @@ +/** + * @license + * Copyright Google Inc. 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 + */ +import * as ts from 'typescript'; + +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; +import {FileSystem} from '../file_system/file_system'; +import {isRequireCall} from '../host/commonjs_host'; + +import {DependencyHost, DependencyInfo} from './dependency_host'; +import {ModuleResolver, ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; + +/** + * Helper functions for computing dependencies. + */ +export class CommonJsDependencyHost implements DependencyHost { + constructor(private fs: FileSystem, private moduleResolver: ModuleResolver) {} + + /** + * Find all the dependencies for the entry-point at the given path. + * + * @param entryPointPath The absolute path to the JavaScript file that represents an entry-point. + * @returns Information about the dependencies of the entry-point, including those that were + * missing or deep imports into other entry-points. + */ + findDependencies(entryPointPath: AbsoluteFsPath): DependencyInfo { + const dependencies = new Set(); + const missing = new Set(); + const deepImports = new Set(); + const alreadySeen = new Set(); + this.recursivelyFindDependencies( + entryPointPath, dependencies, missing, deepImports, alreadySeen); + return {dependencies, missing, deepImports}; + } + + /** + * 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. + */ + private recursivelyFindDependencies( + 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 require calls. + 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); + + for (const statement of sf.statements) { + const declarations = + ts.isVariableStatement(statement) ? statement.declarationList.declarations : []; + for (const declaration of declarations) { + if (declaration.initializer && isRequireCall(declaration.initializer)) { + const importPath = declaration.initializer.arguments[0].text; + const resolvedModule = this.moduleResolver.resolveModuleImport(importPath, file); + if (resolvedModule) { + if (resolvedModule instanceof ResolvedRelativeModule) { + const internalDependency = resolvedModule.modulePath; + if (!alreadySeen.has(internalDependency)) { + alreadySeen.add(internalDependency); + this.recursivelyFindDependencies( + internalDependency, dependencies, missing, deepImports, alreadySeen); + } + } else { + if (resolvedModule instanceof ResolvedDeepImport) { + deepImports.add(resolvedModule.importPath); + } else { + dependencies.add(resolvedModule.entryPointPath); + } + } + } else { + missing.add(importPath); + } + } + } + } + } + + /** + * 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. + */ + 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 b2210eb39c..c2a6924c6f 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AbsoluteFsPath} from '../../../src/ngtsc/path'; +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; export interface DependencyHost { findDependencies(entryPointPath: AbsoluteFsPath): DependencyInfo; @@ -14,6 +14,6 @@ export interface DependencyHost { export interface DependencyInfo { dependencies: Set; - missing: Set; - deepImports: Set; + missing: Set; + deepImports: Set; } 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 33c7630698..0b164fac20 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts @@ -7,7 +7,7 @@ */ import * as ts from 'typescript'; -import {AbsoluteFsPath} from '../../../src/ngtsc/path'; +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; import {FileSystem} from '../file_system/file_system'; import {DependencyHost, DependencyInfo} from './dependency_host'; import {ModuleResolver, ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver'; @@ -28,8 +28,8 @@ export class EsmDependencyHost implements DependencyHost { */ findDependencies(entryPointPath: AbsoluteFsPath): DependencyInfo { const dependencies = new Set(); - const missing = new Set(); - const deepImports = new Set(); + const missing = new Set(); + const deepImports = new Set(); const alreadySeen = new Set(); this.recursivelyFindDependencies( entryPointPath, dependencies, missing, deepImports, alreadySeen); 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 f8ad0ae9d6..d00790a494 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts @@ -7,7 +7,7 @@ */ import * as ts from 'typescript'; -import {AbsoluteFsPath} from '../../../src/ngtsc/path'; +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; import {FileSystem} from '../file_system/file_system'; import {getImportsOfUmdModule, parseStatementForUmdModule} from '../host/umd_host'; @@ -31,8 +31,8 @@ export class UmdDependencyHost implements DependencyHost { */ findDependencies(entryPointPath: AbsoluteFsPath): DependencyInfo { const dependencies = new Set(); - const missing = new Set(); - const deepImports = new Set(); + const missing = new Set(); + const deepImports = new Set(); const alreadySeen = new Set(); this.recursivelyFindDependencies( entryPointPath, dependencies, missing, deepImports, alreadySeen); diff --git a/packages/compiler-cli/ngcc/test/dependencies/commonjs_dependency_host_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/commonjs_dependency_host_spec.ts new file mode 100644 index 0000000000..951fec0ef6 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/dependencies/commonjs_dependency_host_spec.ts @@ -0,0 +1,185 @@ +/** + * @license + * Copyright Google Inc. 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 + */ +import * as ts from 'typescript'; + +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; +import {CommonJsDependencyHost} from '../../src/dependencies/commonjs_dependency_host'; +import {ModuleResolver} from '../../src/dependencies/module_resolver'; +import {MockFileSystem} from '../helpers/mock_file_system'; + +const _ = AbsoluteFsPath.from; + +describe('CommonJsDependencyHost', () => { + let host: CommonJsDependencyHost; + beforeEach(() => { + const fs = createMockFileSystem(); + host = new CommonJsDependencyHost(fs, new ModuleResolver(fs)); + }); + + describe('getDependencies()', () => { + it('should not generate a TS AST if the source does not contain any require calls', () => { + spyOn(ts, 'createSourceFile'); + host.findDependencies(_('/no/imports/or/re-exports/index.js')); + expect(ts.createSourceFile).not.toHaveBeenCalled(); + }); + + it('should resolve all the external imports of the source file', () => { + const {dependencies, missing, deepImports} = + host.findDependencies(_('/external/imports/index.js')); + expect(dependencies.size).toBe(2); + expect(missing.size).toBe(0); + expect(deepImports.size).toBe(0); + expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); + expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); + }); + + it('should resolve all the external re-exports of the source file', () => { + const {dependencies, missing, deepImports} = + host.findDependencies(_('/external/re-exports/index.js')); + expect(dependencies.size).toBe(2); + expect(missing.size).toBe(0); + expect(deepImports.size).toBe(0); + expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); + expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); + }); + + it('should capture missing external imports', () => { + const {dependencies, missing, deepImports} = + host.findDependencies(_('/external/imports-missing/index.js')); + + expect(dependencies.size).toBe(1); + expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); + expect(missing.size).toBe(1); + expect(missing.has(PathSegment.fromFsPath('missing'))).toBe(true); + expect(deepImports.size).toBe(0); + }); + + it('should not register deep imports as missing', () => { + // This scenario verifies the behavior of the dependency analysis when an external import + // is found that does not map to an entry-point but still exists on disk, i.e. a deep import. + // Such deep imports are captured for diagnostics purposes. + const {dependencies, missing, deepImports} = + host.findDependencies(_('/external/deep-import/index.js')); + + expect(dependencies.size).toBe(0); + expect(missing.size).toBe(0); + expect(deepImports.size).toBe(1); + expect(deepImports.has(_('/node_modules/lib_1/deep/import'))).toBe(true); + }); + + it('should recurse into internal dependencies', () => { + const {dependencies, missing, deepImports} = + host.findDependencies(_('/internal/outer/index.js')); + + expect(dependencies.size).toBe(1); + expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); + expect(missing.size).toBe(0); + expect(deepImports.size).toBe(0); + }); + + it('should handle circular internal dependencies', () => { + const {dependencies, missing, deepImports} = + host.findDependencies(_('/internal/circular_a/index.js')); + expect(dependencies.size).toBe(2); + expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); + expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); + expect(missing.size).toBe(0); + expect(deepImports.size).toBe(0); + }); + + it('should support `paths` alias mappings when resolving modules', () => { + const fs = createMockFileSystem(); + host = new CommonJsDependencyHost(fs, new ModuleResolver(fs, { + baseUrl: '/dist', + paths: { + '@app/*': ['*'], + '@lib/*/test': ['lib/*/test'], + } + })); + const {dependencies, missing, deepImports} = host.findDependencies(_('/path-alias/index.js')); + expect(dependencies.size).toBe(4); + expect(dependencies.has(_('/dist/components'))).toBe(true); + expect(dependencies.has(_('/dist/shared'))).toBe(true); + expect(dependencies.has(_('/dist/lib/shared/test'))).toBe(true); + expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); + expect(missing.size).toBe(0); + expect(deepImports.size).toBe(0); + }); + }); + + function createMockFileSystem() { + return new MockFileSystem({ + '/no/imports/or/re-exports/index.js': '// some text but no import-like statements', + '/no/imports/or/re-exports/package.json': '{"esm2015": "./index.js"}', + '/no/imports/or/re-exports/index.metadata.json': 'MOCK METADATA', + '/external/imports/index.js': commonJs(['lib_1', 'lib_1/sub_1']), + '/external/imports/package.json': '{"esm2015": "./index.js"}', + '/external/imports/index.metadata.json': 'MOCK METADATA', + '/external/re-exports/index.js': + commonJs(['lib_1', 'lib_1/sub_1'], ['lib_1.X', 'lib_1sub_1.Y']), + '/external/re-exports/package.json': '{"esm2015": "./index.js"}', + '/external/re-exports/index.metadata.json': 'MOCK METADATA', + '/external/imports-missing/index.js': commonJs(['lib_1', 'missing']), + '/external/imports-missing/package.json': '{"esm2015": "./index.js"}', + '/external/imports-missing/index.metadata.json': 'MOCK METADATA', + '/external/deep-import/index.js': commonJs(['lib_1/deep/import']), + '/external/deep-import/package.json': '{"esm2015": "./index.js"}', + '/external/deep-import/index.metadata.json': 'MOCK METADATA', + '/internal/outer/index.js': commonJs(['../inner']), + '/internal/outer/package.json': '{"esm2015": "./index.js"}', + '/internal/outer/index.metadata.json': 'MOCK METADATA', + '/internal/inner/index.js': commonJs(['lib_1/sub_1'], ['X']), + '/internal/circular_a/index.js': commonJs(['../circular_b', 'lib_1/sub_1'], ['Y']), + '/internal/circular_b/index.js': commonJs(['../circular_a', 'lib_1'], ['X']), + '/internal/circular_a/package.json': '{"esm2015": "./index.js"}', + '/internal/circular_a/index.metadata.json': 'MOCK METADATA', + '/re-directed/index.js': commonJs(['lib_1/sub_2']), + '/re-directed/package.json': '{"esm2015": "./index.js"}', + '/re-directed/index.metadata.json': 'MOCK METADATA', + '/path-alias/index.js': + commonJs(['@app/components', '@app/shared', '@lib/shared/test', 'lib_1']), + '/path-alias/package.json': '{"esm2015": "./index.js"}', + '/path-alias/index.metadata.json': 'MOCK METADATA', + '/node_modules/lib_1/index.d.ts': 'export declare class X {}', + '/node_modules/lib_1/package.json': '{"esm2015": "./index.js", "typings": "./index.d.ts"}', + '/node_modules/lib_1/index.metadata.json': 'MOCK METADATA', + '/node_modules/lib_1/deep/import/index.js': 'export class DeepImport {}', + '/node_modules/lib_1/sub_1/index.d.ts': 'export declare class Y {}', + '/node_modules/lib_1/sub_1/package.json': + '{"esm2015": "./index.js", "typings": "./index.d.ts"}', + '/node_modules/lib_1/sub_1/index.metadata.json': 'MOCK METADATA', + '/node_modules/lib_1/sub_2.d.ts': `export * from './sub_2/sub_2';`, + '/node_modules/lib_1/sub_2/sub_2.d.ts': `export declare class Z {}';`, + '/node_modules/lib_1/sub_2/package.json': + '{"esm2015": "./sub_2.js", "typings": "./sub_2.d.ts"}', + '/node_modules/lib_1/sub_2/sub_2.metadata.json': 'MOCK METADATA', + '/dist/components/index.d.ts': `export declare class MyComponent {};`, + '/dist/components/package.json': '{"esm2015": "./index.js", "typings": "./index.d.ts"}', + '/dist/components/index.metadata.json': 'MOCK METADATA', + '/dist/shared/index.d.ts': `import {X} from 'lib_1';\nexport declare class Service {}`, + '/dist/shared/package.json': '{"esm2015": "./index.js", "typings": "./index.d.ts"}', + '/dist/shared/index.metadata.json': 'MOCK METADATA', + '/dist/lib/shared/test/index.d.ts': `export class TestHelper {}`, + '/dist/lib/shared/test/package.json': '{"esm2015": "./index.js", "typings": "./index.d.ts"}', + '/dist/lib/shared/test/index.metadata.json': 'MOCK METADATA', + }); + } +}); + +function commonJs(importPaths: string[], exportNames: string[] = []) { + const commonJsRequires = + importPaths + .map( + p => + `var ${p.replace('@angular/', '').replace(/\.?\.?\//g, '').replace(/@/,'')} = require('${p}');`) + .join('\n'); + const exportStatements = + exportNames.map(e => ` exports.${e.replace(/.+\./, '')} = ${e};`).join('\n'); + return `${commonJsRequires} +${exportStatements}`; +} 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 69ec54d5a5..6298ba3f2c 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 @@ -7,7 +7,7 @@ */ import * as ts from 'typescript'; -import {AbsoluteFsPath} from '../../../src/ngtsc/path'; +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host'; import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {MockFileSystem} from '../helpers/mock_file_system'; @@ -56,7 +56,7 @@ describe('EsmDependencyHost', () => { expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib-1'))).toBe(true); expect(missing.size).toBe(1); - expect(missing.has('missing')).toBe(true); + expect(missing.has(PathSegment.fromFsPath('missing'))).toBe(true); expect(deepImports.size).toBe(0); }); diff --git a/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts index 5f348c4566..521af669b8 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts @@ -7,7 +7,7 @@ */ import * as ts from 'typescript'; -import {AbsoluteFsPath} from '../../../src/ngtsc/path'; +import {AbsoluteFsPath, PathSegment} from '../../../src/ngtsc/path'; import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {UmdDependencyHost} from '../../src/dependencies/umd_dependency_host'; import {MockFileSystem} from '../helpers/mock_file_system'; @@ -55,7 +55,7 @@ describe('UmdDependencyHost', () => { expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); expect(missing.size).toBe(1); - expect(missing.has('missing')).toBe(true); + expect(missing.has(PathSegment.fromFsPath('missing'))).toBe(true); expect(deepImports.size).toBe(0); });