refactor(compiler): Show an error when a property binding duplicates an i18n attribute (#54063)

dylhunn · thePunderWoman · commit f600f0fb278f · 2024-02-02T20:52:39.000Z
Consider the following very quirky Angular template, which has both an i18n attribute binding and a property binding to `in`: ``` <cmp [in]="foo" in="bar" i18n-in /> ``` What would you expect the above template to do? `TemplateDefinitionBuilder` will emit the following Ivy instructions: ``` // Element constant attributes consts: () => { __i18nMsg__('bar', [], {}, {}) return [["in", i18n_0, __AttributeMarker.I18n__, "in"]]; } // ... function MyComponent_Template(rf, ctx) { if (rf & 1) { // Create mode i0.ɵɵelement(0, "cmp", 0); } } ``` This makes some sense -- we create a single element, and attach an i18n message to the `in` attribute. But is this actually correct? Notice that the property binding is completely missing! Indeed, Template Pipeline actually produces this code: ``` // Element constant attributes consts: () => { __i18nMsg__('bar', [], {}, {}) return [["in", i18n_0, __AttributeMarker.I18n__, "in"]]; } // ... function MyComponent_Template(rf, ctx) { if (rf & 1) { // Create mode i0.ɵɵelement(0, "cmp", 0); } else if (rf & 2) { // Update mode i0.ɵɵproperty("in", ctx.foo); } } ``` Aha! There's the property binding! Arguably, this is a bug in `TemplateDefinitionBuilder`, but after some discussion on Slack, we have decided to ban this practice in a future Angular version. For now, we allow Template Pipeline to have slightly different output, but print an error to warn the user of the issue. PR Close #54063
diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts
@@ -895,16 +895,26 @@ function ingestElementBindings(
     unit: ViewCompilationUnit, op: ir.ElementOpBase, element: t.Element): void {
   let bindings = new Array<ir.BindingOp|ir.ExtractedAttributeOp|null>();
 
+  let i18nAttributeBindingNames = new Set<string>();
+
   for (const attr of element.attributes) {
     // Attribute literal bindings, such as `attr.foo="bar"`.
     const securityContext = domSchema.securityContext(element.name, attr.name, true);
     bindings.push(ir.createBindingOp(
         op.xref, ir.BindingKind.Attribute, attr.name,
         convertAstWithInterpolation(unit.job, attr.value, attr.i18n), null, securityContext, true,
         false, null, asMessage(attr.i18n), attr.sourceSpan));
+    if (attr.i18n) {
+      i18nAttributeBindingNames.add(attr.name);
+    }
   }
 
   for (const input of element.inputs) {
+    if (i18nAttributeBindingNames.has(input.name)) {
+      console.error(`On component ${unit.job.componentName}, the binding ${
+          input
+              .name} is both an i18n attribute and a property. You may want to remove the property binding. This will become a compilation error in future versions of Angular.`);
+    }
     // All dynamic bindings (both attribute and property bindings).
     bindings.push(ir.createBindingOp(
         op.xref, BINDING_KINDS.get(input.type)!, input.name,
