refactor(compiler): Fix handling of namespaced attributes (angular#53646

) It's possible for attributes to have a namespace, we need to handle this possiblity for both attribute instructions and attributes extracted to the consts array. PR Close angular#53646
danieljancar · Jan 26, 2024 · b5c5b4d · b5c5b4d
1 parent 6f7b82c
commit b5c5b4d
Show file tree

Hide file tree

Showing 13 changed files with 87 additions and 36 deletions.
diff --git a/...compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/GOLDEN_PARTIAL.js b/...compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/GOLDEN_PARTIAL.js
@@ -718,3 +718,37 @@ export declare class MyComponent {
     static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never, false, never>;
 }
 
+/****************************************************************************************************
+ * PARTIAL FILE: namespace_attr.js
+ ****************************************************************************************************/
+import { Component } from '@angular/core';
+import * as i0 from "@angular/core";
+export class MyComponent {
+}
+MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
+MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-component", ngImport: i0, template: `
+    <svg:use [attr.xlink:href]="value"/>
+    <svg:use id="foo" xlink:href="/foo" name="foo"/>
+  `, isInline: true });
+i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{
+            type: Component,
+            args: [{
+                    selector: 'my-component',
+                    standalone: true,
+                    template: `
+    <svg:use [attr.xlink:href]="value"/>
+    <svg:use id="foo" xlink:href="/foo" name="foo"/>
+  `
+                }]
+        }] });
+
+/****************************************************************************************************
+ * PARTIAL FILE: namespace_attr.d.ts
+ ****************************************************************************************************/
+import * as i0 from "@angular/core";
+export declare class MyComponent {
+    value: any;
+    static ɵfac: i0.ɵɵFactoryDeclaration<MyComponent, never>;
+    static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never, true, never>;
+}
+
diff --git a/...s/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/TEST_CASES.json b/...s/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/TEST_CASES.json
@@ -275,8 +275,7 @@
         {
           "failureMessage": "Incorrect generated template."
         }
-      ],
-      "focusTest": true
+      ]
     }
   ]
 }
diff --git a/...compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.js b/...compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.js
@@ -1,9 +1,9 @@
-consts: [["id", ""]],
+consts: [["id", "foo", __AttributeMarker.NamespaceURI__, "xlink", "href", "/foo", "name", "foo"]],
 template: function MyComponent_Template(rf, ctx) {
 	if (rf & 1) {
 		i0.ɵɵnamespaceSVG();
-		i0.ɵɵelement(0, "use", null, 0);
+		i0.ɵɵelement(0, "use")(1, "use", 0);
 	} if (rf & 2) {
 		i0.ɵɵattribute("href", ctx.value, null, "xlink");
 	}
-}
+}
diff --git a/...compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.ts b/...compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.ts
@@ -3,7 +3,10 @@ import {Component} from '@angular/core';
 @Component({
   selector: 'my-component',
   standalone: true,
-  template: `<svg:use [attr.xlink:href]="value" #id/>`
+  template: `
+    <svg:use [attr.xlink:href]="value"/>
+    <svg:use id="foo" xlink:href="/foo" name="foo"/>
+  `
 })
 export class MyComponent {
   value: any;

diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts
@@ -683,6 +683,11 @@ export interface ExtractedAttributeOp extends Op<CreateOp> {
    */
   bindingKind: BindingKind;
 
+  /**
+   * The namespace of the attribute (or null if none).
+   */
+  namespace: string|null;
+
   /**
    * The name of the extracted attribute.
    */
@@ -716,13 +721,14 @@ export interface ExtractedAttributeOp extends Op<CreateOp> {
  * Create an `ExtractedAttributeOp`.
  */
 export function createExtractedAttributeOp(
-    target: XrefId, bindingKind: BindingKind, name: string, expression: o.Expression|null,
-    i18nContext: XrefId|null, i18nMessage: i18n.Message|null,
+    target: XrefId, bindingKind: BindingKind, namespace: string|null, name: string,
+    expression: o.Expression|null, i18nContext: XrefId|null, i18nMessage: i18n.Message|null,
     securityContext: SecurityContext|SecurityContext[]): ExtractedAttributeOp {
   return {
     kind: OpKind.ExtractedAttribute,
     target,
     bindingKind,
+    namespace,
     name,
     expression,
     i18nContext,

diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/update.ts b/packages/compiler/src/template/pipeline/ir/src/ops/update.ts
@@ -406,6 +406,11 @@ export interface AttributeOp extends Op<UpdateOp> {
    */
   target: XrefId;
 
+  /**
+   * The namespace of the attribute (or null if none).
+   */
+  namespace: string|null;
+
   /**
    * The name of the attribute.
    */
@@ -456,13 +461,14 @@ export interface AttributeOp extends Op<UpdateOp> {
  * Create an `AttributeOp`.
  */
 export function createAttributeOp(
-    target: XrefId, name: string, expression: o.Expression|Interpolation,
+    target: XrefId, namespace: string|null, name: string, expression: o.Expression|Interpolation,
     securityContext: SecurityContext|SecurityContext[], isTextAttribute: boolean,
     isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
     i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): AttributeOp {
   return {
     kind: OpKind.Attribute,
     target,
+    namespace,
     name,
     expression,
     securityContext,

diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts
@@ -967,7 +967,7 @@ function ingestTemplateBindings(
       // Animation bindings are excluded from the structural template's const array.
       const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, output.name, false);
       unit.create.push(ir.createExtractedAttributeOp(
-          op.xref, ir.BindingKind.Property, output.name, null, null, null, securityContext));
+          op.xref, ir.BindingKind.Property, null, output.name, null, null, null, securityContext));
     }
   }
 
@@ -1024,7 +1024,7 @@ function createTemplateBinding(
       // the ng-template's consts (e.g. for the purposes of directive matching). However, we should
       // not generate an update instruction for it.
       return ir.createExtractedAttributeOp(
-          xref, ir.BindingKind.Property, name, null, null, i18nMessage, securityContext);
+          xref, ir.BindingKind.Property, null, name, null, null, i18nMessage, securityContext);
     }
 
     if (!isTextBinding && (type === e.BindingType.Attribute || type === e.BindingType.Animation)) {

diff --git a/packages/compiler/src/template/pipeline/src/instruction.ts b/packages/compiler/src/template/pipeline/src/instruction.ts
@@ -331,10 +331,14 @@ export function property(
 }
 
 export function attribute(
-    name: string, expression: o.Expression, sanitizer: o.Expression|null): ir.UpdateOp {
+    name: string, expression: o.Expression, sanitizer: o.Expression|null,
+    namespace: string|null): ir.UpdateOp {
   const args = [o.literal(name), expression];
-  if (sanitizer !== null) {
-    args.push(sanitizer);
+  if (sanitizer !== null || namespace !== null) {
+    args.push(sanitizer ?? o.literal(null));
+  }
+  if (namespace !== null) {
+    args.push(o.literal(namespace));
   }
   return call(Identifiers.attribute, args, null);
 }

diff --git a/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts b/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts
@@ -40,7 +40,8 @@ export function extractAttributes(job: CompilationJob): void {
             ir.OpList.insertBefore<ir.CreateOp>(
                 // Deliberaly null i18nMessage value
                 ir.createExtractedAttributeOp(
-                    op.target, bindingKind, op.name, /* expression */ null, /* i18nContext */ null,
+                    op.target, bindingKind, null, op.name, /* expression */ null,
+                    /* i18nContext */ null,
                     /* i18nMessage */ null, op.securityContext),
                 lookupElement(elements, op.target));
           }
@@ -56,7 +57,7 @@ export function extractAttributes(job: CompilationJob): void {
               op.expression instanceof ir.EmptyExpr) {
             ir.OpList.insertBefore<ir.CreateOp>(
                 ir.createExtractedAttributeOp(
-                    op.target, ir.BindingKind.Property, op.name, /* expression */ null,
+                    op.target, ir.BindingKind.Property, null, op.name, /* expression */ null,
                     /* i18nContext */ null,
                     /* i18nMessage */ null, SecurityContext.STYLE),
                 lookupElement(elements, op.target));
@@ -65,7 +66,7 @@ export function extractAttributes(job: CompilationJob): void {
         case ir.OpKind.Listener:
           if (!op.isAnimationListener) {
             const extractedAttributeOp = ir.createExtractedAttributeOp(
-                op.target, ir.BindingKind.Property, op.name, /* expression */ null,
+                op.target, ir.BindingKind.Property, null, op.name, /* expression */ null,
                 /* i18nContext */ null,
                 /* i18nMessage */ null, SecurityContext.NONE);
             if (job.kind === CompilationJobKind.Host) {
@@ -122,7 +123,7 @@ function extractAttributeOp(
     const extractedAttributeOp = ir.createExtractedAttributeOp(
         op.target,
         op.isStructuralTemplateAttribute ? ir.BindingKind.Template : ir.BindingKind.Attribute,
-        op.name, op.expression, op.i18nContext, op.i18nMessage, op.securityContext);
+        op.namespace, op.name, op.expression, op.i18nContext, op.i18nMessage, op.securityContext);
     if (unit.job.kind === CompilationJobKind.Host) {
       // This attribute will apply to the enclosing host binding compilation unit, so order doesn't
       // matter.

diff --git a/packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts b/packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts
@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {splitNsName} from '../../../../ml_parser/tags';
 import * as ir from '../../ir';
 import {CompilationJob, CompilationJobKind} from '../compilation';
 
@@ -44,12 +45,13 @@ export function specializeBindings(job: CompilationJob): void {
             const target = lookupElement(elements, op.target);
             target.nonBindable = true;
           } else {
+            const [namespace, name] = splitNsName(op.name);
             ir.OpList.replace<ir.UpdateOp>(
                 op,
                 ir.createAttributeOp(
-                    op.target, op.name, op.expression, op.securityContext, op.isTextAttribute,
-                    op.isStructuralTemplateAttribute, op.templateKind, op.i18nMessage,
-                    op.sourceSpan));
+                    op.target, namespace, name, op.expression, op.securityContext,
+                    op.isTextAttribute, op.isStructuralTemplateAttribute, op.templateKind,
+                    op.i18nMessage, op.sourceSpan));
           }
           break;
         case ir.BindingKind.Property:

diff --git a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts
@@ -8,7 +8,6 @@
 
 
 import * as core from '../../../../core';
-import {splitNsName} from '../../../../ml_parser/tags';
 import * as o from '../../../../output/output_ast';
 import * as ir from '../../ir';
 
@@ -28,7 +27,7 @@ export function collectElementConsts(job: CompilationJob): void {
         const attributes =
             allElementAttributes.get(op.target) || new ElementAttributes(job.compatibility);
         allElementAttributes.set(op.target, attributes);
-        attributes.add(op.bindingKind, op.name, op.expression, op.trustedValueFn);
+        attributes.add(op.bindingKind, op.name, op.expression, op.namespace, op.trustedValueFn);
         ir.OpList.remove<ir.CreateOp>(op);
       }
     }
@@ -139,7 +138,7 @@ class ElementAttributes {
     return false;
   }
 
-  add(kind: ir.BindingKind, name: string, value: o.Expression|null,
+  add(kind: ir.BindingKind, name: string, value: o.Expression|null, namespace: string|null,
       trustedValueFn: o.Expression|null): void {
     // TemplateDefinitionBuilder puts duplicate attribute, class, and style values into the consts
     // array. This seems inefficient, we can probably keep just the first one or the last value
@@ -164,7 +163,7 @@ class ElementAttributes {
 
 
     const array = this.arrayFor(kind);
-    array.push(...getAttributeNameLiterals(name));
+    array.push(...getAttributeNameLiterals(namespace, name));
     if (kind === ir.BindingKind.Attribute || kind === ir.BindingKind.StyleProperty) {
       if (value === null) {
         throw Error('Attribute, i18n attribute, & style element attributes must have a value');
@@ -193,14 +192,11 @@ class ElementAttributes {
 /**
  * Gets an array of literal expressions representing the attribute's namespaced name.
  */
-function getAttributeNameLiterals(name: string): o.LiteralExpr[] {
-  const [attributeNamespace, attributeName] = splitNsName(name, false);
-  const nameLiteral = o.literal(attributeName);
-
-  if (attributeNamespace) {
-    return [
-      o.literal(core.AttributeMarker.NamespaceURI), o.literal(attributeNamespace), nameLiteral
-    ];
+function getAttributeNameLiterals(namespace: string|null, name: string): o.LiteralExpr[] {
+  const nameLiteral = o.literal(name);
+
+  if (namespace) {
+    return [o.literal(core.AttributeMarker.NamespaceURI), o.literal(namespace), nameLiteral];
   }
 
   return [nameLiteral];

diff --git a/packages/compiler/src/template/pipeline/src/phases/parse_extracted_styles.ts b/packages/compiler/src/template/pipeline/src/phases/parse_extracted_styles.ts
@@ -49,7 +49,7 @@ export function parseExtractedStyles(job: CompilationJob) {
           for (let i = 0; i < parsedStyles.length - 1; i += 2) {
             ir.OpList.insertBefore<ir.CreateOp>(
                 ir.createExtractedAttributeOp(
-                    op.target, ir.BindingKind.StyleProperty, parsedStyles[i],
+                    op.target, ir.BindingKind.StyleProperty, null, parsedStyles[i],
                     o.literal(parsedStyles[i + 1]), null, null, SecurityContext.STYLE),
                 op);
           }
@@ -59,7 +59,7 @@ export function parseExtractedStyles(job: CompilationJob) {
           for (const parsedClass of parsedClasses) {
             ir.OpList.insertBefore<ir.CreateOp>(
                 ir.createExtractedAttributeOp(
-                    op.target, ir.BindingKind.ClassName, parsedClass, null, null, null,
+                    op.target, ir.BindingKind.ClassName, null, parsedClass, null, null, null,
                     SecurityContext.NONE),
                 op);
           }

diff --git a/packages/compiler/src/template/pipeline/src/phases/reify.ts b/packages/compiler/src/template/pipeline/src/phases/reify.ts
@@ -354,7 +354,7 @@ function reifyUpdateOperations(_unit: CompilationUnit, ops: ir.OpList<ir.UpdateO
                   op.name, op.expression.strings, op.expression.expressions, op.sanitizer,
                   op.sourceSpan));
         } else {
-          ir.OpList.replace(op, ng.attribute(op.name, op.expression, op.sanitizer));
+          ir.OpList.replace(op, ng.attribute(op.name, op.expression, op.sanitizer, op.namespace));
         }
         break;
       case ir.OpKind.HostProperty: