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 7026b2ad5e112f..a5178194cda13d 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 @@ -2287,6 +2287,20 @@ class TcbExpressionTranslator { * context). This method assists in resolving those. */ protected resolve(ast: AST): ts.Expression|null { + // TODO: this is actually a bug, because `ImpliticReceiver` 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, becuase 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) { // 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 diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json index b4318ea8007e70..55df8b8385b717 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json @@ -399,8 +399,7 @@ { "failureMessage": "Incorrect template" } - ], - "skipForTemplatePipeline": true + ] } ] } 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 968fac79c23b0a..bd438ff81bac8c 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,6 +2,7 @@ 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$); } diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index e811e30f805349..1bcf5bbeaf1dbc 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -654,7 +654,30 @@ function convertAst( if (ast instanceof e.ASTWithSource) { return convertAst(ast.ast, job, baseSourceSpan); } else if (ast instanceof e.PropertyRead) { - if (ast.receiver instanceof e.ImplicitReceiver && !(ast.receiver instanceof e.ThisReceiver)) { + 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 behvaior 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)) { return new ir.LexicalReadExpr(ast.name); } else { return new o.ReadPropExpr(