refactor(compiler-cli): ngcc - track non-Angular entry-points (#29643)

Previously we completely ignored entry-points that had not been
compiled with Angular, since we do not need to compile them
with ngcc. But this makes it difficult to reason about dependencies
between entry-points that were compiled with Angular and those that
were not.

Now we do track these non-Angular compiled entry-points but they
are marked as `compiledByAngular: false`.

PR Close #29643
This commit is contained in:
Pete Bacon Darwin 2019-04-28 20:47:56 +01:00 committed by Andrew Kushnir
parent c2cf500da9
commit 321da5cc83
4 changed files with 69 additions and 38 deletions

View File

@ -83,8 +83,12 @@ export class DependencyResolver {
let sortedEntryPointNodes: string[]; let sortedEntryPointNodes: string[];
if (target) { if (target) {
sortedEntryPointNodes = graph.dependenciesOf(target.path); if (target.compiledByAngular) {
sortedEntryPointNodes.push(target.path); sortedEntryPointNodes = graph.dependenciesOf(target.path);
sortedEntryPointNodes.push(target.path);
} else {
sortedEntryPointNodes = [];
}
} else { } else {
sortedEntryPointNodes = graph.overallOrder(); sortedEntryPointNodes = graph.overallOrder();
} }
@ -107,11 +111,13 @@ export class DependencyResolver {
const ignoredDependencies: IgnoredDependency[] = []; const ignoredDependencies: IgnoredDependency[] = [];
const graph = new DepGraph<EntryPoint>(); const graph = new DepGraph<EntryPoint>();
// Add the entry points to the graph as nodes const angularEntryPoints = entryPoints.filter(entryPoint => entryPoint.compiledByAngular);
entryPoints.forEach(entryPoint => graph.addNode(entryPoint.path, entryPoint));
// Add the Angular compiled entry points to the graph as nodes
angularEntryPoints.forEach(entryPoint => graph.addNode(entryPoint.path, entryPoint));
// Now add the dependencies between them // Now add the dependencies between them
entryPoints.forEach(entryPoint => { angularEntryPoints.forEach(entryPoint => {
const entryPointPath = getEntryPointPath(entryPoint); const entryPointPath = getEntryPointPath(entryPoint);
const {dependencies, missing, deepImports} = this.host.computeDependencies(entryPointPath); const {dependencies, missing, deepImports} = this.host.computeDependencies(entryPointPath);

View File

@ -33,6 +33,8 @@ export interface EntryPoint {
path: AbsoluteFsPath; path: AbsoluteFsPath;
/** The path to a typings (.d.ts) file for this entry-point. */ /** The path to a typings (.d.ts) file for this entry-point. */
typings: AbsoluteFsPath; typings: AbsoluteFsPath;
/** Is this EntryPoint compiled with the Angular View Engine compiler? */
compiledByAngular: boolean;
} }
interface PackageJsonFormatProperties { interface PackageJsonFormatProperties {
@ -89,9 +91,6 @@ export function getEntryPointInfo(
// Also there must exist a `metadata.json` file next to the typings entry-point. // Also there must exist a `metadata.json` file next to the typings entry-point.
const metadataPath = const metadataPath =
path.resolve(entryPointPath, typings.replace(/\.d\.ts$/, '') + '.metadata.json'); path.resolve(entryPointPath, typings.replace(/\.d\.ts$/, '') + '.metadata.json');
if (!fs.existsSync(metadataPath)) {
return null;
}
const entryPointInfo: EntryPoint = { const entryPointInfo: EntryPoint = {
name: entryPointPackageJson.name, name: entryPointPackageJson.name,
@ -99,6 +98,7 @@ export function getEntryPointInfo(
package: packagePath, package: packagePath,
path: entryPointPath, path: entryPointPath,
typings: AbsoluteFsPath.from(path.resolve(entryPointPath, typings)), typings: AbsoluteFsPath.from(path.resolve(entryPointPath, typings)),
compiledByAngular: fs.existsSync(metadataPath),
}; };
return entryPointInfo; return entryPointInfo;

View File

@ -21,18 +21,38 @@ describe('DependencyResolver', () => {
resolver = new DependencyResolver(new MockLogger(), host); resolver = new DependencyResolver(new MockLogger(), host);
}); });
describe('sortEntryPointsByDependency()', () => { describe('sortEntryPointsByDependency()', () => {
const first = { path: _('/first'), packageJson: {esm5: 'index.ts'} } as EntryPoint; const first = {
const second = { path: _('/second'), packageJson: {esm2015: 'sub/index.ts'} } as EntryPoint; path: _('/first'),
const third = { path: _('/third'), packageJson: {esm5: 'index.ts'} } as EntryPoint; packageJson: {esm5: './index.js'},
const fourth = { path: _('/fourth'), packageJson: {esm2015: 'sub2/index.ts'} } as EntryPoint; compiledByAngular: true
const fifth = { path: _('/fifth'), packageJson: {esm5: 'index.ts'} } as EntryPoint; } as EntryPoint;
const second = {
path: _('/second'),
packageJson: {esm2015: './sub/index.js'},
compiledByAngular: true
} as EntryPoint;
const third = {
path: _('/third'),
packageJson: {fesm5: './index.js'},
compiledByAngular: true
} as EntryPoint;
const fourth = {
path: _('/fourth'),
packageJson: {fesm2015: './sub2/index.js'},
compiledByAngular: true
} as EntryPoint;
const fifth = {
path: _('/fifth'),
packageJson: {module: './index.js'},
compiledByAngular: true
} as EntryPoint;
const dependencies = { const dependencies = {
[_('/first/index.ts')]: {resolved: [second.path, third.path, '/ignored-1'], missing: []}, [_('/first/index.js')]: {resolved: [second.path, third.path, '/ignored-1'], missing: []},
[_('/second/sub/index.ts')]: {resolved: [third.path, fifth.path], missing: []}, [_('/second/sub/index.js')]: {resolved: [third.path, fifth.path], missing: []},
[_('/third/index.ts')]: {resolved: [fourth.path, '/ignored-2'], missing: []}, [_('/third/index.js')]: {resolved: [fourth.path, '/ignored-2'], missing: []},
[_('/fourth/sub2/index.ts')]: {resolved: [fifth.path], missing: []}, [_('/fourth/sub2/index.js')]: {resolved: [fifth.path], missing: []},
[_('/fifth/index.ts')]: {resolved: [], missing: []}, [_('/fifth/index.js')]: {resolved: [], missing: []},
}; };
it('should order the entry points by their dependency on each other', () => { it('should order the entry points by their dependency on each other', () => {
@ -43,8 +63,8 @@ describe('DependencyResolver', () => {
it('should remove entry-points that have missing direct dependencies', () => { it('should remove entry-points that have missing direct dependencies', () => {
spyOn(host, 'computeDependencies').and.callFake(createFakeComputeDependencies({ spyOn(host, 'computeDependencies').and.callFake(createFakeComputeDependencies({
[_('/first/index.ts')]: {resolved: [], missing: ['/missing']}, [_('/first/index.js')]: {resolved: [], missing: ['/missing']},
[_('/second/sub/index.ts')]: {resolved: [], missing: []}, [_('/second/sub/index.js')]: {resolved: [], missing: []},
})); }));
const result = resolver.sortEntryPointsByDependency([first, second]); const result = resolver.sortEntryPointsByDependency([first, second]);
expect(result.entryPoints).toEqual([second]); expect(result.entryPoints).toEqual([second]);
@ -55,9 +75,9 @@ describe('DependencyResolver', () => {
it('should remove entry points that depended upon an invalid entry-point', () => { it('should remove entry points that depended upon an invalid entry-point', () => {
spyOn(host, 'computeDependencies').and.callFake(createFakeComputeDependencies({ spyOn(host, 'computeDependencies').and.callFake(createFakeComputeDependencies({
[_('/first/index.ts')]: {resolved: [second.path], missing: []}, [_('/first/index.js')]: {resolved: [second.path], missing: []},
[_('/second/sub/index.ts')]: {resolved: [], missing: ['/missing']}, [_('/second/sub/index.js')]: {resolved: [], missing: ['/missing']},
[_('/third/index.ts')]: {resolved: [], missing: []}, [_('/third/index.js')]: {resolved: [], missing: []},
})); }));
// Note that we will process `first` before `second`, which has the missing dependency. // Note that we will process `first` before `second`, which has the missing dependency.
const result = resolver.sortEntryPointsByDependency([first, second, third]); const result = resolver.sortEntryPointsByDependency([first, second, third]);
@ -70,9 +90,9 @@ describe('DependencyResolver', () => {
it('should remove entry points that will depend upon an invalid entry-point', () => { it('should remove entry points that will depend upon an invalid entry-point', () => {
spyOn(host, 'computeDependencies').and.callFake(createFakeComputeDependencies({ spyOn(host, 'computeDependencies').and.callFake(createFakeComputeDependencies({
[_('/first/index.ts')]: {resolved: [second.path], missing: []}, [_('/first/index.js')]: {resolved: [second.path], missing: []},
[_('/second/sub/index.ts')]: {resolved: [], missing: ['/missing']}, [_('/second/sub/index.js')]: {resolved: [], missing: ['/missing']},
[_('/third/index.ts')]: {resolved: [], missing: []}, [_('/third/index.js')]: {resolved: [], missing: []},
})); }));
// Note that we will process `first` after `second`, which has the missing dependency. // Note that we will process `first` after `second`, which has the missing dependency.
const result = resolver.sortEntryPointsByDependency([second, first, third]); const result = resolver.sortEntryPointsByDependency([second, first, third]);
@ -85,7 +105,7 @@ describe('DependencyResolver', () => {
it('should error if the entry point does not have either the esm5 nor esm2015 formats', () => { it('should error if the entry point does not have either the esm5 nor esm2015 formats', () => {
expect(() => resolver.sortEntryPointsByDependency([ expect(() => resolver.sortEntryPointsByDependency([
{ path: '/first', packageJson: {} } as EntryPoint { path: '/first', packageJson: {}, compiledByAngular: true } as EntryPoint
])).toThrowError(`There is no format with import statements in '/first' entry-point.`); ])).toThrowError(`There is no format with import statements in '/first' entry-point.`);
}); });

View File

@ -30,6 +30,7 @@ describe('getEntryPointInfo()', () => {
path: _('/some_package/valid_entry_point'), path: _('/some_package/valid_entry_point'),
typings: _(`/some_package/valid_entry_point/valid_entry_point.d.ts`), typings: _(`/some_package/valid_entry_point/valid_entry_point.d.ts`),
packageJson: loadPackageJson('/some_package/valid_entry_point'), packageJson: loadPackageJson('/some_package/valid_entry_point'),
compiledByAngular: true,
}); });
}); });
@ -45,17 +46,19 @@ describe('getEntryPointInfo()', () => {
expect(entryPoint).toBe(null); expect(entryPoint).toBe(null);
}); });
it('should return null if there is no esm2015 nor fesm2015 field in the package.json', () => { it('should return an object with `compiledByAngular` set to false if there is no metadata.json file next to the typing file',
const entryPoint = () => {
getEntryPointInfo(new MockLogger(), SOME_PACKAGE, _('/some_package/missing_esm2015')); const entryPoint =
expect(entryPoint).toBe(null); getEntryPointInfo(new MockLogger(), SOME_PACKAGE, _('/some_package/missing_metadata'));
}); expect(entryPoint).toEqual({
name: 'some-package/missing_metadata',
it('should return null if there is no metadata.json file next to the typing file', () => { package: SOME_PACKAGE,
const entryPoint = path: _('/some_package/missing_metadata'),
getEntryPointInfo(new MockLogger(), SOME_PACKAGE, _('/some_package/missing_metadata.json')); typings: _(`/some_package/missing_metadata/missing_metadata.d.ts`),
expect(entryPoint).toBe(null); packageJson: loadPackageJson('/some_package/missing_metadata'),
}); compiledByAngular: false,
});
});
it('should work if the typings field is named `types', () => { it('should work if the typings field is named `types', () => {
const entryPoint = getEntryPointInfo( const entryPoint = getEntryPointInfo(
@ -66,6 +69,7 @@ describe('getEntryPointInfo()', () => {
path: _('/some_package/types_rather_than_typings'), path: _('/some_package/types_rather_than_typings'),
typings: _(`/some_package/types_rather_than_typings/types_rather_than_typings.d.ts`), typings: _(`/some_package/types_rather_than_typings/types_rather_than_typings.d.ts`),
packageJson: loadPackageJson('/some_package/types_rather_than_typings'), packageJson: loadPackageJson('/some_package/types_rather_than_typings'),
compiledByAngular: true,
}); });
}); });
@ -78,6 +82,7 @@ describe('getEntryPointInfo()', () => {
path: _('/some_package/material_style'), path: _('/some_package/material_style'),
typings: _(`/some_package/material_style/material_style.d.ts`), typings: _(`/some_package/material_style/material_style.d.ts`),
packageJson: JSON.parse(readFileSync('/some_package/material_style/package.json', 'utf8')), packageJson: JSON.parse(readFileSync('/some_package/material_style/package.json', 'utf8')),
compiledByAngular: true,
}); });
}); });