From 69950e3888fad6b20725a7e76cbc32b4547d1bda Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 19 Dec 2019 22:43:12 +0000 Subject: [PATCH] refactor(ngcc): resolve modules based on the provided `moduleResolver` (#34494) The `DependencyHost` implementations were duplicating the "postfix" strings which are used to find matching paths when resolving module specifiers. Now the hosts reuse the postfixes given to the `ModuleResolver` that is passed to the host. PR Close #34494 --- .../ngcc/src/dependencies/dependency_host.ts | 2 +- .../ngcc/src/dependencies/module_resolver.ts | 2 +- .../dependencies/esm_dependency_host_spec.ts | 34 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts index c5fbf25d64..d71beca388 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_host.ts @@ -39,7 +39,7 @@ export abstract class DependencyHostBase implements DependencyHost { collectDependencies( entryPointPath: AbsoluteFsPath, {dependencies, missing, deepImports}: DependencyInfo): void { const resolvedFile = - resolveFileWithPostfixes(this.fs, entryPointPath, ['', '.js', '/index.js']); + resolveFileWithPostfixes(this.fs, entryPointPath, this.moduleResolver.relativeExtensions); if (resolvedFile !== null) { const alreadySeen = new Set(); this.recursivelyCollectDependencies( diff --git a/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts b/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts index b059452ec2..1179dcd53f 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts @@ -24,7 +24,7 @@ import {PathMappings, isRelativePath, resolveFileWithPostfixes} from '../utils'; export class ModuleResolver { private pathMappings: ProcessedPathMapping[]; - constructor(private fs: FileSystem, pathMappings?: PathMappings, private relativeExtensions = [ + constructor(private fs: FileSystem, pathMappings?: PathMappings, readonly relativeExtensions = [ '', '.js', '/index.js' ]) { this.pathMappings = pathMappings ? this.processPathMappings(pathMappings) : []; 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 fe754b939e..4a272abe25 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 @@ -135,6 +135,40 @@ runInEachFileSystem(() => { expect(dependencies.has(_('/node_modules/lib-1'))).toBe(true); expect(dependencies.has(_('/node_modules/lib-1/sub-1'))).toBe(true); }); + + it('should resolve modules based on the provided `moduleResolver`', () => { + const fs = getFileSystem(); + loadTestFiles([ + { + name: _('/external/index.d.ts'), + contents: `import * from './internal-typings';`, + }, + { + name: _('/external/internal-typings.d.ts'), + contents: `export {X} from 'lib-1';\nexport {Y} from 'lib-1/sub-1';`, + } + ]); + + // Default JS mode will not pick up `internal-typings.d.ts` dependency + const jsHost = new EsmDependencyHost(fs, new ModuleResolver(fs)); + const jsDeps = createDependencyInfo(); + jsHost.collectDependencies(_('/external/index.d.ts'), jsDeps); + expect(jsDeps.dependencies.size).toEqual(0); + expect(jsDeps.deepImports.size).toEqual(0); + expect(jsDeps.missing.size).toEqual(1); + expect(jsDeps.missing.has(relativeFrom('./internal-typings'))).toBeTruthy(); + + // Typings mode will pick up `internal-typings.d.ts` dependency + const dtsHost = new EsmDependencyHost( + fs, new ModuleResolver(fs, undefined, ['', '.d.ts', 'index.d.ts'])); + const dtsDeps = createDependencyInfo(); + dtsHost.collectDependencies(_('/external/index.d.ts'), dtsDeps); + expect(dtsDeps.dependencies.size).toEqual(2); + expect(dtsDeps.dependencies.has(_('/node_modules/lib-1'))).toBeTruthy(); + expect(dtsDeps.dependencies.has(_('/node_modules/lib-1/sub-1'))).toBeTruthy(); + expect(dtsDeps.deepImports.size).toEqual(0); + expect(dtsDeps.missing.size).toEqual(0); + }); }); function setupMockFileSystem(): void {