From cc4b813e759f16fb0f4dfa92a0e6464ed768a629 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 30 Mar 2020 21:32:04 +0100 Subject: [PATCH] fix(ngcc): handle bad path mappings when finding entry-points (#36331) Previously, a bad baseUrl or path mapping passed to an `EntryPointFinder` could cause the original `sourceDirectory` to be superceded by a higher directory. This could result in none of the sourceDirectory entry-points being processed. Now missing basePaths computed from path-mappings are discarded with a warning. Further, if the `baseUrl` is the root directory then a warning is given as this is most likely an error in the tsconfig.json. Resolves #36313 Resolves #36283 PR Close #36331 --- .../directory_walker_entry_point_finder.ts | 2 +- .../targeted_entry_point_finder.ts | 2 +- .../ngcc/src/entry_point_finder/utils.ts | 35 ++++- ...irectory_walker_entry_point_finder_spec.ts | 6 +- .../test/entry_point_finder/utils_spec.ts | 142 ++++++++++++++++++ 5 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 packages/compiler-cli/ngcc/test/entry_point_finder/utils_spec.ts diff --git a/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts b/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts index f2b00325ae..bf9e6cca9c 100644 --- a/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts +++ b/packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts @@ -21,7 +21,7 @@ import {getBasePaths, trackDuration} from './utils'; * `pathMappings`. */ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder { - private basePaths = getBasePaths(this.sourceDirectory, this.pathMappings); + private basePaths = getBasePaths(this.logger, this.sourceDirectory, this.pathMappings); constructor( private fs: FileSystem, private config: NgccConfiguration, private logger: Logger, private resolver: DependencyResolver, private entryPointManifest: EntryPointManifest, 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 7014c247cf..f21d4cd64c 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 @@ -25,7 +25,7 @@ import {getBasePaths} from './utils'; export class TargetedEntryPointFinder implements EntryPointFinder { private unprocessedPaths: AbsoluteFsPath[] = []; private unsortedEntryPoints = new Map(); - private basePaths = getBasePaths(this.basePath, this.pathMappings); + private basePaths = getBasePaths(this.logger, this.basePath, this.pathMappings); constructor( private fs: FileSystem, private config: NgccConfiguration, private logger: Logger, diff --git a/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts b/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts index c81e5cc01f..f2ea90da10 100644 --- a/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts +++ b/packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts @@ -5,7 +5,8 @@ * 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, relative, resolve} from '../../../src/ngtsc/file_system'; +import {AbsoluteFsPath, getFileSystem, relative, resolve} from '../../../src/ngtsc/file_system'; +import {Logger} from '../logging/logger'; import {PathMappings} from '../utils'; /** @@ -28,22 +29,44 @@ import {PathMappings} from '../utils'; * @param pathMappings Path mapping configuration, from which to extract additional base-paths. */ export function getBasePaths( - sourceDirectory: AbsoluteFsPath, pathMappings: PathMappings | undefined): AbsoluteFsPath[] { + logger: Logger, sourceDirectory: AbsoluteFsPath, + pathMappings: PathMappings | undefined): AbsoluteFsPath[] { const fs = getFileSystem(); const basePaths = [sourceDirectory]; if (pathMappings) { const baseUrl = resolve(pathMappings.baseUrl); + if (fs.isRoot(baseUrl)) { + logger.warn( + `The provided pathMappings baseUrl is the root path ${baseUrl}.\n` + + `This is likely to mess up how ngcc finds entry-points and is probably not correct.\n` + + `Please check your path mappings configuration such as in the tsconfig.json file.`); + } Object.values(pathMappings.paths).forEach(paths => paths.forEach(path => { // We only want base paths that exist and are not files - let basePath = join(baseUrl, extractPathPrefix(path)); - while (basePath !== baseUrl && (!fs.exists(basePath) || fs.stat(basePath).isFile())) { + let basePath = fs.resolve(baseUrl, extractPathPrefix(path)); + if (fs.exists(basePath) && fs.stat(basePath).isFile()) { basePath = fs.dirname(basePath); } - basePaths.push(basePath); + if (fs.exists(basePath)) { + basePaths.push(basePath); + } else { + logger.warn( + `The basePath "${basePath}" computed from baseUrl "${baseUrl}" and path mapping "${path}" does not exist in the file-system.\n` + + `It will not be scanned for entry-points.`); + } })); } basePaths.sort().reverse(); // Get the paths in order with the longer ones first. - return basePaths.filter(removeContainedPaths); + const dedupedBasePaths = basePaths.filter(removeContainedPaths); + + // We want to ensure that the `sourceDirectory` is included when it is a node_modules folder. + // Otherwise our entry-point finding algorithm would fail to walk that folder. + if (fs.basename(sourceDirectory) === 'node_modules' && + !dedupedBasePaths.includes(sourceDirectory)) { + dedupedBasePaths.unshift(sourceDirectory); + } + + return dedupedBasePaths; } /** diff --git a/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts b/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts index e10784bec7..9d9c873fab 100644 --- a/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts +++ b/packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts @@ -340,7 +340,7 @@ runInEachFileSystem(() => { const pathMappings: PathMappings = { baseUrl: '/path_mapped/dist', paths: { - '@test': ['pkg2/fesm2015/pkg2.js'], + '@test': ['pkg2/pkg2.d.ts'], '@missing': ['pkg3'], } }; @@ -371,6 +371,10 @@ runInEachFileSystem(() => { fesm2015: `./fesm2015/${packageName}.js`, }) }, + { + name: _Abs(`${basePath}/${packageName}/${packageName}.d.ts`), + contents: deps.map((dep, i) => `import * as i${i} from '${dep}';`).join('\n'), + }, { name: _Abs(`${basePath}/${packageName}/fesm2015/${packageName}.js`), contents: deps.map((dep, i) => `import * as i${i} from '${dep}';`).join('\n'), diff --git a/packages/compiler-cli/ngcc/test/entry_point_finder/utils_spec.ts b/packages/compiler-cli/ngcc/test/entry_point_finder/utils_spec.ts new file mode 100644 index 0000000000..996370b9da --- /dev/null +++ b/packages/compiler-cli/ngcc/test/entry_point_finder/utils_spec.ts @@ -0,0 +1,142 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * 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 {absoluteFrom, getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system'; + +import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; +import {getBasePaths} from '../../src/entry_point_finder/utils'; +import {MockLogger} from '../helpers/mock_logger'; + +runInEachFileSystem(() => { + let _: typeof absoluteFrom; + let logger: MockLogger; + + beforeEach(() => { + _ = absoluteFrom; + logger = new MockLogger(); + }); + + describe('getBasePaths', () => { + it('should just return the `sourceDirectory if there are no `pathMappings', () => { + const sourceDirectory = _('/path/to/project/node_modules'); + const basePaths = getBasePaths(logger, sourceDirectory, undefined); + expect(basePaths).toEqual([sourceDirectory]); + }); + + it('should use each path mapping prefix and sort in descending order', () => { + const projectDirectory = _('/path/to/project'); + const fs = getFileSystem(); + fs.ensureDir(fs.resolve(projectDirectory, 'dist-1')); + fs.ensureDir(fs.resolve(projectDirectory, 'sub-folder/dist-2')); + fs.ensureDir(fs.resolve(projectDirectory, 'libs')); + + const sourceDirectory = _('/path/to/project/node_modules'); + const pathMappings = { + baseUrl: projectDirectory, + paths: {'@dist': ['dist-1', 'sub-folder/dist-2'], '@lib/*': ['libs/*']} + }; + const basePaths = getBasePaths(logger, sourceDirectory, pathMappings); + expect(basePaths).toEqual([ + fs.resolve(projectDirectory, 'sub-folder/dist-2'), + sourceDirectory, + fs.resolve(projectDirectory, 'libs'), + fs.resolve(projectDirectory, 'dist-1'), + ]); + }); + + it('should discard paths that are already contained by another path', () => { + const projectDirectory = _('/path/to/project'); + const fs = getFileSystem(); + fs.ensureDir(fs.resolve(projectDirectory, 'dist-1')); + fs.ensureDir(fs.resolve(projectDirectory, 'dist-1/sub-folder')); + fs.ensureDir(fs.resolve(projectDirectory, 'node_modules/libs')); + + const sourceDirectory = _('/path/to/project/node_modules'); + const pathMappings = { + baseUrl: projectDirectory, + paths: {'@dist': ['dist-1', 'dist-1/sub-folder'], '@lib/*': ['node_modules/libs/*']} + }; + const basePaths = getBasePaths(logger, sourceDirectory, pathMappings); + expect(basePaths).toEqual([ + sourceDirectory, + fs.resolve(projectDirectory, 'dist-1'), + ]); + }); + + it('should use the containing directory of path mapped files', () => { + const projectDirectory = _('/path/to/project'); + const fs = getFileSystem(); + fs.ensureDir(fs.resolve(projectDirectory, 'dist-1')); + fs.writeFile(fs.resolve(projectDirectory, 'dist-1/file.js'), 'dummy content'); + + const sourceDirectory = _('/path/to/project/node_modules'); + const pathMappings = {baseUrl: projectDirectory, paths: {'@dist': ['dist-1/file.js']}}; + const basePaths = getBasePaths(logger, sourceDirectory, pathMappings); + expect(basePaths).toEqual([ + sourceDirectory, + fs.resolve(projectDirectory, 'dist-1'), + ]); + }); + + it('should always include the `sourceDirectory` if it is a node_modules directory in the returned basePaths, even if it is contained by another basePath', + () => { + const projectDirectory = _('/path/to/project'); + const sourceDirectory = _('/path/to/project/node_modules'); + const fs = getFileSystem(); + fs.ensureDir(fs.resolve(sourceDirectory)); + + const pathMappings = {baseUrl: projectDirectory, paths: {'*': ['./*']}}; + const basePaths = getBasePaths(logger, sourceDirectory, pathMappings); + expect(basePaths).toEqual([ + sourceDirectory, + projectDirectory, + ]); + }); + + it('should log a warning if baseUrl is the root path', () => { + const fs = getFileSystem(); + fs.ensureDir(fs.resolve('/dist')); + + const sourceDirectory = _('/path/to/project/node_modules'); + const pathMappings = {baseUrl: _('/'), paths: {'@dist': ['dist']}}; + const basePaths = getBasePaths(logger, sourceDirectory, pathMappings); + expect(basePaths).toEqual([ + sourceDirectory, + fs.resolve('/dist'), + ]); + expect(logger.logs.warn).toEqual([ + [`The provided pathMappings baseUrl is the root path ${_('/')}.\n` + + `This is likely to mess up how ngcc finds entry-points and is probably not correct.\n` + + `Please check your path mappings configuration such as in the tsconfig.json file.`] + ]); + }); + + it('should discard basePaths that do not exists and log a warning', () => { + const projectDirectory = _('/path/to/project'); + const fs = getFileSystem(); + fs.ensureDir(fs.resolve(projectDirectory, 'dist-1')); + fs.ensureDir(fs.resolve(projectDirectory, 'sub-folder')); + + const sourceDirectory = _('/path/to/project/node_modules'); + const pathMappings = { + baseUrl: projectDirectory, + paths: {'@dist': ['dist-1', 'sub-folder/dist-2'], '@lib/*': ['libs/*']} + }; + const basePaths = getBasePaths(logger, sourceDirectory, pathMappings); + expect(basePaths).toEqual([ + sourceDirectory, + fs.resolve(projectDirectory, 'dist-1'), + ]); + expect(logger.logs.warn).toEqual([ + [`The basePath "${fs.resolve(projectDirectory, 'sub-folder/dist-2')}" computed from baseUrl "${projectDirectory}" and path mapping "sub-folder/dist-2" does not exist in the file-system.\n` + + `It will not be scanned for entry-points.`], + [`The basePath "${fs.resolve(projectDirectory, 'libs')}" computed from baseUrl "${projectDirectory}" and path mapping "libs/*" does not exist in the file-system.\n` + + `It will not be scanned for entry-points.`], + ]); + }); + }); +}); \ No newline at end of file