From 19daa075e807cb8ba0ea9d1bb362ce89f87d35a7 Mon Sep 17 00:00:00 2001
From: Dylan Hunn <dylhunn@gmail.com>
Date: Wed, 6 Dec 2023 11:47:21 -0800
Subject: [PATCH] refactor(compiler): Listeners should be ingested before
 i18nStart (#53405)

Listener instructions should not be inside the i18n block. In order to avoid this, we ingest bindings on an element before starting the i18n block.

We previously missed this case because almost all bindings result in *update* instructions, which don't need to be ordered relative to i18nStart/i18nEnd create instructions. However, listeners are the only kind of binding that gets ingested into the create block.

PR Close #53405
---
 .../nested_nodes/TEST_CASES.json                 | 10 ++++++++--
 ..._listeners.js => event_listeners_template.js} |  0
 .../event_listeners_template.pipeline.js         | 16 ++++++++++++++++
 .../compiler/src/template/pipeline/src/ingest.ts | 12 +++++++++---
 4 files changed, 33 insertions(+), 5 deletions(-)
 rename packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/{event_listeners.js => event_listeners_template.js} (100%)
 create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners_template.pipeline.js

diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/TEST_CASES.json
index 866074bc69a..b27f73f74c3 100644
--- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/TEST_CASES.json
+++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/TEST_CASES.json
@@ -240,13 +240,19 @@
       ],
       "expectations": [
         {
+          "files": [
+            {
+              "generated": "event_listeners.js",
+              "expected": "event_listeners_template.js",
+              "templatePipelineExpected": "event_listeners_template.pipeline.js"
+            }
+          ],
           "extraChecks": [
             "verifyPlaceholdersIntegrity",
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should handle ng-content in i18n block",
diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners_template.js
similarity index 100%
rename from packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners.js
rename to packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners_template.js
diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners_template.pipeline.js
new file mode 100644
index 00000000000..d8ed304fe12
--- /dev/null
+++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/event_listeners_template.pipeline.js
@@ -0,0 +1,16 @@
+consts: () => {
+	__i18nMsg__('Hello', [], {}, {})
+	return [
+	  $i18n_0$,
+	  [__AttributeMarker.Bindings__, "click"]
+	];
+  },
+  template: function MyComponent_Template(rf, ctx) {
+	if (rf & 1) {
+	  $r3$.ɵɵelementStart(0, "div", 1);
+	  $r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener() { return ctx.onClick(); });
+	  $r3$.ɵɵi18n(1, 0);
+	  $r3$.ɵɵelementEnd();
+	}
+  }
+  
\ No newline at end of file
diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts
index 90843c73cc7..2d70a764f3b 100644
--- a/packages/compiler/src/template/pipeline/src/ingest.ts
+++ b/packages/compiler/src/template/pipeline/src/ingest.ts
@@ -167,6 +167,14 @@ function ingestElement(unit: ViewCompilationUnit, element: t.Element): void {
 
   ingestBindings(unit, startOp, element);
   ingestReferences(startOp, element);
+
+  // Start i18n, if needed, goes after the element create and bindings, but before the nodes
+  let i18nBlockId: ir.XrefId|null = null;
+  if (element.i18n instanceof i18n.Message) {
+    i18nBlockId = unit.job.allocateXrefId();
+    unit.create.push(ir.createI18nStartOp(i18nBlockId, element.i18n));
+  }
+
   ingestNodes(unit, element.children);
 
   // The source span for the end op is typically the element closing tag. However, if no closing tag
@@ -178,9 +186,7 @@ function ingestElement(unit: ViewCompilationUnit, element: t.Element): void {
   unit.create.push(endOp);
 
   // If there is an i18n message associated with this element, insert i18n start and end ops.
-  if (element.i18n instanceof i18n.Message) {
-    const i18nBlockId = unit.job.allocateXrefId();
-    ir.OpList.insertAfter<ir.CreateOp>(ir.createI18nStartOp(i18nBlockId, element.i18n), startOp);
+  if (i18nBlockId !== null) {
     ir.OpList.insertBefore<ir.CreateOp>(ir.createI18nEndOp(i18nBlockId), endOp);
   }
 }