From afd11662a36f1954c3af9db0f110db9e1130a0c0 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 9 Jan 2021 18:20:03 +0000 Subject: [PATCH] fix(ngcc): compute the correct package paths for target entry-points (#40376) Previously, if there were path-mapped entry-points, where one contaied the string of another - for example `worker-client` and `worker` - then the base paths were incorrectly computed resulting in the wrong package path for the longer entry-point. This was because, when searching for a matching base path, the strings were tested using `startsWith()`, whereas we should only match if the path was contained in a directory from a file-system point of view. Now we not only check whether the target path "starts with" the base path but then also whether the target path is actually contained in the base path using `fs.relative()`. Fixes #40352 Fixes #40357 PR Close #40376 --- .../targeted_entry_point_finder.ts | 16 +++++- .../targeted_entry_point_finder_spec.ts | 50 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/entry_point_finder/targeted_entry_point_finder.ts b/packages/compiler-cli/ngcc/src/entry_point_finder/targeted_entry_point_finder.ts index 26d15045c3..9b729e2183 100644 --- a/packages/compiler-cli/ngcc/src/entry_point_finder/targeted_entry_point_finder.ts +++ b/packages/compiler-cli/ngcc/src/entry_point_finder/targeted_entry_point_finder.ts @@ -119,7 +119,7 @@ export class TargetedEntryPointFinder extends TracingEntryPointFinder { private computePackagePath(entryPointPath: AbsoluteFsPath): AbsoluteFsPath { // First try the main basePath, to avoid having to compute the other basePaths from the paths // mappings, which can be computationally intensive. - if (entryPointPath.startsWith(this.basePath)) { + if (this.isPathContainedBy(this.basePath, entryPointPath)) { const packagePath = this.computePackagePathFromContainingPath(entryPointPath, this.basePath); if (packagePath !== null) { return packagePath; @@ -129,7 +129,7 @@ export class TargetedEntryPointFinder extends TracingEntryPointFinder { // The main `basePath` didn't work out so now we try the `basePaths` computed from the paths // mappings in `tsconfig.json`. for (const basePath of this.getBasePaths()) { - if (entryPointPath.startsWith(basePath)) { + if (this.isPathContainedBy(basePath, entryPointPath)) { const packagePath = this.computePackagePathFromContainingPath(entryPointPath, basePath); if (packagePath !== null) { return packagePath; @@ -147,6 +147,18 @@ export class TargetedEntryPointFinder extends TracingEntryPointFinder { return this.computePackagePathFromNearestNodeModules(entryPointPath); } + /** + * Compute whether the `test` path is contained within the `base` path. + * + * Note that this doesn't use a simple `startsWith()` since that would result in a false positive + * for `test` paths such as `a/b/c-x` when the `base` path is `a/b/c`. + * + * Since `fs.relative()` can be quite expensive we check the fast possibilities first. + */ + private isPathContainedBy(base: AbsoluteFsPath, test: AbsoluteFsPath): boolean { + return test === base || + (test.startsWith(base) && !this.fs.relative(base, test).startsWith('..')); + } /** * Search down to the `entryPointPath` from the `containingPath` for the first `package.json` that diff --git a/packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts b/packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts index d3433ff505..87885a2c9b 100644 --- a/packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts +++ b/packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts @@ -355,6 +355,56 @@ runInEachFileSystem(() => { ]); }); + it('should correctly compute the package path for a target whose name contains the string of another package', + () => { + // Create the "my-lib" package - it doesn't need to be a real entry-point + const myLibPath = _Abs('/project/dist/my-lib'); + loadTestFiles([{ + name: fs.resolve(myLibPath, 'package.json'), + contents: JSON.stringify({name: 'my-lib'}) + }]); + + // Create the "my-lib-other" Angular entry-point + const myLibOtherPath = _Abs('/project/dist/my-lib-other'); + loadTestFiles([ + { + name: fs.resolve(myLibOtherPath, 'package.json'), + contents: JSON.stringify({ + name: `my-lib-other`, + typings: `./my-lib-other.d.ts`, + fesm2015: `./fesm2015/my-lib-other.js`, + esm5: `./esm5/my-lib-other.js`, + main: `./common/my-lib-other.js`, + }) + }, + {name: fs.resolve(myLibOtherPath, 'my-lib-other.metadata.json'), contents: 'metadata'}, + {name: fs.resolve(myLibOtherPath, 'my-lib-other.d.ts'), contents: 'typings'}, + {name: fs.resolve(myLibOtherPath, 'fesm2015/my-lib-other.js'), contents: ''}, + {name: fs.resolve(myLibOtherPath, 'esm5/my-lib-other.js'), contents: ''}, + {name: fs.resolve(myLibOtherPath, 'commonjs/my-lib-other.js'), contents: ''}, + ]); + + const basePath = _Abs('/project/node_modules'); + const pathMappings: PathMappings = { + baseUrl: '/project', + paths: { + 'lib1': ['dist/my-lib'], + 'lib2': ['dist/my-lib-other'], + } + }; + + const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings)); + const dtsHost = new DtsDependencyHost(fs, pathMappings); + resolver = new DependencyResolver(fs, logger, config, {esm2015: srcHost}, dtsHost); + const finder = new TargetedEntryPointFinder( + fs, config, logger, resolver, basePath, pathMappings, myLibOtherPath); + const {entryPoints} = finder.findEntryPoints(); + + expect(dumpEntryPointPaths(basePath, entryPoints)).toEqual([ + ['../dist/my-lib-other', '../dist/my-lib-other'], + ]); + }); + it('should handle pathMappings that map to files or non-existent directories', () => { const basePath = _Abs('/path_mapped/node_modules'); const targetPath = _Abs('/path_mapped/node_modules/test');