fix(ngcc): capture path-mapped entry-points that start with same string (#35592)

Previously if there were two path-mapped libraries that are in
different directories but the path of one started with same string
as the path of the other, we would incorrectly return the shorter
path - e.g. `dist/my-lib` and `dist/my-lib-second`. This was because
the list of `basePaths` was searched in ascending alphabetic order and
we were using `startsWith()` to match the path.

Now the `basePaths` are searched in reverse alphabetic order so the
longer path will be matched correctly.

// FW-1873

Fixes #35536

PR Close #35592
This commit is contained in:
Pete Bacon Darwin 2020-02-20 21:03:14 +01:00 committed by Miško Hevery
parent 53f059ee8f
commit 71b5970450
3 changed files with 52 additions and 10 deletions

View File

@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {AbsoluteFsPath, getFileSystem, join, resolve} from '../../../src/ngtsc/file_system';
import {AbsoluteFsPath, getFileSystem, join, relative, resolve} from '../../../src/ngtsc/file_system';
import {PathMappings} from '../utils';
/**
@ -42,8 +42,8 @@ export function getBasePaths(
basePaths.push(basePath);
}));
}
basePaths.sort(); // Get the paths in order with the shorter ones first.
return basePaths.filter(removeDeeperPaths);
basePaths.sort().reverse(); // Get the paths in order with the longer ones first.
return basePaths.filter(removeContainedPaths);
}
/**
@ -56,16 +56,25 @@ function extractPathPrefix(path: string) {
}
/**
* A filter function that removes paths that are already covered by higher paths.
* A filter function that removes paths that are contained by other paths.
*
* For example:
* Given `['a/b/c', 'a/b/x', 'a/b', 'd/e', 'd/f']` we will end up with `['a/b', 'd/e', 'd/f]`.
* (Note that we do not get `d` even though `d/e` and `d/f` share a base directory, since `d` is not
* one of the base paths.)
*
* @param value The current path.
* @param index The index of the current path.
* @param array The array of paths (sorted alphabetically).
* @returns true if this path is not already covered by a previous path.
* @param array The array of paths (sorted in reverse alphabetical order).
* @returns true if this path is not contained by another path.
*/
function removeDeeperPaths(value: AbsoluteFsPath, index: number, array: AbsoluteFsPath[]) {
for (let i = 0; i < index; i++) {
if (value.startsWith(array[i])) return false;
function removeContainedPaths(value: AbsoluteFsPath, index: number, array: AbsoluteFsPath[]) {
// We only need to check the following paths since the `array` is sorted in reverse alphabetic
// order.
for (let i = index + 1; i < array.length; i++) {
// We need to use `relative().startsWith()` rather than a simple `startsWith()` to ensure we
// don't assume that `a/b` contains `a/b-2`.
if (!relative(array[i], value).startsWith('..')) return false;
}
return true;
}

View File

@ -189,8 +189,8 @@ runInEachFileSystem(() => {
fs, config, logger, resolver, basePath, pathMappings);
const {entryPoints} = finder.findEntryPoints();
expect(dumpEntryPointPaths(basePath, entryPoints)).toEqual([
['../dist/pkg2', '../dist/pkg2'],
['test', 'test'],
['../dist/pkg2', '../dist/pkg2'],
]);
});

View File

@ -228,6 +228,39 @@ runInEachFileSystem(() => {
expect(entryPoint.package).toEqual(_Abs('/path_mapped/dist/primary'));
});
it('should correctly compute an entry-point whose path starts with the same string as another entry-point, via pathMappings',
() => {
const basePath = _Abs('/path_mapped/node_modules');
const targetPath = _Abs('/path_mapped/node_modules/test');
const pathMappings: PathMappings = {
baseUrl: '/path_mapped/dist',
paths: {
'lib1': ['my-lib/my-lib', 'my-lib'],
'lib2': ['my-lib/a', 'my-lib/a'],
'lib3': ['my-lib/b', 'my-lib/b'],
'lib4': ['my-lib-other/my-lib-other', 'my-lib-other']
}
};
loadTestFiles([
...createPackage(_Abs('/path_mapped/node_modules'), 'test', ['lib2', 'lib4']),
...createPackage(_Abs('/path_mapped/dist/my-lib'), 'my-lib'),
...createPackage(_Abs('/path_mapped/dist/my-lib'), 'a'),
...createPackage(_Abs('/path_mapped/dist/my-lib'), 'b'),
...createPackage(_Abs('/path_mapped/dist/my-lib-other'), 'my-lib-other'),
]);
const srcHost = new EsmDependencyHost(fs, new ModuleResolver(fs, pathMappings));
const dtsHost = new DtsDependencyHost(fs, pathMappings);
resolver = new DependencyResolver(fs, logger, {esm2015: srcHost}, dtsHost);
const finder = new TargetedEntryPointFinder(
fs, config, logger, resolver, basePath, targetPath, pathMappings);
const {entryPoints} = finder.findEntryPoints();
expect(dumpEntryPointPaths(basePath, entryPoints)).toEqual([
['../dist/my-lib/a', '../dist/my-lib/a'],
['../dist/my-lib-other/my-lib-other', '../dist/my-lib-other/my-lib-other'],
['test', 'test'],
]);
});
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');