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

refactor(compiler-cli): remove the overrideComponentTemplate API (#40585) 

The `TemplateTypeChecker.overrideComponentTemplate` operation was originally
conceived as a "fast path" for the Language Service to react to a template
change without needing to go through a full incremental compilation step. It
served this purpose until the previous commit, which switches the LS to use
the new resource-only incremental change operation provided by `NgCompiler`.

`overrideComponentTemplate` is now no longer utilized, and is known to have
several hard-to-overcome issues that prevent it from being useful in any
other situations. As such, this commit removes it entirely.

PR Close #40585
This commit is contained in:
Alex Rickabaugh 2021-01-26 15:52:38 -08:00 committed by Misko Hevery
parent e3bd23c915
commit a3b0864428
8 changed files with 9 additions and 351 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/compiler-cli/src/ngtsc/annotations/src/component.ts
View File

@ -881,9 +881,6 @@ export class ComponentDecoratorHandler implements
//
// In order to guarantee the correctness of diagnostics, templates are parsed a second time
// with the above options set to preserve source mappings.
//
// Note: template parse options should be aligned with `template_target_spec.ts` and
// `TemplateTypeCheckerImpl.overrideComponentTemplate`.
const {nodes: diagNodes} = parseTemplate(templateStr, template.sourceMapUrl, {
preserveWhitespaces: true,

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

@ -29,29 +29,11 @@ import {DirectiveSymbol, ElementSymbol, ShimLocation, Symbol, TemplateSymbol} fr
* 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;
/**
* Retrieve the template in use for the given component.
*
* If the template has been overridden via `overrideComponentTemplate`, this will retrieve the
* overridden template nodes.
*/
getTemplate(component: ts.ClassDeclaration): TmplAstNode[]|null;
/**
* 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[]|null};
/**
* Get all `ts.Diagnostic`s currently available for the given `ts.SourceFile`.
*

110
packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
View File

@ -20,7 +20,7 @@ import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion,
import {TemplateDiagnostic} from '../diagnostics';
import {CompletionEngine} from './completion';
import {InliningMode, ShimTypeCheckingData, TemplateData, TemplateOverride, TypeCheckContextImpl, TypeCheckingHost} from './context';
import {InliningMode, ShimTypeCheckingData, TemplateData, TypeCheckContextImpl, TypeCheckingHost} from './context';
import {shouldReportDiagnostic, translateDiagnostic} from './diagnostics';
import {TemplateSourceManager} from './source';
import {findTypeCheckBlock, getTemplateMapping, TemplateSourceResolver} from './tcb_util';
@ -56,10 +56,9 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
/**
* Stores directives and pipes that are in scope for each component.
*
* Unlike other caches, the scope of a component is not affected by its template, so this
* cache does not need to be invalidate if the template is overridden. It will be destroyed when
* the `ts.Program` changes and the `TemplateTypeCheckerImpl` as a whole is destroyed and
* replaced.
* Unlike other caches, the scope of a component is not affected by its template. It will be
* destroyed when the `ts.Program` changes and the `TemplateTypeCheckerImpl` as a whole is
* destroyed and replaced.
*/
private scopeCache = new Map<ts.ClassDeclaration, ScopeData>();
@ -67,10 +66,9 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
* Stores potential element tags for each component (a union of DOM tags as well as directive
* tags).
*
* Unlike other caches, the scope of a component is not affected by its template, so this
* cache does not need to be invalidate if the template is overridden. It will be destroyed when
* the `ts.Program` changes and the `TemplateTypeCheckerImpl` as a whole is destroyed and
* replaced.
* Unlike other caches, the scope of a component is not affected by its template. It will be
* destroyed when the `ts.Program` changes and the `TemplateTypeCheckerImpl` as a whole is
* destroyed and replaced.
*/
private elementTagCache = new Map<ts.ClassDeclaration, Map<string, DirectiveInScope|null>>();
@ -86,22 +84,6 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
private readonly componentScopeReader: ComponentScopeReader,
private readonly typeCheckScopeRegistry: TypeCheckScopeRegistry) {}
resetOverrides(): void {
for (const fileRecord of this.state.values()) {
if (fileRecord.templateOverrides !== null) {
fileRecord.templateOverrides = null;
fileRecord.shimData.clear();
fileRecord.isComplete = false;
}
}
// Ideally only those components with overridden templates would have their caches invalidated,
// but the `TemplateTypeCheckerImpl` does not track the class for components with overrides. As
// a quick workaround, clear the entire cache instead.
this.completionCache.clear();
this.symbolBuilderCache.clear();
}
getTemplate(component: ts.ClassDeclaration): TmplAstNode[]|null {
const {data} = this.getLatestComponentState(component);
if (data === null) {
@ -151,42 +133,6 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
return {data, tcb, shimPath};
}
overrideComponentTemplate(component: ts.ClassDeclaration, template: string):
{nodes: TmplAstNode[], errors: ParseError[]|null} {
const {nodes, errors} = parseTemplate(template, 'override.html', {
// Set `leadingTriviaChars` and `preserveWhitespaces` such that whitespace is not stripped
// and fully accounted for in source spans. Without these flags the source spans can be
// inaccurate.
// Note: template parse options should be aligned with `template_target_spec.ts` and the
// `diagNodes` in `ComponentDecoratorHandler._parseTemplate`.
preserveWhitespaces: true,
leadingTriviaChars: [],
});
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, errors});
// 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;
// Overriding a component's template invalidates its cached results.
this.completionCache.delete(component);
this.symbolBuilderCache.delete(component);
return {nodes, errors};
}
isTrackedTypeCheckFile(filePath: AbsoluteFsPath): boolean {
return this.getFileAndShimRecordsForPath(filePath) !== null;
}
@ -337,7 +283,6 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
fileData.shimData.delete(shimPath);
fileData.isComplete = false;
fileData.templateOverrides?.delete(templateId);
this.isComplete = false;
}
@ -361,10 +306,6 @@ 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.
@ -373,8 +314,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
}
const previousResults = this.priorBuild.priorTypeCheckingResultsFor(sf);
if (previousResults === null || !previousResults.isComplete ||
previousResults.templateOverrides !== null) {
if (previousResults === null || !previousResults.isComplete) {
return;
}
@ -496,7 +436,6 @@ 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(),
@ -679,11 +618,6 @@ export interface FileTypeCheckingData {
*/
sourceManager: TemplateSourceManager;
/**
* Map of template overrides applied to any components in this input file.
*/
templateOverrides: Map<TemplateId, TemplateOverride>|null;
/**
* Data for each shim generated from this input file.
*
@ -716,20 +650,6 @@ class WholeProgramTypeCheckingHost implements TypeCheckingHost {
return !fileData.shimData.has(shimPath);
}
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TemplateOverride|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);
@ -774,20 +694,6 @@ class SingleFileTypeCheckingHost implements TypeCheckingHost {
return !this.fileData.shimData.has(shimPath);
}
getTemplateOverride(sfPath: AbsoluteFsPath, node: ts.ClassDeclaration): TemplateOverride|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);

18
packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
View File

@ -51,11 +51,6 @@ export interface ShimTypeCheckingData {
templates: Map<TemplateId, TemplateData>;
}
export interface TemplateOverride {
nodes: TmplAstNode[];
errors: ParseError[]|null;
}
/**
* Data tracked for each template processed by the template type-checking system.
*/
@ -143,12 +138,6 @@ 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): TemplateOverride|null;
/**
* Report data from a shim generated from the given input file path.
*/
@ -225,13 +214,6 @@ export class TypeCheckContextImpl implements TypeCheckContext {
const templateDiagnostics: TemplateDiagnostic[] = [];
const sfPath = absoluteFromSourceFile(ref.node.getSourceFile());
const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node);
if (overrideTemplate !== null) {
template = overrideTemplate.nodes;
parseErrors = overrideTemplate.errors;
}
if (parseErrors !== null) {
templateDiagnostics.push(
...this.getTemplateDiagnostics(parseErrors, templateId, sourceMapping));

26
packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts
View File

@ -71,32 +71,6 @@ runInEachFileSystem(() => {
expect(userAtTopLevel.kind).toBe(CompletionKind.Reference);
expect(userInNgFor.kind).toBe(CompletionKind.Variable);
});
it('should invalidate cached completions when overrides change', () => {
// The template starts with a #foo local reference.
const {completions: before, templateTypeChecker, component} =
setupCompletions('<div #foo></div>');
expect(Array.from(before.templateContext.keys())).toEqual(['foo']);
// Override the template and change the name of the local reference to #bar. This should
// invalidate any cached completions.
templateTypeChecker.overrideComponentTemplate(component, '<div #bar></div>');
// Fresh completions should include the #bar reference instead.
const afterOverride =
templateTypeChecker.getGlobalCompletions(/* root template */ null, component)!;
expect(afterOverride).toBeDefined();
expect(Array.from(afterOverride.templateContext.keys())).toEqual(['bar']);
// Reset the template to its original. This should also invalidate any cached completions.
templateTypeChecker.resetOverrides();
// Fresh completions should include the original #foo now.
const afterReset =
templateTypeChecker.getGlobalCompletions(/* root template */ null, component)!;
expect(afterReset).toBeDefined();
expect(Array.from(afterReset.templateContext.keys())).toEqual(['foo']);
});
});
describe('TemplateTypeChecker scopes', () => {

27
packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts
View File

@ -40,33 +40,6 @@ runInEachFileSystem(() => {
assertElementSymbol(symbol.host);
});
it('should invalidate symbols when template overrides change', () => {
const fileName = absoluteFrom('/main.ts');
const templateString = `<div id="helloWorld"></div>`;
const {templateTypeChecker, program} = setup(
[
{
fileName,
templates: {'Cmp': templateString},
source: `export class Cmp {}`,
},
],
);
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
const {attributes: beforeAttributes} = getAstElements(templateTypeChecker, cmp)[0];
const beforeSymbol = templateTypeChecker.getSymbolOfNode(beforeAttributes[0], cmp)!;
// Replace the <div> with a <span>.
templateTypeChecker.overrideComponentTemplate(cmp, '<span id="helloWorld"></span>');
const {attributes: afterAttributes} = getAstElements(templateTypeChecker, cmp)[0];
const afterSymbol = templateTypeChecker.getSymbolOfNode(afterAttributes[0], cmp)!;
// After the override, the symbol cache should have been invalidated.
expect(beforeSymbol).not.toBe(afterSymbol);
});
describe('should get a symbol for text attributes corresponding with a directive input', () => {
let fileName: AbsoluteFsPath;
let targets: TypeCheckingTarget[];

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

@ -211,143 +211,6 @@ runInEachFileSystem(() => {
expect(absoluteFromSourceFile(diags[0].relatedInformation![0].file!)).toBe(dirSf.fileName);
});
});
describe('template overrides', () => {
it('should override a simple template', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {'Cmp': '<div>{{original}}</div>'},
}]);
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbReal.getText()).toContain('original');
templateTypeChecker.overrideComponentTemplate(cmp, '<div>{{override}}</div>');
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp);
expect(tcbOverridden).not.toBeNull();
expect(tcbOverridden!.getText()).not.toContain('original');
expect(tcbOverridden!.getText()).toContain('override');
});
it('should clear overrides on request', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {'Cmp': '<div>{{original}}</div>'},
}]);
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
templateTypeChecker.overrideComponentTemplate(cmp, '<div>{{override}}</div>');
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbOverridden.getText()).not.toContain('original');
expect(tcbOverridden.getText()).toContain('override');
templateTypeChecker.resetOverrides();
// The template should be back to the original.
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbReal.getText()).toContain('original');
expect(tcbReal.getText()).not.toContain('override');
});
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',
inputs: {'input': 'input'},
isGeneric: true,
}]
},
{
fileName: dirFile,
source: `export class TestDir<T> {}`,
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 [input]="value"></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);
});
});
it('should allow get diagnostics for a single component', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'Cmp1': '<invalid-element-a></invalid-element-a>',
'Cmp2': '<invalid-element-b></invalid-element-b>'
},
}]);
const sf = getSourceFileOrError(program, fileName);
const cmp1 = getClass(sf, 'Cmp1');
const cmp2 = getClass(sf, 'Cmp2');
const diags1 = templateTypeChecker.getDiagnosticsForComponent(cmp1);
expect(diags1.length).toBe(1);
expect(diags1[0].messageText).toContain('invalid-element-a');
expect(diags1[0].messageText).not.toContain('invalid-element-b');
const diags2 = templateTypeChecker.getDiagnosticsForComponent(cmp2);
expect(diags2.length).toBe(1);
expect(diags2[0].messageText).toContain('invalid-element-b');
expect(diags2[0].messageText).not.toContain('invalid-element-a');
});
describe('getTemplateOfComponent()', () => {
it('should provide access to a component\'s real template', () => {
const fileName = absoluteFrom('/main.ts');
@ -363,24 +226,6 @@ runInEachFileSystem(() => {
expect(nodes).not.toBeNull();
expect(nodes[0].sourceSpan.start.file.content).toBe('<div>Template</div>');
});
it('should provide access to an overridden template', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'Cmp': '<div>Template</div>',
},
}]);
const cmp = getClass(getSourceFileOrError(program, fileName), 'Cmp');
templateTypeChecker.overrideComponentTemplate(cmp, '<div>Overridden</div>');
templateTypeChecker.getDiagnosticsForComponent(cmp);
const nodes = templateTypeChecker.getTemplate(cmp)!;
expect(nodes).not.toBeNull();
expect(nodes[0].sourceSpan.start.file.content).toBe('<div>Overridden</div>');
});
});
});
});

3
packages/language-service/ivy/test/legacy/template_target_spec.ts
View File

@ -32,8 +32,7 @@ function parse(template: string): ParseResult {
// and fully accounted for in source spans. Without these flags the source spans can be
// inaccurate.
// Note: template parse options should be aligned with the `diagNodes` in
// `ComponentDecoratorHandler._parseTemplate`. and
// `TemplateTypeCheckerImpl.overrideComponentTemplate`.
// `ComponentDecoratorHandler._parseTemplate`.
leadingTriviaChars: [],
preserveWhitespaces: true,
}),