Skip to content

Commit 0314fd4

Browse files
alxhubSplaktar
authored andcommitted
refactor(compiler-cli): allow overriding templates in the type checker (angular#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 angular#38058 PR Close angular#38105
1 parent 3f2257f commit 0314fd4

File tree

4 files changed

+221
-4
lines changed

4 files changed

+221
-4
lines changed

packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {TmplAstNode} from '@angular/compiler';
10-
9+
import {ParseError, TmplAstNode} from '@angular/compiler';
1110
import * as ts from 'typescript';
1211

1312
/**
@@ -24,6 +23,21 @@ import * as ts from 'typescript';
2423
* query, depending on the method either `null` will be returned or an error will be thrown.
2524
*/
2625
export interface TemplateTypeChecker {
26+
/**
27+
* Clear all overrides and return the template type-checker to the original input program state.
28+
*/
29+
resetOverrides(): void;
30+
31+
/**
32+
* Provide a new template string that will be used in place of the user-defined template when
33+
* checking or operating on the given component.
34+
*
35+
* The compiler will parse this template for diagnostics, and will return any parsing errors if it
36+
* is not valid. If the template cannot be parsed correctly, no override will occur.
37+
*/
38+
overrideComponentTemplate(component: ts.ClassDeclaration, template: string):
39+
{nodes: TmplAstNode[], errors?: ParseError[]};
40+
2741
/**
2842
* Get all `ts.Diagnostic`s currently available for the given `ts.SourceFile`.
2943
*

packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {ParseError, parseTemplate, TmplAstNode} from '@angular/compiler';
910
import * as ts from 'typescript';
1011

1112
import {absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../../file_system';
@@ -14,7 +15,7 @@ import {IncrementalBuild} from '../../incremental/api';
1415
import {ReflectionHost} from '../../reflection';
1516
import {isShim} from '../../shims';
1617
import {getSourceFileOrNull} from '../../util/src/typescript';
17-
import {OptimizeFor, ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
18+
import {OptimizeFor, ProgramTypeCheckAdapter, TemplateId, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
1819

1920
import {InliningMode, ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
2021
import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
@@ -37,6 +38,47 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
3738
private compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
3839
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>) {}
3940

41+
resetOverrides(): void {
42+
for (const fileRecord of this.state.values()) {
43+
if (fileRecord.templateOverrides !== null) {
44+
fileRecord.templateOverrides = null;
45+
fileRecord.shimData.clear();
46+
fileRecord.isComplete = false;
47+
}
48+
}
49+
}
50+
51+
overrideComponentTemplate(component: ts.ClassDeclaration, template: string):
52+
{nodes: TmplAstNode[], errors?: ParseError[]} {
53+
const {nodes, errors} = parseTemplate(template, 'override.html', {
54+
preserveWhitespaces: true,
55+
leadingTriviaChars: [],
56+
});
57+
58+
if (errors !== undefined) {
59+
return {nodes, errors};
60+
}
61+
62+
const filePath = absoluteFromSourceFile(component.getSourceFile());
63+
64+
const fileRecord = this.getFileData(filePath);
65+
const id = fileRecord.sourceManager.getTemplateId(component);
66+
67+
if (fileRecord.templateOverrides === null) {
68+
fileRecord.templateOverrides = new Map();
69+
}
70+
71+
fileRecord.templateOverrides.set(id, nodes);
72+
73+
// Clear data for the shim in question, so it'll be regenerated on the next request.
74+
const shimFile = this.typeCheckingStrategy.shimPathForComponent(component);
75+
fileRecord.shimData.delete(shimFile);
76+
fileRecord.isComplete = false;
77+
this.isComplete = false;
78+
79+
return {nodes};
80+
}
81+
4082
/**
4183
* Retrieve type-checking diagnostics from the given `ts.SourceFile` using the most recent
4284
* type-checking program.
@@ -106,6 +148,10 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
106148
const sfPath = absoluteFromSourceFile(sf);
107149
if (this.state.has(sfPath)) {
108150
const existingResults = this.state.get(sfPath)!;
151+
if (existingResults.templateOverrides !== null) {
152+
// Cannot adopt prior results if template overrides have been requested.
153+
return;
154+
}
109155

110156
if (existingResults.isComplete) {
111157
// All data for this file has already been generated, so no need to adopt anything.
@@ -114,7 +160,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
114160
}
115161

116162
const previousResults = this.priorBuild.priorTypeCheckingResultsFor(sf);
117-
if (previousResults === null || !previousResults.isComplete) {
163+
if (previousResults === null || !previousResults.isComplete ||
164+
previousResults.templateOverrides !== null) {
118165
return;
119166
}
120167

@@ -214,6 +261,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
214261
if (!this.state.has(path)) {
215262
this.state.set(path, {
216263
hasInlines: false,
264+
templateOverrides: null,
217265
sourceManager: new TemplateSourceManager(),
218266
isComplete: false,
219267
shimData: new Map(),
@@ -248,6 +296,11 @@ export interface FileTypeCheckingData {
248296
*/
249297
sourceManager: TemplateSourceManager;
250298

299+
/**
300+
* Map of template overrides applied to any components in this input file.
301+
*/
302+
templateOverrides: Map<TemplateId, TmplAstNode[]>|null;
303+
251304
/**
252305
* Data for each shim generated from this input file.
253306
*
@@ -280,6 +333,20 @@ class WholeProgramTypeCheckingHost implements TypeCheckingHost {
280333
return !fileData.shimData.has(shimPath);
281334
}
282335

336+
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TmplAstNode[]|null {
337+
const fileData = this.impl.getFileData(sfPath);
338+
if (fileData.templateOverrides === null) {
339+
return null;
340+
}
341+
342+
const templateId = fileData.sourceManager.getTemplateId(node);
343+
if (fileData.templateOverrides.has(templateId)) {
344+
return fileData.templateOverrides.get(templateId)!;
345+
}
346+
347+
return null;
348+
}
349+
283350
recordShimData(sfPath: AbsoluteFsPath, data: ShimTypeCheckingData): void {
284351
const fileData = this.impl.getFileData(sfPath);
285352
fileData.shimData.set(data.path, data);
@@ -324,6 +391,20 @@ class SingleFileTypeCheckingHost implements TypeCheckingHost {
324391
return !this.fileData.shimData.has(shimPath);
325392
}
326393

394+
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TmplAstNode[]|null {
395+
this.assertPath(sfPath);
396+
if (this.fileData.templateOverrides === null) {
397+
return null;
398+
}
399+
400+
const templateId = this.fileData.sourceManager.getTemplateId(node);
401+
if (this.fileData.templateOverrides.has(templateId)) {
402+
return this.fileData.templateOverrides.get(templateId)!;
403+
}
404+
405+
return null;
406+
}
407+
327408
recordShimData(sfPath: AbsoluteFsPath, data: ShimTypeCheckingData): void {
328409
this.assertPath(sfPath);
329410

packages/compiler-cli/src/ngtsc/typecheck/src/context.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ export interface TypeCheckingHost {
102102
*/
103103
shouldCheckComponent(node: ts.ClassDeclaration): boolean;
104104

105+
/**
106+
* Check if the given component has had its template overridden, and retrieve the new template
107+
* nodes if so.
108+
*/
109+
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TmplAstNode[]|null;
110+
105111
/**
106112
* Report data from a shim generated from the given input file path.
107113
*/
@@ -175,6 +181,13 @@ export class TypeCheckContextImpl implements TypeCheckContext {
175181
return;
176182
}
177183

184+
const sfPath = absoluteFromSourceFile(ref.node.getSourceFile());
185+
186+
const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node);
187+
if (overrideTemplate !== null) {
188+
template = overrideTemplate;
189+
}
190+
178191
// Accumulate a list of any directives which could not have type constructors generated due to
179192
// unsupported inlining operations.
180193
let missingInlines: ClassDeclaration[] = [];

packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,5 +209,114 @@ runInEachFileSystem(() => {
209209
expect(absoluteFromSourceFile(diags[0].relatedInformation![0].file!)).toBe(dirFile);
210210
});
211211
});
212+
213+
describe('template overrides', () => {
214+
it('should override a simple template', () => {
215+
const fileName = absoluteFrom('/main.ts');
216+
const {program, templateTypeChecker} = setup([{
217+
fileName,
218+
templates: {'Cmp': '<div></div>'},
219+
}]);
220+
221+
const sf = getSourceFileOrError(program, fileName);
222+
const cmp = getClass(sf, 'Cmp');
223+
224+
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
225+
expect(tcbReal.getText()).toContain('div');
226+
227+
templateTypeChecker.overrideComponentTemplate(cmp, '<span></span>');
228+
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp);
229+
expect(tcbOverridden).not.toBeNull();
230+
expect(tcbOverridden!.getText()).not.toContain('div');
231+
expect(tcbOverridden!.getText()).toContain('span');
232+
});
233+
234+
it('should clear overrides on request', () => {
235+
const fileName = absoluteFrom('/main.ts');
236+
const {program, templateTypeChecker} = setup([{
237+
fileName,
238+
templates: {'Cmp': '<div></div>'},
239+
}]);
240+
241+
const sf = getSourceFileOrError(program, fileName);
242+
const cmp = getClass(sf, 'Cmp');
243+
244+
templateTypeChecker.overrideComponentTemplate(cmp, '<span></span>');
245+
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp)!;
246+
expect(tcbOverridden.getText()).not.toContain('div');
247+
expect(tcbOverridden.getText()).toContain('span');
248+
249+
templateTypeChecker.resetOverrides();
250+
251+
// The template should be back to the original, which has <div> and not <span>.
252+
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
253+
expect(tcbReal.getText()).toContain('div');
254+
expect(tcbReal.getText()).not.toContain('span');
255+
});
256+
257+
it('should override a template and make use of previously unused directives', () => {
258+
const fileName = absoluteFrom('/main.ts');
259+
const dirFile = absoluteFrom('/dir.ts');
260+
const {program, templateTypeChecker} = setup(
261+
[
262+
{
263+
fileName,
264+
source: `export class Cmp {}`,
265+
templates: {'Cmp': '<div></div>'},
266+
declarations: [{
267+
name: 'TestDir',
268+
selector: '[dir]',
269+
file: dirFile,
270+
type: 'directive',
271+
}]
272+
},
273+
{
274+
fileName: dirFile,
275+
source: `export class TestDir {}`,
276+
templates: {},
277+
}
278+
],
279+
{inlining: false});
280+
const sf = getSourceFileOrError(program, fileName);
281+
const cmp = getClass(sf, 'Cmp');
282+
283+
// TestDir is initially unused. Note that this checks the entire text of the ngtypecheck
284+
// file, to ensure it captures not just the TCB function but also any inline type
285+
// constructors.
286+
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
287+
expect(tcbReal.getSourceFile().text).not.toContain('TestDir');
288+
289+
templateTypeChecker.overrideComponentTemplate(cmp, '<div dir></div>');
290+
291+
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp);
292+
expect(tcbOverridden).not.toBeNull();
293+
expect(tcbOverridden!.getSourceFile().text).toContain('TestDir');
294+
});
295+
296+
it('should not invalidate other templates when an override is requested', () => {
297+
const file1 = absoluteFrom('/file1.ts');
298+
const file2 = absoluteFrom('/file2.ts');
299+
const {program, templateTypeChecker, programStrategy} = setup([
300+
{fileName: file1, templates: {'Cmp1': '<div></div>'}},
301+
{fileName: file2, templates: {'Cmp2': '<span></span>'}}
302+
]);
303+
304+
const cmp1 = getClass(getSourceFileOrError(program, file1), 'Cmp1');
305+
const cmp2 = getClass(getSourceFileOrError(program, file2), 'Cmp2');
306+
307+
// To test this scenario, Cmp1's type check block will be captured, then Cmp2's template
308+
// will be overridden. Cmp1's type check block should not change as a result.
309+
const originalTcb = templateTypeChecker.getTypeCheckBlock(cmp1)!;
310+
311+
templateTypeChecker.overrideComponentTemplate(cmp2, '<p></p>');
312+
313+
// Trigger generation of the TCB for Cmp2.
314+
templateTypeChecker.getTypeCheckBlock(cmp2);
315+
316+
// Verify that Cmp1's TCB has not changed.
317+
const currentTcb = templateTypeChecker.getTypeCheckBlock(cmp1)!;
318+
expect(currentTcb).toBe(originalTcb);
319+
});
320+
});
212321
});
213322
});

0 commit comments

Comments
 (0)