refactor(compiler-cli): allow overriding templates in the type checker (#38105)

This commit adds an `overrideComponentTemplate` operation to the template
type-checker. This operation changes the template used during template
type-checking operations.

Overriding a template causes any previous work for it to be discarded, and
the template type-checking engine will regenerate the TCB for that template
on the next request.

This operation can be used by a consumer such as the language service to
get rapid feedback or diagnostics as the user is editing a template file,
without the need for a full incremental build iteration.

Closes #38058

PR Close #38105
This commit is contained in:
Alex Rickabaugh 2020-07-15 16:35:06 -07:00 committed by Misko Hevery
parent de14b2c670
commit 3c0424e7e0
4 changed files with 221 additions and 4 deletions

View File

@ -6,8 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {TmplAstNode} from '@angular/compiler';
import {ParseError, TmplAstNode} from '@angular/compiler';
import * as ts from 'typescript';
/**
@ -24,6 +23,21 @@ import * as ts from 'typescript';
* query, depending on the method either `null` will be returned or an error will be thrown.
*/
export interface TemplateTypeChecker {
/**
* Clear all overrides and return the template type-checker to the original input program state.
*/
resetOverrides(): void;
/**
* Provide a new template string that will be used in place of the user-defined template when
* checking or operating on the given component.
*
* The compiler will parse this template for diagnostics, and will return any parsing errors if it
* is not valid. If the template cannot be parsed correctly, no override will occur.
*/
overrideComponentTemplate(component: ts.ClassDeclaration, template: string):
{nodes: TmplAstNode[], errors?: ParseError[]};
/**
* Get all `ts.Diagnostic`s currently available for the given `ts.SourceFile`.
*

View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ParseError, parseTemplate, TmplAstNode} from '@angular/compiler';
import * as ts from 'typescript';
import {absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../../file_system';
@ -14,7 +15,7 @@ import {IncrementalBuild} from '../../incremental/api';
import {ReflectionHost} from '../../reflection';
import {isShim} from '../../shims';
import {getSourceFileOrNull} from '../../util/src/typescript';
import {OptimizeFor, ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
import {OptimizeFor, ProgramTypeCheckAdapter, TemplateId, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
import {InliningMode, ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
@ -37,6 +38,47 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
private compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>) {}
resetOverrides(): void {
for (const fileRecord of this.state.values()) {
if (fileRecord.templateOverrides !== null) {
fileRecord.templateOverrides = null;
fileRecord.shimData.clear();
fileRecord.isComplete = false;
}
}
}
overrideComponentTemplate(component: ts.ClassDeclaration, template: string):
{nodes: TmplAstNode[], errors?: ParseError[]} {
const {nodes, errors} = parseTemplate(template, 'override.html', {
preserveWhitespaces: true,
leadingTriviaChars: [],
});
if (errors !== undefined) {
return {nodes, errors};
}
const filePath = absoluteFromSourceFile(component.getSourceFile());
const fileRecord = this.getFileData(filePath);
const id = fileRecord.sourceManager.getTemplateId(component);
if (fileRecord.templateOverrides === null) {
fileRecord.templateOverrides = new Map();
}
fileRecord.templateOverrides.set(id, nodes);
// Clear data for the shim in question, so it'll be regenerated on the next request.
const shimFile = this.typeCheckingStrategy.shimPathForComponent(component);
fileRecord.shimData.delete(shimFile);
fileRecord.isComplete = false;
this.isComplete = false;
return {nodes};
}
/**
* Retrieve type-checking diagnostics from the given `ts.SourceFile` using the most recent
* type-checking program.
@ -106,6 +148,10 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
const sfPath = absoluteFromSourceFile(sf);
if (this.state.has(sfPath)) {
const existingResults = this.state.get(sfPath)!;
if (existingResults.templateOverrides !== null) {
// Cannot adopt prior results if template overrides have been requested.
return;
}
if (existingResults.isComplete) {
// All data for this file has already been generated, so no need to adopt anything.
@ -114,7 +160,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
}
const previousResults = this.priorBuild.priorTypeCheckingResultsFor(sf);
if (previousResults === null || !previousResults.isComplete) {
if (previousResults === null || !previousResults.isComplete ||
previousResults.templateOverrides !== null) {
return;
}
@ -214,6 +261,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
if (!this.state.has(path)) {
this.state.set(path, {
hasInlines: false,
templateOverrides: null,
sourceManager: new TemplateSourceManager(),
isComplete: false,
shimData: new Map(),
@ -248,6 +296,11 @@ export interface FileTypeCheckingData {
*/
sourceManager: TemplateSourceManager;
/**
* Map of template overrides applied to any components in this input file.
*/
templateOverrides: Map<TemplateId, TmplAstNode[]>|null;
/**
* Data for each shim generated from this input file.
*
@ -280,6 +333,20 @@ class WholeProgramTypeCheckingHost implements TypeCheckingHost {
return !fileData.shimData.has(shimPath);
}
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TmplAstNode[]|null {
const fileData = this.impl.getFileData(sfPath);
if (fileData.templateOverrides === null) {
return null;
}
const templateId = fileData.sourceManager.getTemplateId(node);
if (fileData.templateOverrides.has(templateId)) {
return fileData.templateOverrides.get(templateId)!;
}
return null;
}
recordShimData(sfPath: AbsoluteFsPath, data: ShimTypeCheckingData): void {
const fileData = this.impl.getFileData(sfPath);
fileData.shimData.set(data.path, data);
@ -324,6 +391,20 @@ class SingleFileTypeCheckingHost implements TypeCheckingHost {
return !this.fileData.shimData.has(shimPath);
}
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TmplAstNode[]|null {
this.assertPath(sfPath);
if (this.fileData.templateOverrides === null) {
return null;
}
const templateId = this.fileData.sourceManager.getTemplateId(node);
if (this.fileData.templateOverrides.has(templateId)) {
return this.fileData.templateOverrides.get(templateId)!;
}
return null;
}
recordShimData(sfPath: AbsoluteFsPath, data: ShimTypeCheckingData): void {
this.assertPath(sfPath);

View File

@ -102,6 +102,12 @@ export interface TypeCheckingHost {
*/
shouldCheckComponent(node: ts.ClassDeclaration): boolean;
/**
* Check if the given component has had its template overridden, and retrieve the new template
* nodes if so.
*/
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TmplAstNode[]|null;
/**
* Report data from a shim generated from the given input file path.
*/
@ -175,6 +181,13 @@ export class TypeCheckContextImpl implements TypeCheckContext {
return;
}
const sfPath = absoluteFromSourceFile(ref.node.getSourceFile());
const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node);
if (overrideTemplate !== null) {
template = overrideTemplate;
}
// Accumulate a list of any directives which could not have type constructors generated due to
// unsupported inlining operations.
let missingInlines: ClassDeclaration[] = [];

View File

@ -209,5 +209,114 @@ runInEachFileSystem(() => {
expect(absoluteFromSourceFile(diags[0].relatedInformation![0].file!)).toBe(dirFile);
});
});
describe('template overrides', () => {
it('should override a simple template', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {'Cmp': '<div></div>'},
}]);
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbReal.getText()).toContain('div');
templateTypeChecker.overrideComponentTemplate(cmp, '<span></span>');
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp);
expect(tcbOverridden).not.toBeNull();
expect(tcbOverridden!.getText()).not.toContain('div');
expect(tcbOverridden!.getText()).toContain('span');
});
it('should clear overrides on request', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {'Cmp': '<div></div>'},
}]);
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
templateTypeChecker.overrideComponentTemplate(cmp, '<span></span>');
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbOverridden.getText()).not.toContain('div');
expect(tcbOverridden.getText()).toContain('span');
templateTypeChecker.resetOverrides();
// The template should be back to the original, which has <div> and not <span>.
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbReal.getText()).toContain('div');
expect(tcbReal.getText()).not.toContain('span');
});
it('should override a template and make use of previously unused directives', () => {
const fileName = absoluteFrom('/main.ts');
const dirFile = absoluteFrom('/dir.ts');
const {program, templateTypeChecker} = setup(
[
{
fileName,
source: `export class Cmp {}`,
templates: {'Cmp': '<div></div>'},
declarations: [{
name: 'TestDir',
selector: '[dir]',
file: dirFile,
type: 'directive',
}]
},
{
fileName: dirFile,
source: `export class TestDir {}`,
templates: {},
}
],
{inlining: false});
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
// TestDir is initially unused. Note that this checks the entire text of the ngtypecheck
// file, to ensure it captures not just the TCB function but also any inline type
// constructors.
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbReal.getSourceFile().text).not.toContain('TestDir');
templateTypeChecker.overrideComponentTemplate(cmp, '<div dir></div>');
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp);
expect(tcbOverridden).not.toBeNull();
expect(tcbOverridden!.getSourceFile().text).toContain('TestDir');
});
it('should not invalidate other templates when an override is requested', () => {
const file1 = absoluteFrom('/file1.ts');
const file2 = absoluteFrom('/file2.ts');
const {program, templateTypeChecker, programStrategy} = setup([
{fileName: file1, templates: {'Cmp1': '<div></div>'}},
{fileName: file2, templates: {'Cmp2': '<span></span>'}}
]);
const cmp1 = getClass(getSourceFileOrError(program, file1), 'Cmp1');
const cmp2 = getClass(getSourceFileOrError(program, file2), 'Cmp2');
// To test this scenario, Cmp1's type check block will be captured, then Cmp2's template
// will be overridden. Cmp1's type check block should not change as a result.
const originalTcb = templateTypeChecker.getTypeCheckBlock(cmp1)!;
templateTypeChecker.overrideComponentTemplate(cmp2, '<p></p>');
// Trigger generation of the TCB for Cmp2.
templateTypeChecker.getTypeCheckBlock(cmp2);
// Verify that Cmp1's TCB has not changed.
const currentTcb = templateTypeChecker.getTypeCheckBlock(cmp1)!;
expect(currentTcb).toBe(originalTcb);
});
});
});
});