refactor(compiler): Fix i18n placeholders for slef-closing elements (#…

…52195) Fixes handling of placeholders for self-closing tags. Self-closing tags set a combined value for the start tag placeholder, rather than separate values for the start and close placeholders. This commit also enables a number of now passing tests. For some of these tests I had create a separate golden file due to the different ordering of the const array. In the template pipeline, i18n and attribute const collection happen in different pahses and we therefore get a different order than TemplateDefinitionBuilder, which collected everything in one pass. The order should not affect the overall behavior. PR Close #52195
angular · Oct 16, 2023 · 9f4927e · 9f4927e
1 parent fb487ea
commit 9f4927e
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 28 deletions.
diff --git a/...ompiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/TEST_CASES.json b/...ompiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/nested_nodes/TEST_CASES.json
@@ -69,8 +69,7 @@
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should support named interpolations",

diff --git a/...test/compliance/test_cases/r3_view_compiler_i18n/ng-container_ng-template/TEST_CASES.json b/...test/compliance/test_cases/r3_view_compiler_i18n/ng-container_ng-template/TEST_CASES.json
@@ -94,13 +94,19 @@
       ],
       "expectations": [
         {
+          "files": [
+            {
+              "generated": "self_closing_tags.js",
+              "expected": "self_closing_tags.template.js",
+              "templatePipelineExpected": "self_closing_tags.pipeline.js"
+            }
+          ],
           "extraChecks": [
             "verifyPlaceholdersIntegrity",
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should not emit duplicate i18n consts for nested <ng-container>s",
@@ -126,8 +132,7 @@
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate a self-closing container instruction for ng-container inside i18n",
@@ -141,8 +146,7 @@
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should not generate a self-closing container instruction for ng-container with non-text ",

diff --git a/...e/test_cases/r3_view_compiler_i18n/ng-container_ng-template/self_closing_tags.pipeline.js b/...e/test_cases/r3_view_compiler_i18n/ng-container_ng-template/self_closing_tags.pipeline.js
@@ -0,0 +1,27 @@
+function MyComponent_ng_template_3_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵi18nStart(0, 1);
+    $r3$.ɵɵelement(1, "img", 2);
+    $r3$.ɵɵi18nEnd();
+  }
+}
+…
+consts: () => {
+  __i18nMsg__('{$tagImg} is my logo #1 ', [['tagImg', String.raw`\uFFFD#2\uFFFD\uFFFD/#2\uFFFD`]], {original_code: {tagImg: '<img src="logo.png" title="Logo" />'}}, {})
+  __i18nMsg__('{$tagImg} is my logo #2 ', [['tagImg', String.raw`\uFFFD#1\uFFFD\uFFFD/#1\uFFFD`]], {original_code: {tagImg: '<img src="logo.png" title="Logo" />'}}, {})
+  return [
+    $i18n_0$,
+    $i18n_1$,
+    ["src", "logo.png", "title", "Logo"]
+  ];
+},
+template: function MyComponent_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵelementContainerStart(0);
+    $r3$.ɵɵi18nStart(1, 0);
+    $r3$.ɵɵelement(2, "img", 2);
+    $r3$.ɵɵi18nEnd();
+    $r3$.ɵɵelementContainerEnd();
+    $r3$.ɵɵtemplate(3, MyComponent_ng_template_3_Template, 2, 0, "ng-template");
+  }
+}
diff --git a/...ontainer_ng-template/self_closing_tags.js → ...ng-template/self_closing_tags.template.js b/...ontainer_ng-template/self_closing_tags.js → ...ng-template/self_closing_tags.template.js
diff --git a/...ompliance/test_cases/r3_view_compiler_i18n/self-closing_i18n_instructions/TEST_CASES.json b/...ompliance/test_cases/r3_view_compiler_i18n/self-closing_i18n_instructions/TEST_CASES.json
@@ -13,8 +13,7 @@
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should be generated for ICU-only i18n blocks",
@@ -43,8 +42,7 @@
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should not be generated in case we have styling instructions",
@@ -53,13 +51,19 @@
       ],
       "expectations": [
         {
+          "files": [
+            {
+              "generated": "styles.js",
+              "expected": "styles.template.js",
+              "templatePipelineExpected": "styles.pipeline.js"
+            }
+          ],
           "extraChecks": [
             "verifyPlaceholdersIntegrity",
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     }
   ]
 }
diff --git a/...liance/test_cases/r3_view_compiler_i18n/self-closing_i18n_instructions/styles.pipeline.js b/...liance/test_cases/r3_view_compiler_i18n/self-closing_i18n_instructions/styles.pipeline.js
@@ -0,0 +1,22 @@
+decls: 4,
+vars: 0,
+consts: () => {
+  __i18nMsg__('Text #1', [], {}, {})
+  __i18nMsg__('Text #2', [], {}, {})
+  return [
+    $i18n_0$,
+    $i18n_1$,
+    [__AttributeMarker.Classes__, "myClass"],
+    [__AttributeMarker.Styles__, "padding", "10px"]
+  ];
+},
+template: function MyComponent_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵelementStart(0, "span", 2);
+    $r3$.ɵɵi18n(1, 0);
+    $r3$.ɵɵelementEnd();
+    $r3$.ɵɵelementStart(2, "span", 3);
+    $r3$.ɵɵi18n(3, 1);
+    $r3$.ɵɵelementEnd();
+  }
+}
diff --git a/.../self-closing_i18n_instructions/styles.js → ...sing_i18n_instructions/styles.template.js b/.../self-closing_i18n_instructions/styles.js → ...sing_i18n_instructions/styles.template.js
diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_placeholders.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_placeholders.ts
@@ -50,8 +50,12 @@ const LIST_END_MARKER = ']';
  */
 const LIST_DELIMITER = '|';
 
