From 50c4ec66871aa7aa24d92f562922de08b9cd045d Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 8 Jul 2019 12:43:32 +0100 Subject: [PATCH] fix(ivy): ngcc - resolve path-mapped modules correctly (#31450) Non-wild-card path-mappings were not being matched correctly. Further path-mapped secondary entry-points that were imported from the associated primary entry-point were not being martched correctly. Fixes #31274 PR Close #31450 --- .../ngcc/src/dependencies/module_resolver.ts | 30 +++++++++------ .../test/dependencies/module_resolver_spec.ts | 38 +++++++++++++++++++ 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts b/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts index 98cc8e54fa..549690a092 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts @@ -94,14 +94,13 @@ export class ModuleResolver { const packagePath = this.findPackagePath(fromPath); if (packagePath !== null) { for (const mappedPath of mappedPaths) { - const isRelative = - mappedPath.startsWith(packagePath) && !mappedPath.includes('node_modules'); - if (isRelative) { - return this.resolveAsRelativePath(mappedPath, fromPath); - } else if (this.isEntryPoint(mappedPath)) { + if (this.isEntryPoint(mappedPath)) { return new ResolvedExternalModule(mappedPath); - } else if (this.resolveAsRelativePath(mappedPath, fromPath)) { - return new ResolvedDeepImport(mappedPath); + } + const nonEntryPointImport = this.resolveAsRelativePath(mappedPath, fromPath); + if (nonEntryPointImport !== null) { + return isRelativeImport(packagePath, mappedPath) ? nonEntryPointImport : + new ResolvedDeepImport(mappedPath); } } } @@ -190,7 +189,9 @@ export class ModuleResolver { } } - return (bestMapping && bestMatch) ? this.computeMappedTemplates(bestMapping, bestMatch) : []; + return (bestMapping !== undefined && bestMatch !== undefined) ? + this.computeMappedTemplates(bestMapping, bestMatch) : + []; } /** @@ -203,10 +204,13 @@ export class ModuleResolver { */ private matchMapping(path: string, mapping: ProcessedPathMapping): string|null { const {prefix, postfix, hasWildcard} = mapping.matcher; - if (path.startsWith(prefix) && path.endsWith(postfix)) { - return hasWildcard ? path.substring(prefix.length, path.length - postfix.length) : ''; + if (hasWildcard) { + return (path.startsWith(prefix) && path.endsWith(postfix)) ? + path.substring(prefix.length, path.length - postfix.length) : + null; + } else { + return (path === prefix) ? '' : null; } - return null; } /** @@ -277,3 +281,7 @@ interface PathMappingPattern { postfix: string; hasWildcard: boolean; } + +function isRelativeImport(from: AbsoluteFsPath, to: AbsoluteFsPath) { + return to.startsWith(from) && !to.includes('node_modules'); +} diff --git a/packages/compiler-cli/ngcc/test/dependencies/module_resolver_spec.ts b/packages/compiler-cli/ngcc/test/dependencies/module_resolver_spec.ts index 452b2a75a4..c88856fa01 100644 --- a/packages/compiler-cli/ngcc/test/dependencies/module_resolver_spec.ts +++ b/packages/compiler-cli/ngcc/test/dependencies/module_resolver_spec.ts @@ -43,6 +43,11 @@ runInEachFileSystem(() => { name: _('/dist/package-4/sub-folder/index.js'), contents: `import {X} from '@shared/package-4/x';` }, + {name: _('/dist/package-4/secondary-entry-point/x.js'), contents: `export class X {}`}, + { + name: _('/dist/package-4/secondary-entry-point/package.json'), + contents: 'PACKAGE.JSON for secondary-entry-point' + }, { name: _('/dist/sub-folder/package-4/package.json'), contents: 'PACKAGE.JSON for package-4' @@ -199,6 +204,39 @@ runInEachFileSystem(() => { _('/dist/package-4/sub-folder/index.js'))) .toEqual(new ResolvedExternalModule(_('/dist/sub-folder/package-5/post-fix'))); }); + + it('should resolve primary entry-points if they match non-wildcards exactly', () => { + const resolver = new ModuleResolver( + getFileSystem(), {baseUrl: '/dist', paths: {'package-4': ['package-4']}}); + expect(resolver.resolveModuleImport('package-4', _('/libs/local-package/index.js'))) + .toEqual(new ResolvedExternalModule(_('/dist/package-4'))); + expect(resolver.resolveModuleImport( + 'package-4/secondary-entry-point', _('/libs/local-package/index.js'))) + .toEqual(null); + }); + + it('should resolve secondary entry-points if wildcards match', () => { + const resolver = new ModuleResolver(getFileSystem(), { + baseUrl: '/dist', + paths: {'package-4': ['package-4'], 'package-4/*': ['package-4/*']} + }); + expect(resolver.resolveModuleImport('package-4', _('/libs/local-package/index.js'))) + .toEqual(new ResolvedExternalModule(_('/dist/package-4'))); + expect(resolver.resolveModuleImport( + 'package-4/secondary-entry-point', _('/libs/local-package/index.js'))) + .toEqual(new ResolvedExternalModule(_('/dist/package-4/secondary-entry-point'))); + }); + + it('should resolve secondary-entry-points referenced from their primary entry-point', + () => { + const resolver = new ModuleResolver(getFileSystem(), { + baseUrl: '/dist', + paths: {'package-4': ['package-4'], 'package-4/*': ['package-4/*']} + }); + expect(resolver.resolveModuleImport( + 'package-4/secondary-entry-point', _('/dist/package-4/index.js'))) + .toEqual(new ResolvedExternalModule(_('/dist/package-4/secondary-entry-point'))); + }); }); }); });