fix(ivy): ngcc - do not consider builtin NodeJS modules as missing (#31872)

ngcc analyzes the dependency structure of the entrypoints it needs to
process, as the compilation of entrypoints is ordering sensitive: any
dependent upon entrypoint must be compiled before its dependees. As part
of the analysis of the dependency graph, it is detected when a
dependency of entrypoint is not installed, in which case that entrypoint
will be marked as ignored.

For libraries that work with Angular Universal to run in NodeJS, imports
into builtin NodeJS modules can be present. ngcc's dependency analyzer
can only resolve imports within the TypeScript compilation, which
builtin modules are not part of. Therefore, such imports would
erroneously cause the entrypoint to become ignored.

This commit fixes the problem by taking the NodeJS builtins into account
when dealing with missing imports.

Fixes #31522

PR Close #31872
This commit is contained in:
JoostK 2019-07-27 21:37:04 +02:00 committed by Alex Rickabaugh
parent b70746a113
commit 57e15fc08b
2 changed files with 19 additions and 2 deletions

View File

@ -12,6 +12,8 @@ import {Logger} from '../logging/logger';
import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, getEntryPointFormat} from '../packages/entry_point'; import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, getEntryPointFormat} from '../packages/entry_point';
import {DependencyHost, DependencyInfo} from './dependency_host'; import {DependencyHost, DependencyInfo} from './dependency_host';
const builtinNodeJsModules = new Set<string>(require('module').builtinModules);
/** /**
* Holds information about entry points that are removed because * Holds information about entry points that are removed because
* they have dependencies that are missing (directly or transitively). * they have dependencies that are missing (directly or transitively).
@ -128,10 +130,12 @@ export class DependencyResolver {
angularEntryPoints.forEach(entryPoint => { angularEntryPoints.forEach(entryPoint => {
const {dependencies, missing, deepImports} = this.getEntryPointDependencies(entryPoint); const {dependencies, missing, deepImports} = this.getEntryPointDependencies(entryPoint);
if (missing.size > 0) { const missingDependencies = Array.from(missing).filter(dep => !builtinNodeJsModules.has(dep));
if (missingDependencies.length > 0) {
// This entry point has dependencies that are missing // This entry point has dependencies that are missing
// so remove it from the graph. // so remove it from the graph.
removeNodes(entryPoint, Array.from(missing)); removeNodes(entryPoint, missingDependencies);
} else { } else {
dependencies.forEach(dependencyPath => { dependencies.forEach(dependencyPath => {
if (!graph.hasNode(entryPoint.path)) { if (!graph.hasNode(entryPoint.path)) {

View File

@ -192,6 +192,19 @@ runInEachFileSystem(() => {
expect(sorted.invalidEntryPoints[0].missingDependencies).toEqual(['/missing']); expect(sorted.invalidEntryPoints[0].missingDependencies).toEqual(['/missing']);
}); });
it('should not consider builtin NodeJS modules as missing dependency', () => {
spyOn(host, 'findDependencies').and.callFake(createFakeComputeDependencies({
[_('/first/index.js')]: {resolved: [], missing: ['fs']},
}));
const entryPoints = [first];
let sorted: SortedEntryPointsInfo;
sorted = resolver.sortEntryPointsByDependency(entryPoints, first);
expect(sorted.entryPoints).toEqual([first]);
expect(sorted.invalidEntryPoints).toEqual([]);
expect(sorted.ignoredDependencies).toEqual([]);
});
it('should use the appropriate DependencyHost for each entry-point', () => { it('should use the appropriate DependencyHost for each entry-point', () => {
const esm5Host = new EsmDependencyHost(fs, moduleResolver); const esm5Host = new EsmDependencyHost(fs, moduleResolver);
const esm2015Host = new EsmDependencyHost(fs, moduleResolver); const esm2015Host = new EsmDependencyHost(fs, moduleResolver);