mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
refactor(compiler-cli): ensure proper TS incremental program re-use w/ signal inputs (#53521)
Whenever a signal input is captured in a type check block, we will insert an import. This will change the import graph so that the full TypeScript program cannot be structurally re-used. We can fix this trivially by ensuring the import graph remains stable, by always generating an import to e.g. `@angular/core`. This fixes the issue nicely for type-check block files. A test verifies this. For inline code, such as TCB inline or the type constructors inline, this fix is not applicable because we would change user-input source files, adding new edges that would not exist for subsequent builds- causing the program to be not re-used completely. One idea was to rely on the existing edge that can be assumed to exist for directive code files. This is true technically, but in practice TS does not deduplicate imports- so our new namespace import when referencing our symbols will invalidate the re-use. We will address this in a follow-up. There are a couple of options, such as working with the TS team, updating the existing edge, or inlining our helpers as well. PR Close #53521
This commit is contained in:
parent
e1acc6086d
commit
e6db288ffd
4 changed files with 147 additions and 3 deletions
|
|
@ -21,7 +21,7 @@ import {ImportManager, translateExpression, translateType} from '../../translato
|
|||
*/
|
||||
export class ReferenceEmitEnvironment {
|
||||
constructor(
|
||||
protected importManager: ImportManager, protected refEmitter: ReferenceEmitter,
|
||||
readonly importManager: ImportManager, protected refEmitter: ReferenceEmitter,
|
||||
readonly reflector: ReflectionHost, protected contextFile: ts.SourceFile) {}
|
||||
|
||||
canReferenceType(
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler';
|
||||
import {AbsoluteSourceSpan, ParseSourceSpan, R3Identifiers} from '@angular/compiler';
|
||||
import ts from 'typescript';
|
||||
|
||||
import {ClassDeclaration, ReflectionHost} from '../../../../src/ngtsc/reflection';
|
||||
|
|
@ -18,6 +18,20 @@ import {hasIgnoreForDiagnosticsMarker, readSpanComment} from './comments';
|
|||
import {ReferenceEmitEnvironment} from './reference_emit_environment';
|
||||
import {TypeParameterEmitter} from './type_parameter_emitter';
|
||||
|
||||
/**
|
||||
* External modules that always should exist for type check blocks and
|
||||
* file hosting inline type constructors.
|
||||
*
|
||||
* Importing the modules in preparation helps ensuring a stable import graph
|
||||
* that would not degrade TypeScript's incremental program structure re-use.
|
||||
*/
|
||||
const TCB_FILE_IMPORT_GRAPH_PREPARE_MODULES = [
|
||||
// Imports may be added for signal input checking. We wouldn't want to change the
|
||||
// import graph for incremental compilations when suddenly a signal input is used,
|
||||
// or removed.
|
||||
R3Identifiers.InputSignalBrandWriteType.moduleName,
|
||||
];
|
||||
|
||||
/**
|
||||
* Adapter interface which allows the template type-checking diagnostics code to interpret offsets
|
||||
* in a TCB and map them back to original locations in the template.
|
||||
|
|
@ -174,6 +188,17 @@ function getTemplateId(
|
|||
}) as TemplateId || null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure imports for certain external modules that should always
|
||||
* exist are generated. These are ensures to exist to avoid frequent
|
||||
* import graph changes whenever e.g. a signal input is introduced in user code.
|
||||
*/
|
||||
export function ensureTypeCheckFilePreparationImports(env: ReferenceEmitEnvironment): void {
|
||||
for (const moduleName of TCB_FILE_IMPORT_GRAPH_PREPARE_MODULES) {
|
||||
env.importManager.generateNamespaceImport(moduleName);
|
||||
}
|
||||
}
|
||||
|
||||
export function checkIfGenericTypeBoundsCanBeEmitted(
|
||||
node: ClassDeclaration<ts.ClassDeclaration>, reflector: ReflectionHost,
|
||||
env: ReferenceEmitEnvironment): boolean {
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import {TypeCheckBlockMetadata, TypeCheckingConfig} from '../api';
|
|||
import {DomSchemaChecker} from './dom';
|
||||
import {Environment} from './environment';
|
||||
import {OutOfBandDiagnosticRecorder} from './oob';
|
||||
import {ensureTypeCheckFilePreparationImports} from './tcb_util';
|
||||
import {generateTypeCheckBlock, TcbGenericContextBehavior} from './type_check_block';
|
||||
|
||||
|
||||
|
|
@ -52,6 +53,12 @@ export class TypeCheckFile extends Environment {
|
|||
}
|
||||
|
||||
render(removeComments: boolean): string {
|
||||
// NOTE: We are conditionally adding imports whenever we discover signal inputs. This has a
|
||||
// risk of changing the import graph of the TypeScript program, degrading incremental program
|
||||
// re-use due to program structure changes. For type check block files, we are ensuring an
|
||||
// import to e.g. `@angular/core` always exists to guarantee a stable graph.
|
||||
ensureTypeCheckFilePreparationImports(this);
|
||||
|
||||
let source: string = this.importManager.getAllImports(this.contextFile.fileName)
|
||||
.map(i => `import * as ${i.qualifier.text} from '${i.specifier}';`)
|
||||
.join('\n') +
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import ts from 'typescript';
|
|||
|
||||
import {absoluteFrom} from '../../src/ngtsc/file_system';
|
||||
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
|
||||
import {loadStandardTestFiles} from '../../src/ngtsc/testing';
|
||||
import {expectCompleteReuse, loadStandardTestFiles} from '../../src/ngtsc/testing';
|
||||
|
||||
import {NgtscTestEnvironment} from './env';
|
||||
|
||||
|
|
@ -1392,5 +1392,117 @@ runInEachFileSystem(() => {
|
|||
`Can't bind to 'grandgrandparentA' since it isn't a known property of 'div'.`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('program re-use', () => {
|
||||
it('should completely re-use structure when the first signal input is introduced', () => {
|
||||
env.tsconfig({strictTemplates: true});
|
||||
env.write('dir.ts', `
|
||||
import {Directive, Input} from '@angular/core';
|
||||
|
||||
@Directive({
|
||||
selector: '[dir]',
|
||||
})
|
||||
export class Dir {
|
||||
@Input() dir: string = '';
|
||||
}
|
||||
`);
|
||||
env.write('cmp.ts', `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
selector: 'test-cmp',
|
||||
template: '<div [dir]="foo"></div>',
|
||||
})
|
||||
export class Cmp {
|
||||
foo = 'foo';
|
||||
}
|
||||
`);
|
||||
env.write('mod.ts', `
|
||||
import {NgModule} from '@angular/core';
|
||||
import {Cmp} from './cmp';
|
||||
import {Dir} from './dir';
|
||||
|
||||
@NgModule({
|
||||
declarations: [Cmp, Dir],
|
||||
})
|
||||
export class Mod {}
|
||||
`);
|
||||
env.driveMain();
|
||||
|
||||
// introduce the signal input.
|
||||
env.write('dir.ts', `
|
||||
import {Directive, input} from '@angular/core';
|
||||
|
||||
@Directive({
|
||||
selector: '[dir]',
|
||||
})
|
||||
export class Dir {
|
||||
dir = input.required<string>();
|
||||
}
|
||||
`);
|
||||
env.driveMain();
|
||||
|
||||
expectCompleteReuse(env.getTsProgram());
|
||||
expectCompleteReuse(env.getReuseTsProgram());
|
||||
});
|
||||
|
||||
// TODO(devversion): look into fixing this for inline TCB and inline type ctors.
|
||||
xit('should completely re-use structure when an inline constructor generic directive starts using input signals',
|
||||
() => {
|
||||
env.tsconfig({strictTemplates: true});
|
||||
env.write('dir.ts', `
|
||||
import {Directive, Input} from '@angular/core';
|
||||
|
||||
class SomeNonExportedClass {}
|
||||
|
||||
@Directive({
|
||||
selector: '[dir]',
|
||||
})
|
||||
export class Dir<T extends SomeNonExportedClass> {
|
||||
@Input() dir: T|undefined;
|
||||
}
|
||||
`);
|
||||
env.write('cmp.ts', `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
selector: 'test-cmp',
|
||||
template: '<div [dir]="foo"></div>',
|
||||
})
|
||||
export class Cmp {
|
||||
foo = 'foo';
|
||||
}
|
||||
`);
|
||||
env.write('mod.ts', `
|
||||
import {NgModule} from '@angular/core';
|
||||
import {Cmp} from './cmp';
|
||||
import {Dir} from './dir';
|
||||
|
||||
@NgModule({
|
||||
declarations: [Cmp, Dir],
|
||||
})
|
||||
export class Mod {}
|
||||
`);
|
||||
env.driveMain();
|
||||
|
||||
// turn the input into a signal input- causing a new import.
|
||||
env.write('dir.ts', `
|
||||
import {Directive, input} from '@angular/core';
|
||||
|
||||
class SomeNonExportedClass {}
|
||||
|
||||
@Directive({
|
||||
selector: '[dir]',
|
||||
})
|
||||
export class Dir<T extends SomeNonExportedClass> {
|
||||
dir = input.required<T>();
|
||||
}
|
||||
`);
|
||||
env.driveMain();
|
||||
|
||||
expectCompleteReuse(env.getTsProgram());
|
||||
expectCompleteReuse(env.getReuseTsProgram());
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue