From e2b184515b12e6285968a8c25e4238422c0e15b9 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 19 Dec 2019 22:43:12 +0000 Subject: [PATCH] refactor(ngcc): pass dependency info to `collectDependencies()` (#34494) Rather than return a new object of dependency info from calls to `collectDependencies()` we now pass in an object that will be updated with the dependency info. This is in preparation of a change where we will collect dependency information from more than one `DependencyHost`. Also to better fit with this approach the name is changed from `findDependencies()` to `collectDependencies()`. PR Close #34494 --- .../dependencies/commonjs_dependency_host.ts | 4 +- .../ngcc/src/dependencies/dependency_host.ts | 25 ++-- .../src/dependencies/dependency_resolver.ts | 6 +- .../src/dependencies/esm_dependency_host.ts | 4 +- .../src/dependencies/umd_dependency_host.ts | 4 +- .../commonjs_dependency_host_spec.ts | 42 +++--- .../dependencies/dependency_resolver_spec.ts | 122 ++++++++++-------- .../dependencies/esm_dependency_host_spec.ts | 43 +++--- .../dependencies/umd_dependency_host_spec.ts | 42 +++--- 9 files changed, 164 insertions(+), 128 deletions(-) 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 01e9d55e9c..5bab3a7bc4 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/commonjs_dependency_host.ts @@ -27,7 +27,7 @@ export class CommonJsDependencyHost extends DependencyHostBase { * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck * in a circular dependency loop. */ - protected recursivelyFindDependencies( + protected recursivelyCollectDependencies( file: AbsoluteFsPath, dependencies: Set, missing: Set, deepImports: Set, alreadySeen: Set): void { const fromContents = this.fs.readFile(file); @@ -53,7 +53,7 @@ export class CommonJsDependencyHost extends DependencyHostBase { const internalDependency = resolvedModule.modulePath; if (!alreadySeen.has(internalDependency)) { alreadySeen.add(internalDependency); - this.recursivelyFindDependencies( + this.recursivelyCollectDependencies( internalDependency, dependencies, missing, deepImports, alreadySeen); } } else { diff --git a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts index 900b44a992..c5fbf25d64 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts @@ -11,7 +11,8 @@ import {resolveFileWithPostfixes} from '../utils'; import {ModuleResolver} from './module_resolver'; export interface DependencyHost { - findDependencies(entryPointPath: AbsoluteFsPath): DependencyInfo; + collectDependencies( + entryPointPath: AbsoluteFsPath, {dependencies, missing, deepImports}: DependencyInfo): void; } export interface DependencyInfo { @@ -20,6 +21,10 @@ export interface DependencyInfo { deepImports: Set; } +export function createDependencyInfo(): DependencyInfo { + return {dependencies: new Set(), missing: new Set(), deepImports: new Set()}; +} + export abstract class DependencyHostBase implements DependencyHost { constructor(protected fs: FileSystem, protected moduleResolver: ModuleResolver) {} @@ -27,23 +32,19 @@ export abstract class DependencyHostBase implements DependencyHost { * 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. + * @param dependencyInfo An object containing information about the dependencies of the + * entry-point, including those that were missing or deep imports into other entry-points. The + * sets in this object will be updated with new information about the entry-point's dependencies. */ - findDependencies(entryPointPath: AbsoluteFsPath): DependencyInfo { - const dependencies = new Set(); - const missing = new Set(); - const deepImports = new Set(); - + collectDependencies( + entryPointPath: AbsoluteFsPath, {dependencies, missing, deepImports}: DependencyInfo): void { const resolvedFile = resolveFileWithPostfixes(this.fs, entryPointPath, ['', '.js', '/index.js']); if (resolvedFile !== null) { const alreadySeen = new Set(); - this.recursivelyFindDependencies( + this.recursivelyCollectDependencies( resolvedFile, dependencies, missing, deepImports, alreadySeen); } - - return {dependencies, missing, deepImports}; } /** @@ -58,7 +59,7 @@ 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 recursivelyFindDependencies( + protected abstract recursivelyCollectDependencies( file: AbsoluteFsPath, dependencies: Set, missing: Set, deepImports: Set, alreadySeen: Set): void; } diff --git a/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts b/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts index d910dbfe88..ba622623f5 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts @@ -11,7 +11,7 @@ import {AbsoluteFsPath, FileSystem, resolve} from '../../../src/ngtsc/file_syste import {Logger} from '../logging/logger'; import {EntryPoint, EntryPointFormat, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from '../packages/entry_point'; import {PartiallyOrderedList} from '../utils'; -import {DependencyHost, DependencyInfo} from './dependency_host'; +import {DependencyHost, DependencyInfo, createDependencyInfo} from './dependency_host'; const builtinNodeJsModules = new Set(require('module').builtinModules); @@ -123,7 +123,9 @@ export class DependencyResolver { throw new Error( `Could not find a suitable format for computing dependencies of entry-point: '${entryPoint.path}'.`); } - return host.findDependencies(formatInfo.path); + const depInfo = createDependencyInfo(); + host.collectDependencies(formatInfo.path, depInfo); + return depInfo; } /** 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 2b6b394d85..6b2c1226d1 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts @@ -26,7 +26,7 @@ export class EsmDependencyHost extends DependencyHostBase { * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck * in a circular dependency loop. */ - protected recursivelyFindDependencies( + protected recursivelyCollectDependencies( file: AbsoluteFsPath, dependencies: Set, missing: Set, deepImports: Set, alreadySeen: Set): void { const fromContents = this.fs.readFile(file); @@ -52,7 +52,7 @@ export class EsmDependencyHost extends DependencyHostBase { const internalDependency = resolvedModule.modulePath; if (!alreadySeen.has(internalDependency)) { alreadySeen.add(internalDependency); - this.recursivelyFindDependencies( + this.recursivelyCollectDependencies( internalDependency, dependencies, missing, deepImports, alreadySeen); } } else { 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 0664dfce88..ab2eb677e5 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts @@ -27,7 +27,7 @@ export class UmdDependencyHost extends DependencyHostBase { * @param alreadySeen A set that is used to track internal dependencies to prevent getting stuck * in a circular dependency loop. */ - protected recursivelyFindDependencies( + protected recursivelyCollectDependencies( file: AbsoluteFsPath, dependencies: Set, missing: Set, deepImports: Set, alreadySeen: Set): void { const fromContents = this.fs.readFile(file); @@ -58,7 +58,7 @@ export class UmdDependencyHost extends DependencyHostBase { const internalDependency = resolvedModule.modulePath; if (!alreadySeen.has(internalDependency)) { alreadySeen.add(internalDependency); - this.recursivelyFindDependencies( + this.recursivelyCollectDependencies( internalDependency, dependencies, missing, deepImports, alreadySeen); } } else { 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 index d577dd3680..05eb30e462 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/commonjs_dependency_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/commonjs_dependency_host_spec.ts @@ -10,6 +10,7 @@ import {absoluteFrom, getFileSystem, relativeFrom} from '../../../src/ngtsc/file import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {CommonJsDependencyHost} from '../../src/dependencies/commonjs_dependency_host'; +import {createDependencyInfo} from '../../src/dependencies/dependency_host'; import {ModuleResolver} from '../../src/dependencies/module_resolver'; runInEachFileSystem(() => { @@ -116,13 +117,14 @@ runInEachFileSystem(() => { 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')); + host.collectDependencies(_('/no/imports/or/re-exports/index.js'), createDependencyInfo()); 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')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); @@ -131,8 +133,9 @@ runInEachFileSystem(() => { }); it('should resolve all the external re-exports of the source file', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/re-exports/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/re-exports/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); @@ -141,8 +144,9 @@ runInEachFileSystem(() => { }); it('should capture missing external imports', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports-missing/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports-missing/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); @@ -155,8 +159,9 @@ runInEachFileSystem(() => { // 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')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/deep-import/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(0); expect(missing.size).toBe(0); @@ -165,8 +170,9 @@ runInEachFileSystem(() => { }); it('should recurse into internal dependencies', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/internal/outer/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/internal/outer/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); @@ -175,8 +181,9 @@ runInEachFileSystem(() => { }); it('should handle circular internal dependencies', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/internal/circular_a/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/internal/circular_a/index.js'), {dependencies, missing, deepImports}); 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); @@ -193,8 +200,8 @@ runInEachFileSystem(() => { '@lib/*/test': ['lib/*/test'], } })); - const {dependencies, missing, deepImports} = - host.findDependencies(_('/path-alias/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies(_('/path-alias/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(4); expect(dependencies.has(_('/dist/components'))).toBe(true); expect(dependencies.has(_('/dist/shared'))).toBe(true); @@ -205,8 +212,9 @@ runInEachFileSystem(() => { }); it('should handle entry-point paths with no extension', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports/index')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports/index'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); diff --git a/packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts index f9fa7ff444..63242be203 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts @@ -8,8 +8,9 @@ import {DepGraph} from 'dependency-graph'; -import {FileSystem, absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system'; +import {FileSystem, absoluteFrom, getFileSystem, relativeFrom} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; +import {DependencyInfo} from '../../src/dependencies/dependency_host'; import {DependencyResolver, SortedEntryPointsInfo} from '../../src/dependencies/dependency_resolver'; import {EsmDependencyHost} from '../../src/dependencies/esm_dependency_host'; import {ModuleResolver} from '../../src/dependencies/module_resolver'; @@ -86,67 +87,69 @@ runInEachFileSystem(() => { } as EntryPoint; dependencies = { - [_('/first/index.js')]: {resolved: [second.path, third.path, '/ignored-1'], missing: []}, + [_('/first/index.js')]: + {resolved: [second.path, third.path, _('/ignored-1')], missing: []}, [_('/second/sub/index.js')]: {resolved: [third.path, fifth.path], missing: []}, - [_('/third/index.js')]: {resolved: [fourth.path, '/ignored-2'], missing: []}, + [_('/third/index.js')]: {resolved: [fourth.path, _('/ignored-2')], missing: []}, [_('/fourth/sub2/index.js')]: {resolved: [fifth.path], missing: []}, [_('/fifth/index.js')]: {resolved: [], missing: []}, }; }); it('should order the entry points by their dependency on each other', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies(dependencies)); + spyOn(host, 'collectDependencies') + .and.callFake(createFakeComputeDependencies(dependencies)); const result = resolver.sortEntryPointsByDependency([fifth, first, fourth, second, third]); expect(result.entryPoints).toEqual([fifth, fourth, third, second, first]); }); it('should remove entry-points that have missing direct dependencies', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ - [_('/first/index.js')]: {resolved: [], missing: ['/missing']}, + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ + [_('/first/index.js')]: {resolved: [], missing: [_('/missing')]}, [_('/second/sub/index.js')]: {resolved: [], missing: []}, })); const result = resolver.sortEntryPointsByDependency([first, second]); expect(result.entryPoints).toEqual([second]); expect(result.invalidEntryPoints).toEqual([ - {entryPoint: first, missingDependencies: ['/missing']}, + {entryPoint: first, missingDependencies: [_('/missing')]}, ]); }); it('should remove entry points that depended upon an invalid entry-point', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ [_('/first/index.js')]: {resolved: [second.path, third.path], missing: []}, - [_('/second/sub/index.js')]: {resolved: [], missing: ['/missing']}, + [_('/second/sub/index.js')]: {resolved: [], missing: [_('/missing')]}, [_('/third/index.js')]: {resolved: [], missing: []}, })); // Note that we will process `first` before `second`, which has the missing dependency. const result = resolver.sortEntryPointsByDependency([first, second, third]); expect(result.entryPoints).toEqual([third]); expect(result.invalidEntryPoints).toEqual([ - {entryPoint: second, missingDependencies: ['/missing']}, - {entryPoint: first, missingDependencies: ['/missing']}, + {entryPoint: second, missingDependencies: [_('/missing')]}, + {entryPoint: first, missingDependencies: [_('/missing')]}, ]); }); it('should remove entry points that will depend upon an invalid entry-point', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ [_('/first/index.js')]: {resolved: [second.path, third.path], missing: []}, - [_('/second/sub/index.js')]: {resolved: [], missing: ['/missing']}, + [_('/second/sub/index.js')]: {resolved: [], missing: [_('/missing')]}, [_('/third/index.js')]: {resolved: [], missing: []}, })); // Note that we will process `first` after `second`, which has the missing dependency. const result = resolver.sortEntryPointsByDependency([second, first, third]); expect(result.entryPoints).toEqual([third]); expect(result.invalidEntryPoints).toEqual([ - {entryPoint: second, missingDependencies: ['/missing']}, + {entryPoint: second, missingDependencies: [_('/missing')]}, {entryPoint: first, missingDependencies: [second.path]}, ]); }); it('should cope with entry points that will depend upon an invalid entry-point, when told to ignore missing dependencies', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ [_('/first/index.js')]: {resolved: [sixthIgnoreMissing.path], missing: []}, - [_('/sixth/index.js')]: {resolved: [], missing: ['/missing']}, + [_('/sixth/index.js')]: {resolved: [], missing: [_('/missing')]}, })); // Note that we will process `first` after `second`, which has the missing dependency. const result = resolver.sortEntryPointsByDependency([sixthIgnoreMissing, first]); @@ -155,8 +158,8 @@ runInEachFileSystem(() => { }); it('should not transitively ignore missing dependencies', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ - [_('/first/index.js')]: {resolved: [], missing: ['/missing']}, + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ + [_('/first/index.js')]: {resolved: [], missing: [_('/missing')]}, [_('/second/sub/index.js')]: {resolved: [first.path], missing: []}, [_('/sixth/index.js')]: {resolved: [second.path], missing: []}, })); @@ -167,16 +170,16 @@ runInEachFileSystem(() => { }); it('should cope with entry points having multiple indirect missing dependencies', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ - [_('/first/index.js')]: {resolved: [], missing: ['/missing1']}, - [_('/second/sub/index.js')]: {resolved: [], missing: ['/missing2']}, + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ + [_('/first/index.js')]: {resolved: [], missing: [_('/missing1')]}, + [_('/second/sub/index.js')]: {resolved: [], missing: [_('/missing2')]}, [_('/third/index.js')]: {resolved: [first.path, second.path], missing: []}, })); const result = resolver.sortEntryPointsByDependency([first, second, third]); expect(result.entryPoints).toEqual([]); expect(result.invalidEntryPoints).toEqual([ - {entryPoint: first, missingDependencies: ['/missing1']}, - {entryPoint: second, missingDependencies: ['/missing2']}, + {entryPoint: first, missingDependencies: [_('/missing1')]}, + {entryPoint: second, missingDependencies: [_('/missing2')]}, {entryPoint: third, missingDependencies: [first.path]}, ]); }); @@ -195,16 +198,18 @@ runInEachFileSystem(() => { }); it('should capture any dependencies that were ignored', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies(dependencies)); + spyOn(host, 'collectDependencies') + .and.callFake(createFakeComputeDependencies(dependencies)); const result = resolver.sortEntryPointsByDependency([fifth, first, fourth, second, third]); expect(result.ignoredDependencies).toEqual([ - {entryPoint: first, dependencyPath: '/ignored-1'}, - {entryPoint: third, dependencyPath: '/ignored-2'}, + {entryPoint: first, dependencyPath: _('/ignored-1')}, + {entryPoint: third, dependencyPath: _('/ignored-2')}, ]); }); it('should return the computed dependency graph', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies(dependencies)); + spyOn(host, 'collectDependencies') + .and.callFake(createFakeComputeDependencies(dependencies)); const result = resolver.sortEntryPointsByDependency([fifth, first, fourth, second, third]); expect(result.graph).toEqual(jasmine.any(DepGraph)); @@ -213,7 +218,8 @@ runInEachFileSystem(() => { }); it('should only return dependencies of the target, if provided', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies(dependencies)); + spyOn(host, 'collectDependencies') + .and.callFake(createFakeComputeDependencies(dependencies)); const entryPoints = [fifth, first, fourth, second, third]; let sorted: SortedEntryPointsInfo; @@ -230,8 +236,8 @@ runInEachFileSystem(() => { }); it('should not process the provided target if it has missing dependencies', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ - [_('/first/index.js')]: {resolved: [], missing: ['/missing']}, + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ + [_('/first/index.js')]: {resolved: [], missing: [_('/missing')]}, })); const entryPoints = [first]; let sorted: SortedEntryPointsInfo; @@ -239,11 +245,11 @@ runInEachFileSystem(() => { sorted = resolver.sortEntryPointsByDependency(entryPoints, first); expect(sorted.entryPoints).toEqual([]); expect(sorted.invalidEntryPoints[0].entryPoint).toEqual(first); - expect(sorted.invalidEntryPoints[0].missingDependencies).toEqual(['/missing']); + expect(sorted.invalidEntryPoints[0].missingDependencies).toEqual([_('/missing')]); }); it('should not consider builtin NodeJS modules as missing dependency', () => { - spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({ + spyOn(host, 'collectDependencies').and.callFake(createFakeComputeDependencies({ [_('/first/index.js')]: {resolved: [], missing: ['fs']}, })); const entryPoints = [first]; @@ -260,40 +266,42 @@ runInEachFileSystem(() => { const esm2015Host = new EsmDependencyHost(fs, moduleResolver); resolver = new DependencyResolver(fs, new MockLogger(), {esm5: esm5Host, esm2015: esm2015Host}); - spyOn(esm5Host, 'findDependencies') + spyOn(esm5Host, 'collectDependencies') .and.callFake(createFakeComputeDependencies(dependencies)); - spyOn(esm2015Host, 'findDependencies') + spyOn(esm2015Host, 'collectDependencies') .and.callFake(createFakeComputeDependencies(dependencies)); const result = resolver.sortEntryPointsByDependency([fifth, first, fourth, second, third]); expect(result.entryPoints).toEqual([fifth, fourth, third, second, first]); - expect(esm5Host.findDependencies).toHaveBeenCalledWith(fs.resolve(first.path, 'index.js')); - expect(esm5Host.findDependencies) - .not.toHaveBeenCalledWith(fs.resolve(second.path, 'sub/index.js')); - expect(esm5Host.findDependencies).toHaveBeenCalledWith(fs.resolve(third.path, 'index.js')); - expect(esm5Host.findDependencies) - .not.toHaveBeenCalledWith(fs.resolve(fourth.path, 'sub2/index.js')); - expect(esm5Host.findDependencies).toHaveBeenCalledWith(fs.resolve(fifth.path, 'index.js')); + expect(esm5Host.collectDependencies) + .toHaveBeenCalledWith(fs.resolve(first.path, 'index.js'), jasmine.any(Object)); + expect(esm5Host.collectDependencies) + .not.toHaveBeenCalledWith(fs.resolve(second.path, 'sub/index.js'), jasmine.any(Object)); + expect(esm5Host.collectDependencies) + .toHaveBeenCalledWith(fs.resolve(third.path, 'index.js'), jasmine.any(Object)); + expect(esm5Host.collectDependencies) + .not.toHaveBeenCalledWith( + fs.resolve(fourth.path, 'sub2/index.js'), jasmine.any(Object)); + expect(esm5Host.collectDependencies) + .toHaveBeenCalledWith(fs.resolve(fifth.path, 'index.js'), jasmine.any(Object)); - expect(esm2015Host.findDependencies) - .not.toHaveBeenCalledWith(fs.resolve(first.path, 'index.js')); - expect(esm2015Host.findDependencies) - .toHaveBeenCalledWith(fs.resolve(second.path, 'sub/index.js')); - expect(esm2015Host.findDependencies) - .not.toHaveBeenCalledWith(fs.resolve(third.path, 'index.js')); - expect(esm2015Host.findDependencies) - .toHaveBeenCalledWith(fs.resolve(fourth.path, 'sub2/index.js')); - expect(esm2015Host.findDependencies) - .not.toHaveBeenCalledWith(fs.resolve(fifth.path, 'index.js')); + expect(esm2015Host.collectDependencies) + .not.toHaveBeenCalledWith(fs.resolve(first.path, 'index.js'), jasmine.any(Object)); + expect(esm2015Host.collectDependencies) + .toHaveBeenCalledWith(fs.resolve(second.path, 'sub/index.js'), jasmine.any(Object)); + expect(esm2015Host.collectDependencies) + .not.toHaveBeenCalledWith(fs.resolve(third.path, 'index.js'), jasmine.any(Object)); + expect(esm2015Host.collectDependencies) + .toHaveBeenCalledWith(fs.resolve(fourth.path, 'sub2/index.js'), jasmine.any(Object)); + expect(esm2015Host.collectDependencies) + .not.toHaveBeenCalledWith(fs.resolve(fifth.path, 'index.js'), jasmine.any(Object)); }); function createFakeComputeDependencies(deps: DepMap) { - return (entryPoint: string) => { - const dependencies = new Set(); - const missing = new Set(); - const deepImports = new Set(); - deps[entryPoint].resolved.forEach(dep => dependencies.add(dep)); - deps[entryPoint].missing.forEach(dep => missing.add(dep)); + return (entryPointPath: string, {dependencies, missing, deepImports}: DependencyInfo) => { + deps[entryPointPath].resolved.forEach(dep => dependencies.add(absoluteFrom(dep))); + deps[entryPointPath].missing.forEach( + dep => missing.add(fs.isRooted(dep) ? absoluteFrom(dep) : relativeFrom(dep))); return {dependencies, missing, deepImports}; }; } 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 610132fc24..fe754b939e 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 @@ -10,6 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, relativeFrom} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; +import {createDependencyInfo} from '../../src/dependencies/dependency_host'; import {EsmDependencyHost, hasImportOrReexportStatements, isStringImportOrReexport} from '../../src/dependencies/esm_dependency_host'; import {ModuleResolver} from '../../src/dependencies/module_resolver'; @@ -29,13 +30,15 @@ runInEachFileSystem(() => { it('should not generate a TS AST if the source does not contain any imports or re-exports', () => { spyOn(ts, 'createSourceFile'); - host.findDependencies(_('/no/imports/or/re-exports/index.js')); + host.collectDependencies( + _('/no/imports/or/re-exports/index.js'), createDependencyInfo()); 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')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); @@ -44,8 +47,9 @@ runInEachFileSystem(() => { }); it('should resolve all the external re-exports of the source file', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/re-exports/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/re-exports/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); @@ -54,8 +58,9 @@ runInEachFileSystem(() => { }); it('should capture missing external imports', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports-missing/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports-missing/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib-1'))).toBe(true); @@ -68,8 +73,9 @@ runInEachFileSystem(() => { // 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')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/deep-import/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(0); expect(missing.size).toBe(0); @@ -78,8 +84,9 @@ runInEachFileSystem(() => { }); it('should recurse into internal dependencies', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/internal/outer/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/internal/outer/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib-1/sub-1'))).toBe(true); @@ -88,8 +95,9 @@ runInEachFileSystem(() => { }); it('should handle circular internal dependencies', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/internal/circular-a/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/internal/circular-a/index.js'), {dependencies, missing, deepImports}); 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); @@ -106,8 +114,8 @@ runInEachFileSystem(() => { '@lib/*/test': ['lib/*/test'], } })); - const {dependencies, missing, deepImports} = - host.findDependencies(_('/path-alias/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies(_('/path-alias/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(4); expect(dependencies.has(_('/dist/components'))).toBe(true); expect(dependencies.has(_('/dist/shared'))).toBe(true); @@ -118,8 +126,9 @@ runInEachFileSystem(() => { }); it('should handle entry-point paths with no extension', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports/index')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports/index'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); 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 b20e659ff6..41e52ecdc3 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 @@ -10,6 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, relativeFrom} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; +import {createDependencyInfo} from '../../src/dependencies/dependency_host'; import {ModuleResolver} from '../../src/dependencies/module_resolver'; import {UmdDependencyHost} from '../../src/dependencies/umd_dependency_host'; @@ -28,13 +29,14 @@ runInEachFileSystem(() => { 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')); + host.collectDependencies(_('/no/imports/or/re-exports/index.js'), createDependencyInfo()); 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')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); @@ -43,8 +45,9 @@ runInEachFileSystem(() => { }); it('should resolve all the external re-exports of the source file', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/re-exports/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/re-exports/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0); @@ -53,8 +56,9 @@ runInEachFileSystem(() => { }); it('should capture missing external imports', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports-missing/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports-missing/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true); @@ -67,8 +71,9 @@ runInEachFileSystem(() => { // 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')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/deep-import/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(0); expect(missing.size).toBe(0); @@ -77,8 +82,9 @@ runInEachFileSystem(() => { }); it('should recurse into internal dependencies', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/internal/outer/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/internal/outer/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(1); expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true); @@ -87,8 +93,9 @@ runInEachFileSystem(() => { }); it('should handle circular internal dependencies', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/internal/circular_a/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/internal/circular_a/index.js'), {dependencies, missing, deepImports}); 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); @@ -105,8 +112,8 @@ runInEachFileSystem(() => { '@lib/*/test': ['lib/*/test'], } })); - const {dependencies, missing, deepImports} = - host.findDependencies(_('/path-alias/index.js')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies(_('/path-alias/index.js'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(4); expect(dependencies.has(_('/dist/components'))).toBe(true); expect(dependencies.has(_('/dist/shared'))).toBe(true); @@ -117,8 +124,9 @@ runInEachFileSystem(() => { }); it('should handle entry-point paths with no extension', () => { - const {dependencies, missing, deepImports} = - host.findDependencies(_('/external/imports/index')); + const {dependencies, missing, deepImports} = createDependencyInfo(); + host.collectDependencies( + _('/external/imports/index'), {dependencies, missing, deepImports}); expect(dependencies.size).toBe(2); expect(missing.size).toBe(0); expect(deepImports.size).toBe(0);