From fd5cd100a3c8fb357f2a85769c74e5a847ddcd4a Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Thu, 7 Mar 2019 14:24:54 -0800
Subject: [PATCH] fix(ivy): move i18n instructions after listener ones (#29173)

Prior to this commit, i18n instructions (i18n, i18nStart) were generated before listener instructions. As a result, event listeners were attached to the wrong element (text node, not the parent element). This change updates the order of instructions and puts i18n ones after listeners, to make sure listeners are attached to the right elements.

PR Close #29173
---
 .../compliance/r3_view_compiler_i18n_spec.ts  | 22 +++++++++++++++++++
 .../compiler/src/render3/view/template.ts     | 12 +++++-----
 packages/core/test/i18n_integration_spec.ts   | 20 +++++++++++++++++
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
index 2a13f675b1..6a63a39bd4 100644
--- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
+++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
@@ -1270,6 +1270,28 @@ describe('i18n support in the view compiler', () => {
 
       verify(input, output);
     });
+
+    it('should generate event listeners instructions before i18n ones', () => {
+      const input = `
+        <div i18n (click)="onClick()">Hello</div>
+      `;
+
+      const output = String.raw `
+        const $_c0$ = [${AttributeMarker.Bindings}, "click"];
+        const $MSG_EXTERNAL_3902961887793684628$$APP_SPEC_TS_1$ = goog.getMsg("Hello");
+        …
+        template: function MyComponent_Template(rf, ctx) {
+          if (rf & 1) {
+            $r3$.ɵelementStart(0, "div", $_c0$);
+            $r3$.ɵlistener("click", function MyComponent_Template_div_click_0_listener($event) { return ctx.onClick(); });
+            $r3$.ɵi18n(1, $MSG_EXTERNAL_3902961887793684628$$APP_SPEC_TS_1$);
+            $r3$.ɵelementEnd();
+          }
+        }
+      `;
+
+      verify(input, output);
+    });
   });
 
   describe('self-closing i18n instructions', () => {
diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts
index 5038bf8b44..bca6947265 100644
--- a/packages/compiler/src/render3/view/template.ts
+++ b/packages/compiler/src/render3/view/template.ts
@@ -657,12 +657,6 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
         }
       }
 
-      // Note: it's important to keep i18n/i18nStart instructions after i18nAttributes ones,
-      // to make sure i18nAttributes instruction targets current element at runtime.
-      if (isI18nRootElement) {
-        this.i18nStart(element.sourceSpan, element.i18n !, createSelfClosingI18nInstruction);
-      }
-
       // The style bindings code is placed into two distinct blocks within the template function AOT
       // code: creation and update. The creation code contains the `elementStyling` instructions
       // which will apply the collected binding values to the element. `elementStyling` is
@@ -680,6 +674,12 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
             outputAst.sourceSpan, R3.listener,
             this.prepareListenerParameter(element.name, outputAst, elementIndex));
       });
+
+      // Note: it's important to keep i18n/i18nStart instructions after i18nAttributes and
+      // listeners, to make sure i18nAttributes instruction targets current element at runtime.
+      if (isI18nRootElement) {
+        this.i18nStart(element.sourceSpan, element.i18n !, createSelfClosingI18nInstruction);
+      }
     }
 
     // the code here will collect all update-level styling instructions and add them to the
diff --git a/packages/core/test/i18n_integration_spec.ts b/packages/core/test/i18n_integration_spec.ts
index 7a9e297d39..e04bc596e0 100644
--- a/packages/core/test/i18n_integration_spec.ts
+++ b/packages/core/test/i18n_integration_spec.ts
@@ -28,6 +28,9 @@ class MyComp {
   age = 20;
   count = 2;
   otherLabel = 'other label';
+  clicks = 0;
+
+  onClick() { this.clicks++; }
 }
 
 const TRANSLATIONS: any = {
@@ -254,6 +257,23 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
       const element = fixture.nativeElement.firstChild;
       expect(element).toHaveText('Bonjour John');
     });
+
+    it('should work correctly with event listeners', () => {
+      const content = 'Hello {{ name }}';
+      const template = `
+        <div i18n (click)="onClick()">${content}</div>
+      `;
+      const fixture = getFixtureWithOverrides({template});
+
+      const element = fixture.nativeElement.firstChild;
+      const instance = fixture.componentInstance;
+
+      expect(element).toHaveText('Bonjour John');
+      expect(instance.clicks).toBe(0);
+
+      element.click();
+      expect(instance.clicks).toBe(1);
+    });
   });
 
   describe('ng-container and ng-template support', () => {