iSharkFly-Docs/angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

perf(compiler-cli): detect semantic changes and their effect on an incremental rebuild (#40947)

Browse Source 
In Angular programs, changing a file may require other files to be
emitted as well due to implicit NgModule dependencies. For example, if
the selector of a directive is changed then all components that have
that directive in their compilation scope need to be recompiled, as the
change of selector may affect the directive matching results.

Until now, the compiler solved this problem using a single dependency
graph. The implicit NgModule dependencies were represented in this
graph, such that a changed file would correctly also cause other files
to be re-emitted. This approach is limited in a few ways:

1. The file dependency graph is used to determine whether it is safe to
   reuse the analysis data of an Angular decorated class. This analysis
   data is invariant to unrelated changes to the NgModule scope, but
   because the single dependency graph also tracked the implicit
   NgModule dependencies the compiler had to consider analysis data as
   stale far more often than necessary.
2. It is typical for a change to e.g. a directive to not affect its
   public API—its selector, inputs, outputs, or exportAs clause—in which
   case there is no need to re-emit all declarations in scope, as their
   compilation output wouldn't have changed.

This commit implements a mechanism by which the compiler is able to
determine the impact of a change by comparing it to the prior
compilation. To achieve this, a new graph is maintained that tracks all
public API information of all Angular decorated symbols. During an
incremental compilation this information is compared to the information
that was captured in the most recently succeeded compilation. This
determines the exact impact of the changes to the public API, which
is then used to determine which files need to be re-emitted.

Note that the file dependency graph remains, as it is still used to
track the dependencies of analysis data. This graph does no longer track
the implicit NgModule dependencies, which allows for better reuse of
analysis data.

These changes also fix a bug where template type-checking would fail to
incorporate changes made to a transitive base class of a
directive/component. This used to be a problem because transitive base
classes were not recorded as a transitive dependency in the file
dependency graph, such that prior type-check blocks would erroneously
be reused.

This commit also fixes an incorrectness where a change to a declaration
in NgModule `A` would not cause the declarations in NgModules that
import from NgModule `A` to be re-emitted. This was intentionally
incorrect as otherwise the performance of incremental rebuilds would
have been far worse. This is no longer a concern, as the compiler is now
able to only re-emit when actually necessary.

Fixes #34867
Fixes #40635
Closes #40728

PR Close #40947
This commit is contained in:
JoostK
2020-11-20 21:18:46 +01:00
committed by Andrew Kushnir
parent 8c062493a0
commit fed6a7ce7d
43 changed files with 5253 additions and 378 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/compiler-cli/test/ngtsc/env.ts
+6 -1
View File
@@ -161,7 +161,9 @@ export class NgtscTestEnvironment {
const absFilePath = this.fs.resolve(this.basePath, fileName);
if (this.multiCompileHostExt !== null) {
this.multiCompileHostExt.invalidate(absFilePath);
this.changedResources!.add(absFilePath);
if (!fileName.endsWith('.ts')) {
this.changedResources!.add(absFilePath);
}
}
this.fs.ensureDir(this.fs.dirname(absFilePath));
this.fs.writeFile(absFilePath, content);
@@ -173,6 +175,9 @@ export class NgtscTestEnvironment {
throw new Error(`Not caching files - call enableMultipleCompilations()`);
}
this.multiCompileHostExt.invalidate(absFilePath);
if (!fileName.endsWith('.ts')) {
this.changedResources!.add(absFilePath);
}
}
tsconfig(
packages/compiler-cli/test/ngtsc/incremental_error_spec.ts
+41 -24
View File
@@ -239,12 +239,13 @@ runInEachFileSystem(() => {
export class TargetCmp {}
`);
env.write('module.ts', `
import {NgModule} from '@angular/core';
import {NgModule, NO_ERRORS_SCHEMA} from '@angular/core';
import {TargetCmp} from './target';
import {TestCmp} from './test';
@NgModule({
declarations: [TestCmp, TargetCmp],
schemas: [NO_ERRORS_SCHEMA],
})
export class Module {}
`);
@@ -268,7 +269,7 @@ runInEachFileSystem(() => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({selector: 'test-cmp', template: '...'})
@Component({selector: 'test-cmp-fixed', template: '...'})
export class TestCmp {}
`);
@@ -283,7 +284,7 @@ runInEachFileSystem(() => {
'/module.js',
// Because TargetCmp also belongs to the same module, it should be re-emitted since
// TestCmp's elector may have changed.
// TestCmp's selector was changed.
'/target.js',
]);
});
@@ -329,7 +330,7 @@ runInEachFileSystem(() => {
env.write('a.ts', `
import {Component} from '@angular/core';
@Component({selector: 'test-cmp', template: '...'})
@Component({selector: 'test-cmp', template: '<div dir></div>'})
export class CmpA {}
`);
env.write('b.ts', `
@@ -357,17 +358,16 @@ runInEachFileSystem(() => {
export class Module {}
`);
env.write('lib.ts', `
import {Component, NgModule} from '@angular/core';
import {Directive, NgModule} from '@angular/core';
@Component({
selector: 'lib-cmp',
template: '...',
@Directive({
selector: '[dir]',
})
export class LibCmp {}
export class LibDir {}
@NgModule({
declarations: [LibCmp],
exports: [LibCmp],
declarations: [LibDir],
exports: [LibDir],
})
export class LibModule {}
`);
@@ -378,17 +378,27 @@ runInEachFileSystem(() => {
// Introduce the error in LibModule
env.write('lib.ts', `
import {Component, NgModule} from '@angular/core';
import {Directive, NgModule} from '@angular/core';
@Component({
selector: 'lib-cmp',
template: '...',
@Directive({
selector: '[dir]',
})
export class LibCmp {}
export class LibDir {}
@Directive({
selector: '[dir]',
})
export class NewDir {}
@NgModule({
declarations: [LibCmp],
exports: [LibCmp],
declarations: [NewDir],
})
export class NewModule {}
@NgModule({
declarations: [LibDir],
imports: [NewModule],
exports: [LibDir, NewModule],
})
export class LibModule // missing braces
`);
@@ -407,9 +417,13 @@ runInEachFileSystem(() => {
})
export class LibCmp {}
@NgModule({})
export class NewModule {}
@NgModule({
declarations: [LibCmp],
exports: [LibCmp],
imports: [NewModule],
exports: [LibCmp, NewModule],
})
export class LibModule {}
`);
@@ -417,9 +431,10 @@ runInEachFileSystem(() => {
env.driveMain();
expectToHaveWritten([
// Both CmpA and CmpB should be re-emitted.
// CmpA should be re-emitted as `NewModule` was added since the successful emit, which added
// `NewDir` as a matching directive to CmpA. Alternatively, CmpB should not be re-emitted
// as it does not use the newly added directive.
'/a.js',
'/b.js',
// So should the module itself.
'/module.js',
@@ -468,8 +483,7 @@ runInEachFileSystem(() => {
'/other.js',
'/a.js',
// Bcause they depend on a.ts
'/b.js',
// Because they depend on a.ts
'/module.js',
]);
});
@@ -512,7 +526,10 @@ runInEachFileSystem(() => {
'/other.js',
// Because a.html changed
'/a.js', '/module.js',
'/a.js',
// module.js should not be re-emitted, as it is not affected by the change and its remote
// scope is unaffected.
// b.js and module.js should not be re-emitted, because specifically when tracking
// resource dependencies, the compiler knows that a change to a resource file only affects
packages/compiler-cli/test/ngtsc/incremental_semantic_changes_spec.ts
+2314
View File
File diff suppressed because it is too large Load Diff
packages/compiler-cli/test/ngtsc/incremental_spec.ts
+52 -21
View File
@@ -154,14 +154,20 @@ runInEachFileSystem(() => {
setupFooBarProgram(env);
// Pretend a change was made to BarDir.
env.invalidateCachedFile('bar_directive.ts');
env.write('bar_directive.ts', `
import {Directive} from '@angular/core';
@Directive({selector: '[barr]'})
export class BarDir {}
`);
env.driveMain();
let written = env.getFilesWrittenSinceLastFlush();
expect(written).toContain('/bar_directive.js');
expect(written).toContain('/bar_component.js');
expect(written).toContain('/bar_module.js');
expect(written).toContain('/foo_component.js');
expect(written).not.toContain('/foo_component.js'); // BarDir is not exported by BarModule,
// so upstream NgModule is not affected
expect(written).not.toContain('/foo_pipe.js');
expect(written).not.toContain('/foo_module.js');
});
@@ -178,7 +184,7 @@ runInEachFileSystem(() => {
env.write('component2.ts', `
import {Component} from '@angular/core';
@Component({selector: 'cmp2', template: 'cmp2'})
@Component({selector: 'cmp2', template: '<cmp></cmp>'})
export class Cmp2 {}
`);
env.write('dep.ts', `
@@ -197,36 +203,43 @@ runInEachFileSystem(() => {
export class MyPipe {}
`);
env.write('module.ts', `
import {NgModule} from '@angular/core';
import {NgModule, NO_ERRORS_SCHEMA} from '@angular/core';
import {Cmp1} from './component1';
import {Cmp2} from './component2';
import {Dir} from './directive';
import {MyPipe} from './pipe';
@NgModule({declarations: [Cmp1, Cmp2, Dir, MyPipe]})
@NgModule({declarations: [Cmp1, Cmp2, Dir, MyPipe], schemas: [NO_ERRORS_SCHEMA]})
export class Mod {}
`);
env.driveMain();
// Pretend a change was made to 'dep'. Since this may affect the NgModule scope, like it does
// here if the selector is updated, all components in the module scope need to be recompiled.
// Pretend a change was made to 'dep'. Since the selector is updated this affects the NgModule
// scope, so all components in the module scope need to be recompiled.
env.flushWrittenFileTracking();
env.invalidateCachedFile('dep.ts');
env.write('dep.ts', `
export const SELECTOR = 'cmp_updated';
`);
env.driveMain();
const written = env.getFilesWrittenSinceLastFlush();
expect(written).not.toContain('/directive.js');
expect(written).not.toContain('/pipe.js');
expect(written).not.toContain('/module.js');
expect(written).toContain('/component1.js');
expect(written).toContain('/component2.js');
expect(written).toContain('/dep.js');
expect(written).toContain('/module.js');
});
it('should rebuild components where their NgModule declared dependencies have changed', () => {
setupFooBarProgram(env);
// Pretend a change was made to FooPipe.
env.invalidateCachedFile('foo_pipe.ts');
// Rename the pipe so components that use it need to be recompiled.
env.write('foo_pipe.ts', `
import {Pipe} from '@angular/core';
@Pipe({name: 'foo_changed'})
export class FooPipe {}
`);
env.driveMain();
const written = env.getFilesWrittenSinceLastFlush();
expect(written).not.toContain('/bar_directive.js');
@@ -240,15 +253,25 @@ runInEachFileSystem(() => {
it('should rebuild components where their NgModule has changed', () => {
setupFooBarProgram(env);
// Pretend a change was made to FooPipe.
env.invalidateCachedFile('foo_module.ts');
// Pretend a change was made to FooModule.
env.write('foo_module.ts', `
import {NgModule} from '@angular/core';
import {FooCmp} from './foo_component';
import {FooPipe} from './foo_pipe';
import {BarModule} from './bar_module';
@NgModule({
declarations: [FooCmp], // removed FooPipe
imports: [BarModule],
})
export class FooModule {}
`);
env.driveMain();
const written = env.getFilesWrittenSinceLastFlush();
expect(written).not.toContain('/bar_directive.js');
expect(written).not.toContain('/bar_component.js');
expect(written).not.toContain('/bar_module.js');
expect(written).not.toContain('/foo_pipe.js');
expect(written).toContain('/foo_component.js');
expect(written).toContain('/foo_pipe.js');
expect(written).toContain('/foo_module.js');
});
@@ -396,7 +419,7 @@ runInEachFileSystem(() => {
expect(env.getContents('cmp.js')).not.toContain('DepDir');
});
it('should rebuild only a Component (but with the correct CompilationScope) and its module if its template has changed',
it('should rebuild only a Component (but with the correct CompilationScope) if its template has changed',
() => {
setupFooBarProgram(env);
@@ -407,9 +430,7 @@ runInEachFileSystem(() => {
const written = env.getFilesWrittenSinceLastFlush();
expect(written).not.toContain('/bar_directive.js');
expect(written).toContain('/bar_component.js');
// /bar_module.js should also be re-emitted, because remote scoping of BarComponent might
// have been affected.
expect(written).toContain('/bar_module.js');
expect(written).not.toContain('/bar_module.js');
expect(written).not.toContain('/foo_component.js');
expect(written).not.toContain('/foo_pipe.js');
expect(written).not.toContain('/foo_module.js');
@@ -764,7 +785,10 @@ runInEachFileSystem(() => {
import {Component} from '@angular/core';
import {fooSelector} from './foo_selector';
@Component({selector: fooSelector, template: 'foo'})
@Component({
selector: fooSelector,
template: '{{ 1 | foo }}'
})
export class FooCmp {}
`);
env.write('foo_pipe.ts', `
@@ -796,14 +820,21 @@ runInEachFileSystem(() => {
@Directive({selector: '[bar]'})
export class BarDir {}
`);
env.write('bar_pipe.ts', `
import {Pipe} from '@angular/core';
@Pipe({name: 'foo'})
export class BarPipe {}
`);
env.write('bar_module.ts', `
import {NgModule} from '@angular/core';
import {BarCmp} from './bar_component';
import {BarDir} from './bar_directive';
import {BarPipe} from './bar_pipe';
@NgModule({
declarations: [BarCmp, BarDir],
exports: [BarCmp],
declarations: [BarCmp, BarDir, BarPipe],
exports: [BarCmp, BarPipe],
})
export class BarModule {}
`);
packages/compiler-cli/test/ngtsc/incremental_typecheck_spec.ts
+1396
View File
File diff suppressed because it is too large Load Diff