fix(compiler): avoid generating i18n attributes in plain form (#36422)

Prior to this change, there was a problem while matching template attributes, which mistakenly took i18n attributes (that might be present in attrs array after template ones) into account. This commit updates the logic to avoid template attribute matching logic from entering the i18n section and as a result this also allows generating proper i18n attributes sections instead of keeping these attribute in plain form (with their values) in attribute arrays.

PR Close #36422

packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts

@@ -337,8 +337,6 @@ describe('i18n support in the template compiler', () => {
         <ng-template i18n-title title="Hello"></ng-template>
       `;
 
-      // TODO (FW-1942): update the code to avoid adding `title` attribute in plain form
-      // into the `consts` array on Component def.
       const output = String.raw`
         var $I18N_0$;
         if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
@@ -350,7 +348,7 @@ describe('i18n support in the template compiler', () => {
         }
         const $_c2$ = ["title", $I18N_0$];
         …
-        consts: [["title", "Hello"]],
+        consts: [[${AttributeMarker.I18n}, "title"]],
         template: function MyComponent_Template(rf, ctx) {
           if (rf & 1) {
             $r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 0);
@@ -367,8 +365,6 @@ describe('i18n support in the template compiler', () => {
             <ng-template *ngIf="visible" i18n-title title="Hello">Test</ng-template>
           `;
 
-         // TODO (FW-1942): update the code to avoid adding `title` attribute in plain form
-         // into the `consts` array on Component def.
          const output = String.raw`
             var $I18N_0$;
             if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
@@ -391,7 +387,7 @@ describe('i18n support in the template compiler', () => {
               }
             }
             …
-            consts: [[${AttributeMarker.Template}, "ngIf"], ["title", "Hello"]],
+            consts: [[${AttributeMarker.Template}, "ngIf"], [${AttributeMarker.I18n}, "title"]],
             template: function MyComponent_Template(rf, ctx) {
               if (rf & 1) {
                 $r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 0, undefined, 0);

packages/compiler/src/render3/view/i18n/util.ts

@@ -9,6 +9,7 @@ import * as i18n from '../../../i18n/i18n_ast';
 import {toPublicName} from '../../../i18n/serializers/xmb';
 import * as html from '../../../ml_parser/ast';
 import * as o from '../../../output/output_ast';
+import * as t from '../../r3_ast';
 
 /* Closure variables holding messages must be named `MSG_[A-Z0-9]+` */
 const CLOSURE_TRANSLATION_PREFIX = 'MSG_';
@@ -41,6 +42,10 @@ export function isSingleI18nIcu(meta?: i18n.I18nMeta): boolean {
   return isI18nRootNode(meta) && meta.nodes.length === 1 && meta.nodes[0] instanceof i18n.Icu;
 }
 
+export function hasI18nMeta(node: t.Node&{i18n?: i18n.I18nMeta}): boolean {
+  return !!node.i18n;
+}
+
 export function hasI18nAttrs(element: html.Element): boolean {
   return element.attrs.some((attr: html.Attribute) => isI18nAttribute(attr.name));
 }

packages/compiler/src/render3/view/template.ts

@@ -26,7 +26,7 @@ import {ParseError, ParseSourceSpan} from '../../parse_util';
 import {DomElementSchemaRegistry} from '../../schema/dom_element_schema_registry';
 import {CssSelector, SelectorMatcher} from '../../selector';
 import {BindingParser} from '../../template_parser/binding_parser';
-import {error} from '../../util';
+import {error, partitionArray} from '../../util';
 import * as t from '../r3_ast';
 import {Identifiers as R3} from '../r3_identifiers';
 import {htmlAstToRender3Ast} from '../r3_template_transform';
@@ -36,7 +36,7 @@ import {I18nContext} from './i18n/context';
 import {createGoogleGetMsgStatements} from './i18n/get_msg_utils';
 import {createLocalizeStatements} from './i18n/localize_utils';
 import {I18nMetaVisitor} from './i18n/meta';
-import {assembleBoundTextPlaceholders, assembleI18nBoundString, declareI18nVariable, getTranslationConstPrefix, I18N_ICU_MAPPING_PREFIX, i18nFormatPlaceholderNames, icuFromI18nMessage, isI18nRootNode, isSingleI18nIcu, placeholdersToParams, TRANSLATION_PREFIX, wrapI18nPlaceholder} from './i18n/util';
+import {assembleBoundTextPlaceholders, assembleI18nBoundString, declareI18nVariable, getTranslationConstPrefix, hasI18nMeta, I18N_ICU_MAPPING_PREFIX, i18nFormatPlaceholderNames, icuFromI18nMessage, isI18nRootNode, isSingleI18nIcu, placeholdersToParams, TRANSLATION_PREFIX, wrapI18nPlaceholder} from './i18n/util';
 import {StylingBuilder, StylingInstruction} from './styling_builder';
 import {asLiteral, chainedInstruction, CONTEXT_NAME, getAttrsForDirectiveMatching, getInterpolationArgsLength, IMPLICIT_REFERENCE, invalid, NON_BINDABLE_ATTR, REFERENCE_PREFIX, RENDER_FLAGS, trimTrailingNulls, unsupported} from './util';
 
@@ -848,11 +848,10 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     this.matchDirectives(NG_TEMPLATE_TAG_NAME, template);
 
     // prepare attributes parameter (including attributes used for directive matching)
-    // TODO (FW-1942): exclude i18n attributes from the main attribute list and pass them
-    // as an `i18nAttrs` argument of the `getAttributeExpressions` function below.
+    const [i18nStaticAttrs, staticAttrs] = partitionArray(template.attributes, hasI18nMeta);
     const attrsExprs: o.Expression[] = this.getAttributeExpressions(
-        template.attributes, template.inputs, template.outputs, undefined, template.templateAttrs,
-        undefined);
+        staticAttrs, template.inputs, template.outputs, undefined /* styles */,
+        template.templateAttrs, i18nStaticAttrs);
     parameters.push(this.addAttrsToConsts(attrsExprs));
 
     // local refs (ex.: <ng-template #foo>)
@@ -896,12 +895,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
 
     // Only add normal input/output binding instructions on explicit <ng-template> elements.
     if (template.tagName === NG_TEMPLATE_TAG_NAME) {
-      const inputs: t.BoundAttribute[] = [];
-      const i18nAttrs: (t.TextAttribute|t.BoundAttribute)[] =
-          template.attributes.filter(attr => !!attr.i18n);
-
-      template.inputs.forEach(
-          (input: t.BoundAttribute) => (input.i18n ? i18nAttrs : inputs).push(input));
+      const [i18nInputs, inputs] = partitionArray(template.inputs, hasI18nMeta);
+      const i18nAttrs = [...i18nStaticAttrs, ...i18nInputs];
 
       // Add i18n attributes that may act as inputs to directives. If such attributes are present,
       // generate `i18nAttributes` instruction. Note: we generate it only for explicit <ng-template>

packages/compiler/src/util.ts

@@ -270,4 +270,22 @@ export function newArray<T>(size: number, value?: T): T[] {
     list.push(value!);
   }
   return list;
+}
+
+/**
+ * Partitions a given array into 2 arrays, based on a boolean value returned by the condition
+ * function.
+ *
+ * @param arr Input array that should be partitioned
+ * @param conditionFn Condition function that is called for each item in a given array and returns a
+ * boolean value.
+ */
+export function partitionArray<T>(
+    arr: T[], conditionFn: <K extends T>(value: K) => boolean): [T[], T[]] {
+  const truthy: T[] = [];
+  const falsy: T[] = [];
+  arr.forEach(item => {
+    (conditionFn(item) ? truthy : falsy).push(item);
+  });
+  return [truthy, falsy];
 }

packages/compiler/test/util_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {escapeRegExp, splitAtColon, stringify, utf8Encode} from '../src/util';
+import {escapeRegExp, partitionArray, splitAtColon, stringify, utf8Encode} from '../src/util';
 
 {
   describe('util', () => {
@@ -83,5 +83,21 @@ import {escapeRegExp, splitAtColon, stringify, utf8Encode} from '../src/util';
         expect(stringify(Object.create(null))).toEqual('object');
       });
     });
+
+    describe('partitionArray()', () => {
+      it('should handle empty arrays', () => {
+        expect(partitionArray([], () => true)).toEqual([[], []]);
+      });
+
+      it('should handle arrays with primitive type values', () => {
+        expect(partitionArray([1, 2, 3], (el: number) => el < 2)).toEqual([[1], [2, 3]]);
+      });
+
+      it('should handle arrays of objects', () => {
+        expect(partitionArray([{id: 1}, {id: 2}, {id: 3}], (el: any) => el.id < 2)).toEqual([
+          [{id: 1}], [{id: 2}, {id: 3}]
+        ]);
+      });
+    });
   });
 }

packages/core/src/render3/node_selector_matcher.ts

@@ -290,7 +290,11 @@ function matchTemplateAttribute(attrs: TAttributes, name: string): number {
   if (i > -1) {
     i++;
     while (i < attrs.length) {
-      if (attrs[i] === name) return i;
+      const attr = attrs[i];
+      // Return in case we checked all template attrs and are switching to the next section in the
+      // attrs array (that starts with a number that represents an attribute marker).
+      if (typeof attr === 'number') return -1;
+      if (attr === name) return i;
       i++;
     }
   }