Skip to content

Commit

Permalink
fix(ngcc): ensure that path-mapped secondary entry-points are process…
Browse files Browse the repository at this point in the history
…ed correctly (#35227)

The `TargetedEntryPointFinder` must work out what the
containing package is for each entry-point that it finds.

The logic for doing this was flawed in the case that the
package was in a path-mapped directory and not in a
node_modules folder. This meant that secondary entry-points
were incorrectly setting their own path as the package
path, rather than the primary entry-point path.

Fixes #35188

PR Close #35227
  • Loading branch information
petebacondarwin authored and kara committed Feb 7, 2020
1 parent 24ffe37 commit c3c1140
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,27 +110,37 @@ export class TargetedEntryPointFinder implements EntryPointFinder {
if (entryPointPath.startsWith(basePath)) {
let packagePath = basePath;
const segments = this.splitPath(relative(basePath, entryPointPath));

// Start the search at the last nested `node_modules` folder if the relative
// `entryPointPath` contains one or more.
let nodeModulesIndex = segments.lastIndexOf(relativeFrom('node_modules'));

// If there are no `node_modules` in the relative path between the `basePath` and the
// `entryPointPath` then just try the `basePath` as the `packagePath`.
// (This can be the case with path-mapped entry-points.)
if (nodeModulesIndex === -1) {
if (this.fs.exists(join(packagePath, 'package.json'))) {
return packagePath;
}
}

// Start the search at the deepest nested `node_modules` folder that is below the `basePath`
// but above the `entryPointPath`, if there are any.
while (nodeModulesIndex >= 0) {
packagePath = join(packagePath, segments.shift() !);
nodeModulesIndex--;
}

// Note that we skip the first `packagePath` and start looking from the first folder below
// it because that will be the `node_modules` folder.
// Note that we start at the folder below the current candidate `packagePath` because the
// initial candidate `packagePath` is either a `node_modules` folder or the `basePath` with
// no `package.json`.
for (const segment of segments) {
packagePath = join(packagePath, segment);
if (this.fs.exists(join(packagePath, 'package.json'))) {
return packagePath;
}
}

// If we got here then we couldn't find a `packagePath` for the current `basePath` but since
// `basePath`s are guaranteed not to be a sub-directory each other then no other `basePath`
// will match either.
// If we got here then we couldn't find a `packagePath` for the current `basePath`.
// Since `basePath`s are guaranteed not to be a sub-directory of each other then no other
// `basePath` will match either.
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,31 @@ runInEachFileSystem(() => {
]);
});

it('should correctly compute the packagePath of secondary entry-points via pathMappings',
() => {
const basePath = _Abs('/path_mapped/node_modules');
const targetPath = _Abs('/path_mapped/dist/primary/secondary');
const pathMappings: PathMappings = {
baseUrl: '/path_mapped/dist',
paths: {'libs': ['primary'], 'extras': ['primary/*']}
};
loadTestFiles([
...createPackage(_Abs('/path_mapped/dist'), 'primary'),
...createPackage(_Abs('/path_mapped/dist/primary'), 'secondary'),
]);
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(entryPoints.length).toEqual(1);
const entryPoint = entryPoints[0];
expect(entryPoint.name).toEqual('secondary');
expect(entryPoint.path).toEqual(_Abs('/path_mapped/dist/primary/secondary'));
expect(entryPoint.package).toEqual(_Abs('/path_mapped/dist/primary'));
});

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');
Expand Down Expand Up @@ -451,6 +476,7 @@ runInEachFileSystem(() => {
{
name: _Abs(`${basePath}/${packageName}/package.json`),
contents: JSON.stringify({
name: packageName,
typings: `./${packageName}.d.ts`,
fesm2015: `./fesm2015/${packageName}.js`,
esm5: `./esm5/${packageName}.js`,
Expand Down

0 comments on commit c3c1140

Please sign in to comment.