diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index a9ff113286af..156a4a36d19e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -2709,21 +2709,11 @@ class TcbExpressionTranslator { * context). This method assists in resolving those. */ protected resolve(ast: AST): ts.Expression | null { - // TODO: this is actually a bug, because `ImplicitReceiver` extends `ThisReceiver`. Consider a - // case when the explicit `this` read is inside a template with a context that also provides the - // variable name being read: - // ``` - // {{this.a}} - // ``` - // Clearly, `this.a` should refer to the class property `a`. However, because of this code, - // `this.a` will refer to `let-a` on the template context. - // - // Note that the generated code is actually consistent with this bug. To fix it, we have to: - // - Check `!(ast.receiver instanceof ThisReceiver)` in this condition - // - Update `ingest.ts` in the Template Pipeline (see the corresponding comment) - // - Turn off legacy TemplateDefinitionBuilder - // - Fix g3, and release in a major version - if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver) { + if ( + ast instanceof PropertyRead && + ast.receiver instanceof ImplicitReceiver && + !(ast.receiver instanceof ThisReceiver) + ) { // Try to resolve a bound target for this expression. If no such target is available, then // the expression is referencing the top-level component context. In that case, `null` is // returned here to let it fall through resolution so it will be caught when the diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js index c65ce590881c..330673b8425c 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js @@ -1249,13 +1249,13 @@ import * as i0 from "@angular/core"; export class MyComponent { } MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); -MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-component", ngImport: i0, template: '{{this.a}}', isInline: true }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-component", ngImport: i0, template: '{{a}}', isInline: true }); i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ type: Component, args: [{ selector: 'my-component', standalone: true, - template: '{{this.a}}', + template: '{{a}}', }] }] }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.js index bd438ff81bac..9591f67d00b0 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.js @@ -2,7 +2,6 @@ MyComponent_ng_template_0_Template(rf, ctx) { if (rf & 1) { i0.ɵɵtext(0); } if (rf & 2) { - // NOTE: The fact that `this.` still refers to template context is a TDB and TCB bug; we should fix this eventually. const $a_r1$ = ctx.$implicit; i0.ɵɵtextInterpolate($a_r1$); } @@ -16,4 +15,4 @@ function MyComponent_Template(rf, ctx) { } if (rf & 2) { i0.ɵɵproperty("ngIf", true); } -} \ No newline at end of file +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.ts index d4033861a55c..62a9a9a1563d 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.ts +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/ng_template_implicit.ts @@ -3,7 +3,7 @@ import {Component} from '@angular/core'; @Component({ selector: 'my-component', standalone: true, - template: '{{this.a}}', + template: '{{a}}', }) export class MyComponent { p1!: any; diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index be87bac5a782..8d19507022f8 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -922,21 +922,13 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { private maybeMap(ast: PropertyRead | SafePropertyRead | PropertyWrite, name: string): void { // If the receiver of the expression isn't the `ImplicitReceiver`, this isn't the root of an // `AST` expression that maps to a `Variable` or `Reference`. - if (!(ast.receiver instanceof ImplicitReceiver)) { + if (!(ast.receiver instanceof ImplicitReceiver) || ast.receiver instanceof ThisReceiver) { return; } // Check whether the name exists in the current scope. If so, map it. Otherwise, the name is // probably a property on the top-level component context. const target = this.scope.lookup(name); - - // It's not allowed to read template entities via `this`, however it previously worked by - // accident (see #55115). Since `@let` declarations are new, we can fix it from the beginning, - // whereas pre-existing template entities will be fixed in #55115. - if (target instanceof LetDeclaration && ast.receiver instanceof ThisReceiver) { - return; - } - if (target !== null) { this.bindings.set(ast, target); } diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index c891c9066b77..5c51a7623dca 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -1003,30 +1003,10 @@ function convertAst( if (ast instanceof e.ASTWithSource) { return convertAst(ast.ast, job, baseSourceSpan); } else if (ast instanceof e.PropertyRead) { - const isThisReceiver = ast.receiver instanceof e.ThisReceiver; // Whether this is an implicit receiver, *excluding* explicit reads of `this`. const isImplicitReceiver = ast.receiver instanceof e.ImplicitReceiver && !(ast.receiver instanceof e.ThisReceiver); - // Whether the name of the read is a node that should be never retain its explicit this - // receiver. - const isSpecialNode = ast.name === '$any' || ast.name === '$event'; - // TODO: The most sensible condition here would be simply `isImplicitReceiver`, to convert only - // actual implicit `this` reads, and not explicit ones. However, TemplateDefinitionBuilder (and - // the Typecheck block!) both have the same bug, in which they also consider explicit `this` - // reads to be implicit. This causes problems when the explicit `this` read is inside a - // template with a context that also provides the variable name being read: - // ``` - // {{this.a}} - // ``` - // The whole point of the explicit `this` was to access the class property, but TDB and the - // current TCB treat the read as implicit, and give you the context property instead! - // - // For now, we emulate this old behavior by aggressively converting explicit reads to to - // implicit reads, except for the special cases that TDB and the current TCB protect. However, - // it would be an improvement to fix this. - // - // See also the corresponding comment for the TCB, in `type_check_block.ts`. - if (isImplicitReceiver || (isThisReceiver && !isSpecialNode)) { + if (isImplicitReceiver) { return new ir.LexicalReadExpr(ast.name); } else { return new o.ReadPropExpr( diff --git a/packages/compiler/test/render3/view/binding_spec.ts b/packages/compiler/test/render3/view/binding_spec.ts index 9d97a459e99b..2d054b9dd0a7 100644 --- a/packages/compiler/test/render3/view/binding_spec.ts +++ b/packages/compiler/test/render3/view/binding_spec.ts @@ -373,6 +373,35 @@ describe('t2 binding', () => { expect((target as a.LetDeclaration)?.name).toBe('value'); }); + it('should not resolve a `this` access to a template reference', () => { + const template = parseTemplate( + ` + + {{this.value}} + `, + '', + ); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const interpolationWrapper = (template.nodes[1] as a.BoundText).value as e.ASTWithSource; + const propertyRead = (interpolationWrapper.ast as e.Interpolation).expressions[0]; + const target = res.getExpressionTarget(propertyRead); + + expect(target).toBe(null); + }); + + it('should not resolve a `this` access to a template variable', () => { + const template = parseTemplate(`{{this.value}}`, ''); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const templateNode = template.nodes[0] as a.Template; + const interpolationWrapper = (templateNode.children[0] as a.BoundText).value as e.ASTWithSource; + const propertyRead = (interpolationWrapper.ast as e.Interpolation).expressions[0]; + const target = res.getExpressionTarget(propertyRead); + + expect(target).toBe(null); + }); + it('should not resolve a `this` access to a `@let` declaration', () => { const template = parseTemplate( ` diff --git a/packages/core/test/acceptance/embedded_views_spec.ts b/packages/core/test/acceptance/embedded_views_spec.ts index a7ce9bd7731a..839ec7811a0b 100644 --- a/packages/core/test/acceptance/embedded_views_spec.ts +++ b/packages/core/test/acceptance/embedded_views_spec.ts @@ -44,7 +44,7 @@ describe('embedded views', () => { }); it('should resolve template input variables through the implicit receiver', () => { - @Component({template: `{{this.a}}`}) + @Component({template: `{{a}}`}) class TestCmp {} TestBed.configureTestingModule({declarations: [TestCmp]});