diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 5db7d33e45..84ba9b1168 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -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, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts index 06f55664f7..da1d0bb294 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts @@ -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`. * diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index 6a63764c81..2cbad7db17 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -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(); @@ -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>(); @@ -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|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); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 1bb8845a0f..e9729b1ff6 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -51,11 +51,6 @@ export interface ShimTypeCheckingData { templates: Map; } -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)); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts index ba1036320c..104b3ed279 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts @@ -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('
'); - 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, '
'); - - // 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', () => { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index a70c8bd09c..1d4cb3d1b3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -40,33 +40,6 @@ runInEachFileSystem(() => { assertElementSymbol(symbol.host); }); - it('should invalidate symbols when template overrides change', () => { - const fileName = absoluteFrom('/main.ts'); - const templateString = `
`; - 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
with a . - templateTypeChecker.overrideComponentTemplate(cmp, ''); - - 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[]; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts index 3a06cc6459..226840a4bc 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts @@ -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': '
{{original}}
'}, - }]); - - const sf = getSourceFileOrError(program, fileName); - const cmp = getClass(sf, 'Cmp'); - - const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!; - expect(tcbReal.getText()).toContain('original'); - - templateTypeChecker.overrideComponentTemplate(cmp, '
{{override}}
'); - 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': '
{{original}}
'}, - }]); - - const sf = getSourceFileOrError(program, fileName); - const cmp = getClass(sf, 'Cmp'); - - templateTypeChecker.overrideComponentTemplate(cmp, '
{{override}}
'); - 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': '
'}, - declarations: [{ - name: 'TestDir', - selector: '[dir]', - file: dirFile, - type: 'directive', - inputs: {'input': 'input'}, - isGeneric: true, - }] - }, - { - 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, '
'); - - 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': '
'}}, - {fileName: file2, templates: {'Cmp2': ''}} - ]); - - 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, '

'); - - // 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': '', - 'Cmp2': '' - }, - }]); - 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('
Template
'); }); - - it('should provide access to an overridden template', () => { - const fileName = absoluteFrom('/main.ts'); - const {program, templateTypeChecker} = setup([{ - fileName, - templates: { - 'Cmp': '
Template
', - }, - }]); - const cmp = getClass(getSourceFileOrError(program, fileName), 'Cmp'); - - templateTypeChecker.overrideComponentTemplate(cmp, '
Overridden
'); - templateTypeChecker.getDiagnosticsForComponent(cmp); - - const nodes = templateTypeChecker.getTemplate(cmp)!; - expect(nodes).not.toBeNull(); - expect(nodes[0].sourceSpan.start.file.content).toBe('
Overridden
'); - }); }); }); }); diff --git a/packages/language-service/ivy/test/legacy/template_target_spec.ts b/packages/language-service/ivy/test/legacy/template_target_spec.ts index 0035173a07..73457a9d2e 100644 --- a/packages/language-service/ivy/test/legacy/template_target_spec.ts +++ b/packages/language-service/ivy/test/legacy/template_target_spec.ts @@ -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, }),