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
This commit is contained in:
Pete Bacon Darwin 2021-01-09 18:20:03 +00:00 committed by atscott
parent 9cbc0df91c
commit afd11662a3
2 changed files with 64 additions and 2 deletions

View File

@ -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

View File

@ -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');