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 90470ae843..0d0b2db958 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 @@ -9,8 +9,9 @@ import {AbsoluteFsPath, FileSystem, join, resolve} from '../../../src/ngtsc/file import {DependencyResolver, SortedEntryPointsInfo} from '../dependencies/dependency_resolver'; import {Logger} from '../logging/logger'; import {NgccConfiguration} from '../packages/configuration'; -import {EntryPoint, getEntryPointInfo} from '../packages/entry_point'; +import {EntryPoint, INVALID_ENTRY_POINT, NO_ENTRY_POINT, getEntryPointInfo} from '../packages/entry_point'; import {PathMappings} from '../utils'; +import {NGCC_DIRECTORY} from '../writing/new_entry_point_file_writer'; import {EntryPointFinder} from './interface'; import {getBasePaths} from './utils'; @@ -40,10 +41,24 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder { * The function will recurse into directories that start with `@...`, e.g. `@angular/...`. * @param sourceDirectory An absolute path to the root directory where searching begins. */ - private walkDirectoryForEntryPoints(sourceDirectory: AbsoluteFsPath): EntryPoint[] { + walkDirectoryForEntryPoints(sourceDirectory: AbsoluteFsPath): EntryPoint[] { const entryPoints = this.getEntryPointsForPackage(sourceDirectory); + if (entryPoints === null) { + return []; + } + if (entryPoints.length > 0) { - // The `sourceDirectory` is an entry-point itself so no need to search its sub-directories. + // The `sourceDirectory` is an entry point itself so no need to search its sub-directories. + // Also check for any nested node_modules in this package but only if it was compiled by + // Angular. + // It is unlikely that a non Angular entry point has a dependency on an Angular library. + if (entryPoints.some(e => e.compiledByAngular)) { + const nestedNodeModulesPath = this.fs.join(sourceDirectory, 'node_modules'); + if (this.fs.exists(nestedNodeModulesPath)) { + entryPoints.push(...this.walkDirectoryForEntryPoints(nestedNodeModulesPath)); + } + } + return entryPoints; } @@ -52,23 +67,16 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder { // Not interested in hidden files .filter(p => !p.startsWith('.')) // Ignore node_modules - .filter(p => p !== 'node_modules') + .filter(p => p !== 'node_modules' && p !== NGCC_DIRECTORY) // Only interested in directories (and only those that are not symlinks) .filter(p => { const stat = this.fs.lstat(resolve(sourceDirectory, p)); return stat.isDirectory() && !stat.isSymbolicLink(); }) .forEach(p => { - // Either the directory is a potential package or a namespace containing packages (e.g - // `@angular`). + // Package is a potential namespace containing packages (e.g `@angular`). const packagePath = join(sourceDirectory, p); entryPoints.push(...this.walkDirectoryForEntryPoints(packagePath)); - - // Also check for any nested node_modules in this package - const nestedNodeModulesPath = join(packagePath, 'node_modules'); - if (this.fs.exists(nestedNodeModulesPath)) { - entryPoints.push(...this.walkDirectoryForEntryPoints(nestedNodeModulesPath)); - } }); return entryPoints; } @@ -76,9 +84,9 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder { /** * Recurse the folder structure looking for all the entry points * @param packagePath The absolute path to an npm package that may contain entry points - * @returns An array of entry points that were discovered. + * @returns An array of entry points that were discovered or null when it's not a valid entrypoint */ - private getEntryPointsForPackage(packagePath: AbsoluteFsPath): EntryPoint[] { + private getEntryPointsForPackage(packagePath: AbsoluteFsPath): EntryPoint[]|null { const entryPoints: EntryPoint[] = []; // Try to get an entry point from the top level package directory @@ -86,20 +94,30 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder { getEntryPointInfo(this.fs, this.config, this.logger, packagePath, packagePath); // If there is no primary entry-point then exit - if (topLevelEntryPoint === null) { + if (topLevelEntryPoint === NO_ENTRY_POINT) { return []; } + if (topLevelEntryPoint === INVALID_ENTRY_POINT) { + return null; + } + // Otherwise store it and search for secondary entry-points entryPoints.push(topLevelEntryPoint); this.walkDirectory(packagePath, packagePath, (path, isDirectory) => { + if (!path.endsWith('.js') && !isDirectory) { + return false; + } + // If the path is a JS file then strip its extension and see if we can match an entry-point. const possibleEntryPointPath = isDirectory ? path : stripJsExtension(path); const subEntryPoint = getEntryPointInfo(this.fs, this.config, this.logger, packagePath, possibleEntryPointPath); - if (subEntryPoint !== null) { - entryPoints.push(subEntryPoint); + if (subEntryPoint === NO_ENTRY_POINT || subEntryPoint === INVALID_ENTRY_POINT) { + return false; } + entryPoints.push(subEntryPoint); + return true; }); return entryPoints; @@ -113,26 +131,25 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder { */ private walkDirectory( packagePath: AbsoluteFsPath, dir: AbsoluteFsPath, - fn: (path: AbsoluteFsPath, isDirectory: boolean) => void) { + fn: (path: AbsoluteFsPath, isDirectory: boolean) => boolean) { return this.fs .readdir(dir) // Not interested in hidden files .filter(path => !path.startsWith('.')) // Ignore node_modules - .filter(path => path !== 'node_modules') - .map(path => resolve(dir, path)) + .filter(path => path !== 'node_modules' && path !== NGCC_DIRECTORY) .forEach(path => { - const stat = this.fs.lstat(path); + const absolutePath = resolve(dir, path); + const stat = this.fs.lstat(absolutePath); if (stat.isSymbolicLink()) { // We are not interested in symbolic links return; } - fn(path, stat.isDirectory()); - - if (stat.isDirectory()) { - this.walkDirectory(packagePath, path, fn); + const containsEntryPoint = fn(absolutePath, stat.isDirectory()); + if (containsEntryPoint) { + this.walkDirectory(packagePath, absolutePath, fn); } }); } 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 6228d7b64e..35a63a5698 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 @@ -10,7 +10,7 @@ import {DependencyResolver, SortedEntryPointsInfo} from '../dependencies/depende import {Logger} from '../logging/logger'; import {hasBeenProcessed} from '../packages/build_marker'; import {NgccConfiguration} from '../packages/configuration'; -import {EntryPoint, EntryPointJsonProperty, getEntryPointInfo} from '../packages/entry_point'; +import {EntryPoint, EntryPointJsonProperty, INVALID_ENTRY_POINT, NO_ENTRY_POINT, getEntryPointInfo} from '../packages/entry_point'; import {PathMappings} from '../utils'; import {EntryPointFinder} from './interface'; import {getBasePaths} from './utils'; @@ -78,20 +78,26 @@ export class TargetedEntryPointFinder implements EntryPointFinder { private processNextPath(): void { const path = this.unprocessedPaths.shift() !; const entryPoint = this.getEntryPoint(path); - if (entryPoint !== null && entryPoint.compiledByAngular) { - this.unsortedEntryPoints.set(entryPoint.path, entryPoint); - const deps = this.resolver.getEntryPointDependencies(entryPoint); - deps.dependencies.forEach(dep => { - if (!this.unsortedEntryPoints.has(dep)) { - this.unprocessedPaths.push(dep); - } - }); + if (entryPoint === null || !entryPoint.compiledByAngular) { + return; } + this.unsortedEntryPoints.set(entryPoint.path, entryPoint); + const deps = this.resolver.getEntryPointDependencies(entryPoint); + deps.dependencies.forEach(dep => { + if (!this.unsortedEntryPoints.has(dep)) { + this.unprocessedPaths.push(dep); + } + }); } private getEntryPoint(entryPointPath: AbsoluteFsPath): EntryPoint|null { const packagePath = this.computePackagePath(entryPointPath); - return getEntryPointInfo(this.fs, this.config, this.logger, packagePath, entryPointPath); + const entryPoint = + getEntryPointInfo(this.fs, this.config, this.logger, packagePath, entryPointPath); + if (entryPoint === NO_ENTRY_POINT || entryPoint === INVALID_ENTRY_POINT) { + return null; + } + return entryPoint; } /** diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 30ed8e27cd..ea64e05ed2 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point.ts @@ -75,41 +75,77 @@ export type EntryPointJsonProperty = Exclude { ]); }); + it('should handle try to process nested node_modules of non Angular packages', () => { + const basePath = _Abs('/nested_node_modules/node_modules'); + loadTestFiles([ + ...createPackage(basePath, 'outer', ['inner'], false), + ...createPackage(_Abs(`${basePath}/outer/node_modules`), 'inner', undefined, false), + ]); + + const finder = new DirectoryWalkerEntryPointFinder( + fs, config, logger, resolver, _Abs('/nested_node_modules/node_modules'), undefined); + const spy = spyOn(finder, 'walkDirectoryForEntryPoints').and.callThrough(); + const {entryPoints} = finder.findEntryPoints(); + expect(spy.calls.allArgs()).toEqual([ + [_Abs(basePath)], + [_Abs(`${basePath}/outer`)], + ]); + + expect(entryPoints).toEqual([]); + }); + + it('should not try to process deeply nested folders of non TypeScript packages', () => { + const basePath = _Abs('/namespaced/node_modules'); + loadTestFiles([ + ...createNonTsPackage(_Abs(`${basePath}/@schematics`), 'angular'), + { + name: _Abs(`${basePath}/@schematics/angular/src/nested/index.js`), + contents: 'index', + }, + ]); + + const finder = + new DirectoryWalkerEntryPointFinder(fs, config, logger, resolver, basePath, undefined); + const spy = spyOn(finder, 'walkDirectoryForEntryPoints').and.callThrough(); + const {entryPoints} = finder.findEntryPoints(); + expect(spy.calls.allArgs()).toEqual([ + [_Abs(basePath)], + [_Abs(`${basePath}/@schematics`)], + [_Abs(`${basePath}/@schematics/angular`)], + ]); + + expect(entryPoints).toEqual([]); + }); + + it('should not try to process nested node_modules of non TypeScript packages', () => { + const basePath = _Abs('/namespaced/node_modules'); + loadTestFiles([ + ...createNonTsPackage(_Abs(`${basePath}/@schematics`), 'angular'), + ...createNonTsPackage(_Abs(`${basePath}/@schematics/angular/node_modules`), 'test'), + { + name: _Abs(`${basePath}/@schematics/angular/src/nested/index.js`), + contents: 'index', + }, + ]); + + const finder = + new DirectoryWalkerEntryPointFinder(fs, config, logger, resolver, basePath, undefined); + const spy = spyOn(finder, 'walkDirectoryForEntryPoints').and.callThrough(); + const {entryPoints} = finder.findEntryPoints(); + expect(spy.calls.allArgs()).toEqual([ + [_Abs(basePath)], + [_Abs(`${basePath}/@schematics`)], + [_Abs(`${basePath}/@schematics/angular`)], + ]); + + expect(entryPoints).toEqual([]); + }); it('should handle dependencies via pathMappings', () => { const basePath = _Abs('/path_mapped/node_modules'); @@ -195,8 +260,9 @@ runInEachFileSystem(() => { }); function createPackage( - basePath: AbsoluteFsPath, packageName: string, deps: string[] = []): TestFile[] { - return [ + basePath: AbsoluteFsPath, packageName: string, deps: string[] = [], + isCompiledByAngular = true): TestFile[] { + const files: TestFile[] = [ { name: _Abs(`${basePath}/${packageName}/package.json`), contents: JSON.stringify({ @@ -205,8 +271,29 @@ runInEachFileSystem(() => { }) }, { + name: _Abs(`${basePath}/${packageName}/fesm2015/${packageName}.js`), + contents: deps.map((dep, i) => `import * as i${i} from '${dep}';`).join('\n'), + }, + ]; + + if (isCompiledByAngular) { + files.push({ name: _Abs(`${basePath}/${packageName}/${packageName}.metadata.json`), contents: 'metadata info' + }); + } + + return files; + } + + function createNonTsPackage( + basePath: AbsoluteFsPath, packageName: string, deps: string[] = []): TestFile[] { + return [ + { + name: _Abs(`${basePath}/${packageName}/package.json`), + contents: JSON.stringify({ + fesm2015: `./fesm2015/${packageName}.js`, + }) }, { name: _Abs(`${basePath}/${packageName}/fesm2015/${packageName}.js`), diff --git a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts index b187b905fa..9b51b87d42 100644 --- a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts @@ -10,7 +10,7 @@ import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem} from '../../../ import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {NgccConfiguration} from '../../src/packages/configuration'; -import {EntryPoint, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat, getEntryPointInfo} from '../../src/packages/entry_point'; +import {EntryPoint, INVALID_ENTRY_POINT, NO_ENTRY_POINT, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat, getEntryPointInfo} from '../../src/packages/entry_point'; import {MockLogger} from '../helpers/mock_logger'; runInEachFileSystem(() => { @@ -55,7 +55,7 @@ runInEachFileSystem(() => { }); }); - it('should return null if configured to ignore the specified entry-point', () => { + it('should return `NO_ENTRY_POINT` if configured to ignore the specified entry-point', () => { loadTestFiles([ { name: _('/project/node_modules/some_package/valid_entry_point/package.json'), @@ -75,7 +75,7 @@ runInEachFileSystem(() => { const entryPoint = getEntryPointInfo( fs, config, new MockLogger(), SOME_PACKAGE, _('/project/node_modules/some_package/valid_entry_point')); - expect(entryPoint).toBe(null); + expect(entryPoint).toBe(NO_ENTRY_POINT); }); it('should override the properties on package.json if the entry-point is configured', () => { @@ -116,7 +116,7 @@ runInEachFileSystem(() => { }); }); - it('should return null if there is no package.json at the entry-point path', () => { + it('should return `NO_ENTRY_POINT` if there is no package.json at the entry-point path', () => { loadTestFiles([ { name: _( @@ -128,7 +128,7 @@ runInEachFileSystem(() => { const entryPoint = getEntryPointInfo( fs, config, new MockLogger(), SOME_PACKAGE, _('/project/node_modules/some_package/missing_package_json')); - expect(entryPoint).toBe(null); + expect(entryPoint).toBe(NO_ENTRY_POINT); }); it('should return a configured entry-point if there is no package.json at the entry-point path', @@ -165,26 +165,27 @@ runInEachFileSystem(() => { }); - it('should return null if there is no typings or types field in the package.json', () => { - loadTestFiles([ - { - name: _('/project/node_modules/some_package/missing_typings/package.json'), - contents: createPackageJson('missing_typings', {excludes: ['typings']}) - }, - { - name: - _('/project/node_modules/some_package/missing_typings/missing_typings.metadata.json'), - contents: 'some meta data' - }, - ]); - const config = new NgccConfiguration(fs, _('/project')); - const entryPoint = getEntryPointInfo( - fs, config, new MockLogger(), SOME_PACKAGE, - _('/project/node_modules/some_package/missing_typings')); - expect(entryPoint).toBe(null); - }); + it('should return `INVALID_ENTRY_POINT` if there is no typings or types field in the package.json', + () => { + loadTestFiles([ + { + name: _('/project/node_modules/some_package/missing_typings/package.json'), + contents: createPackageJson('missing_typings', {excludes: ['typings']}) + }, + { + name: _( + '/project/node_modules/some_package/missing_typings/missing_typings.metadata.json'), + contents: 'some meta data' + }, + ]); + const config = new NgccConfiguration(fs, _('/project')); + const entryPoint = getEntryPointInfo( + fs, config, new MockLogger(), SOME_PACKAGE, + _('/project/node_modules/some_package/missing_typings')); + expect(entryPoint).toBe(INVALID_ENTRY_POINT); + }); - it('should return null if the typings or types field is not a string in the package.json', + it('should return `INVALID_ENTRY_POINT` if the typings or types field is not a string in the package.json', () => { loadTestFiles([ { @@ -201,7 +202,7 @@ runInEachFileSystem(() => { const entryPoint = getEntryPointInfo( fs, config, new MockLogger(), SOME_PACKAGE, _('/project/node_modules/some_package/typings_array')); - expect(entryPoint).toBe(null); + expect(entryPoint).toBe(INVALID_ENTRY_POINT); }); for (let prop of SUPPORTED_FORMAT_PROPERTIES) { @@ -358,7 +359,7 @@ runInEachFileSystem(() => { }); }); - it('should return null if the package.json is not valid JSON', () => { + it('should return `INVALID_ENTRY_POINT` if the package.json is not valid JSON', () => { loadTestFiles([ // package.json might not be a valid JSON // for example, @schematics/angular contains a package.json blueprint @@ -372,7 +373,7 @@ runInEachFileSystem(() => { const entryPoint = getEntryPointInfo( fs, config, new MockLogger(), SOME_PACKAGE, _('/project/node_modules/some_package/unexpected_symbols')); - expect(entryPoint).toBe(null); + expect(entryPoint).toBe(INVALID_ENTRY_POINT); }); }); @@ -391,9 +392,13 @@ runInEachFileSystem(() => { contents: createPackageJson('valid_entry_point') }]); const config = new NgccConfiguration(fs, _('/project')); - entryPoint = getEntryPointInfo( + const result = getEntryPointInfo( fs, config, new MockLogger(), SOME_PACKAGE, - _('/project/node_modules/some_package/valid_entry_point')) !; + _('/project/node_modules/some_package/valid_entry_point')); + if (result === NO_ENTRY_POINT || result === INVALID_ENTRY_POINT) { + return fail(`Expected an entry point but got ${result}`); + } + entryPoint = result; }); it('should return `esm2015` format for `fesm2015` property', diff --git a/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts b/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts index f3772d8727..89a0e22d7d 100644 --- a/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts +++ b/packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts @@ -9,7 +9,7 @@ import {FileSystem, absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_s import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {NgccConfiguration} from '../../src/packages/configuration'; -import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, getEntryPointInfo} from '../../src/packages/entry_point'; +import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, INVALID_ENTRY_POINT, NO_ENTRY_POINT, getEntryPointInfo} from '../../src/packages/entry_point'; import {EntryPointBundle, makeEntryPointBundle} from '../../src/packages/entry_point_bundle'; import {FileWriter} from '../../src/writing/file_writer'; import {NewEntryPointFileWriter} from '../../src/writing/new_entry_point_file_writer'; @@ -103,8 +103,12 @@ runInEachFileSystem(() => { fs = getFileSystem(); fileWriter = new NewEntryPointFileWriter(fs, new DirectPackageJsonUpdater(fs)); const config = new NgccConfiguration(fs, _('/')); - entryPoint = getEntryPointInfo( + const result = getEntryPointInfo( fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test')) !; + if (result === NO_ENTRY_POINT || result === INVALID_ENTRY_POINT) { + return fail(`Expected an entry point but got ${result}`); + } + entryPoint = result; esm5bundle = makeTestBundle(fs, entryPoint, 'module', 'esm5'); esm2015bundle = makeTestBundle(fs, entryPoint, 'es2015', 'esm2015'); }); @@ -239,8 +243,12 @@ runInEachFileSystem(() => { fs = getFileSystem(); fileWriter = new NewEntryPointFileWriter(fs, new DirectPackageJsonUpdater(fs)); const config = new NgccConfiguration(fs, _('/')); - entryPoint = getEntryPointInfo( + const result = getEntryPointInfo( fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test/a')) !; + if (result === NO_ENTRY_POINT || result === INVALID_ENTRY_POINT) { + return fail(`Expected an entry point but got ${result}`); + } + entryPoint = result; esm5bundle = makeTestBundle(fs, entryPoint, 'module', 'esm5'); esm2015bundle = makeTestBundle(fs, entryPoint, 'es2015', 'esm2015'); }); @@ -364,8 +372,12 @@ runInEachFileSystem(() => { fs = getFileSystem(); fileWriter = new NewEntryPointFileWriter(fs, new DirectPackageJsonUpdater(fs)); const config = new NgccConfiguration(fs, _('/')); - entryPoint = getEntryPointInfo( + const result = getEntryPointInfo( fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test/b')) !; + if (result === NO_ENTRY_POINT || result === INVALID_ENTRY_POINT) { + return fail(`Expected an entry point but got ${result}`); + } + entryPoint = result; esm5bundle = makeTestBundle(fs, entryPoint, 'module', 'esm5'); esm2015bundle = makeTestBundle(fs, entryPoint, 'es2015', 'esm2015'); });