+/**
+ * Flags that describe what an i18n param value. These determine how the value is serialized into
+ * the final map.
+ */
 enum I18nParamValueFlags {
-  None = 0b000,
+  None = 0b0000,
 
   /**
    *  This value represtents an element tag.
@@ -61,13 +65,17 @@ enum I18nParamValueFlags {
   /**
    * This value represents a template tag.
    */
-  TemplateTag = 0b010,
+  TemplateTag = 0b0010,
+
+  /**
+   * This value represents the opening of a tag.
+   */
+  OpenTag = 0b0100,
 
   /**
-   * This value represents the closing of a tag. (Can only be used together with ElementTag or
-   * TemplateTag)
+   * This value represents the closing of a tag.
    */
-  CloseTag = 0b100,
+  CloseTag = 0b1000,
 }
 
 /**
@@ -130,7 +138,8 @@ class I18nPlaceholderParams {
       const currentValues = this.values.get(placeholder) || [];
       // Child element close tag params should be prepended to maintain the same order as
       // TemplateDefinitionBuilder.
-      if (otherValues[0]!.flags & I18nParamValueFlags.CloseTag) {
+      const flags = otherValues[0]!.flags;
+      if ((flags & I18nParamValueFlags.CloseTag) && !(flags & I18nParamValueFlags.OpenTag)) {
         this.values.set(placeholder, [...otherValues, ...currentValues]);
       } else {
         this.values.set(placeholder, [...currentValues, ...otherValues]);
@@ -164,6 +173,12 @@ class I18nPlaceholderParams {
     }
     const context =
         value.subTemplateIndex === null ? '' : `${CONTEXT_MARKER}${value.subTemplateIndex}`;
+    // Self-closing tags use a special form that concatenates the start and close tag values.
+    if ((value.flags & I18nParamValueFlags.OpenTag) &&
+        (value.flags & I18nParamValueFlags.CloseTag)) {
+      return `${ESCAPE}${tagMarker}${value.value}${context}${ESCAPE}${ESCAPE}${closeMarker}${
+          tagMarker}${value.value}${context}${ESCAPE}`;
+    }
     return `${ESCAPE}${closeMarker}${tagMarker}${value.value}${context}${ESCAPE}`;
   }
 }
@@ -216,16 +231,22 @@ function resolvePlaceholders(
           currentI18nOp = null;
           break;
         case ir.OpKind.ElementStart:
-          // For elements with i18n placeholders, record its slot value in the params map under both
-          // the start and close placeholders.
+          // For elements with i18n placeholders, record its slot value in the params map under the
+          // corresponding tag start placeholder.
           if (op.i18nPlaceholder !== undefined) {
             if (currentI18nOp === null) {
               throw Error('i18n tag placeholder should only occur inside an i18n block');
             }
             elements.set(op.xref, op);
+            const {startName, closeName} = op.i18nPlaceholder;
+            let flags = I18nParamValueFlags.ElementTag | I18nParamValueFlags.OpenTag;
+            // For self-closing tags, there is no close tag placeholder. Instead, the start tag
+            // placeholder accounts for the start and close of the element.
+            if (closeName === '') {
+              flags |= I18nParamValueFlags.CloseTag;
+            }
             addParam(
-                params, currentI18nOp, op.i18nPlaceholder.startName, op.slot!,
-                currentI18nOp.subTemplateIndex, I18nParamValueFlags.ElementTag);
+                params, currentI18nOp, startName, op.slot!, currentI18nOp.subTemplateIndex, flags);
           }
           break;
         case ir.OpKind.ElementEnd:
@@ -234,10 +255,13 @@ function resolvePlaceholders(
             if (currentI18nOp === null) {
               throw Error('i18n tag placeholder should only occur inside an i18n block');
             }
-            addParam(
-                params, currentI18nOp, startOp.i18nPlaceholder.closeName, startOp.slot!,
-                currentI18nOp.subTemplateIndex,
-                I18nParamValueFlags.ElementTag | I18nParamValueFlags.CloseTag);
+            const {closeName} = startOp.i18nPlaceholder;
+            // Self-closing tags don't have a closing tag placeholder.
+            if (closeName !== '') {
+              addParam(
+                  params, currentI18nOp, closeName, startOp.slot!, currentI18nOp.subTemplateIndex,
+                  I18nParamValueFlags.ElementTag | I18nParamValueFlags.CloseTag);
+            }
           }
           break;
         case ir.OpKind.Template: