fix: preserve i18n metadata on structural directive attribute hoisting and split namespace attrs (#13)

When an element has both a structural directive (*ngIf) and i18n-translated
attributes, the attribute hoisting in filter_animation_attributes/inputs
was discarding i18n metadata (setting i18n: None). Additionally,
ingest_template used the non-i18n variant for hoisted attributes, and
namespace attributes (e.g. xmlns:xlink) were not split into namespace/name
pairs. Together these caused incorrect const deduplication and index shifts.

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: Brooooooklyn

crates/oxc_angular_compiler/src/pipeline/ingest.rs

@@ -1244,94 +1244,6 @@ fn ingest_element<'a>(
     }
 }
 
-/// Ingests static attributes from an element into BindingOp operations.
-///
-/// Static attributes (e.g., `<div class="foo" ngNonBindable>`) are ingested as
-/// BindingOp with `is_text_attribute: true` into the UPDATE list, just like
-/// Angular's TypeScript implementation. This allows binding_specialization
-/// to detect special attributes like `ngNonBindable` and handle them appropriately.
-///
-/// Ported from Angular's ingestElementBindings in template/pipeline/src/ingest.ts
-///
-/// `is_structural_template_attribute` - If true, use BindingKind::Template for the extracted
-/// attributes. This is needed for structural directive attributes like `*cdkPortal` where
-/// the attribute (cdkPortal) should be extracted with the Template marker, not as a regular
-/// Attribute.
-fn ingest_static_attributes<'a>(
-    job: &mut ComponentCompilationJob<'a>,
-    view_xref: XrefId,
-    element_xref: XrefId,
-    attributes: std::vec::Vec<(Atom<'a>, Atom<'a>)>,
-    is_structural_template_attribute: bool,
-) {
-    use crate::output::ast::{LiteralExpr, LiteralValue, OutputExpression};
-
-    let allocator = job.allocator;
-
-    for (name, value) in attributes {
-        // ngNonBindable and animate.* require special handling: they must be added to the
-        // update list as BindingOp so binding_specialization can detect and process them.
-        // - ngNonBindable: marks element as non-bindable
-        // - animate.*: converts to AnimationBindingOp for animation instructions
-        if name.as_str() == "ngNonBindable" || name.as_str().starts_with("animate.") {
-            let literal_expr = OutputExpression::Literal(Box::new_in(
-                LiteralExpr { value: LiteralValue::String(value), source_span: None },
-                allocator,
-            ));
-            let value_expr = IrExpression::OutputExpr(Box::new_in(literal_expr, allocator));
-
-            let binding = BindingOp {
-                base: UpdateOpBase::default(),
-                target: element_xref,
-                kind: BindingKind::Attribute,
-                name,
-                expression: Box::new_in(value_expr, allocator),
-                unit: None,
-                security_context: SecurityContext::None,
-                i18n_message: None,
-                is_text_attribute: true,
-            };
-
-            if let Some(view) = job.view_mut(view_xref) {
-                view.update.push(UpdateOp::Binding(binding));
-            }
-            continue;
-        }
-
-        // All other static attributes go to the create list as ExtractedAttributeOp
-        let literal_expr = OutputExpression::Literal(Box::new_in(
-            LiteralExpr { value: LiteralValue::String(value), source_span: None },
-            allocator,
-        ));
-        let value_expr = IrExpression::OutputExpr(Box::new_in(literal_expr, allocator));
-
-        // Use Template kind for structural template attributes, Attribute otherwise
-        let binding_kind = if is_structural_template_attribute {
-            BindingKind::Template
-        } else {
-            BindingKind::Attribute
-        };
-
-        let extracted = ExtractedAttributeOp {
-            base: CreateOpBase::default(),
-            target: element_xref,
-            binding_kind,
-            namespace: None,
-            name,
-            value: Some(Box::new_in(value_expr, allocator)),
-            security_context: SecurityContext::None,
-            truthy_expression: false,
-            i18n_context: None,
-            i18n_message: None,
-            trusted_value_fn: None,
-        };
-
-        if let Some(view) = job.view_mut(view_xref) {
-            view.create.push(CreateOp::ExtractedAttribute(extracted));
-        }
-    }
-}
-
 /// Ingests static attributes from R3TextAttribute, preserving i18n metadata.
 ///
 /// This version takes R3TextAttribute directly so it can access the i18n field.
@@ -1448,12 +1360,17 @@ fn ingest_static_attributes_with_i18n<'a>(
             BindingKind::Attribute
         };
 
+        // Split namespace from attribute name (e.g., `:xmlns:xlink` → namespace="xmlns", name="xlink")
+        let (ns, local_name) = split_ns_name(name.as_str());
+        let namespace = ns.map(|n| Atom::from(n));
+        let local_name = Atom::from(local_name);
+
         let extracted = ExtractedAttributeOp {
             base: CreateOpBase::default(),
             target: element_xref,
             binding_kind,
-            namespace: None,
-            name,
+            namespace,
+            name: local_name,
             value: Some(Box::new_in(value_expr, allocator)),
             security_context: SecurityContext::None,
             truthy_expression: false,
@@ -1523,12 +1440,17 @@ fn ingest_single_static_attribute<'a>(
         }
     } else {
         // For regular (non-structural) attributes, create ExtractedAttributeOp directly
+        // Split namespace from attribute name (e.g., `:xmlns:xlink` → namespace="xmlns", name="xlink")
+        let (ns, local_name) = split_ns_name(name.as_str());
+        let namespace = ns.map(|n| Atom::from(n));
+        let local_name = Atom::from(local_name);
+
         let extracted = ExtractedAttributeOp {
             base: CreateOpBase::default(),
             target: element_xref,
             binding_kind: BindingKind::Attribute,
-            namespace: None,
-            name,
+            namespace,
+            name: local_name,
             value: Some(Box::new_in(value_expr, allocator)),
             security_context: SecurityContext::None,
             truthy_expression: false,
@@ -2088,12 +2010,12 @@ fn ingest_template<'a>(
     }
 
     // Process hoisted static attributes from the wrapped element.
-    // Ported from Angular's `ingestTemplateBindings` - attributes processing (lines 1471-1489).
-    let static_attrs: std::vec::Vec<_> =
-        attributes.into_iter().map(|attr| (attr.name, attr.value)).collect();
-    if !static_attrs.is_empty() {
+    // Ported from Angular's `ingestTemplateBindings` - attributes processing (lines 1471-1501).
+    // IMPORTANT: Use ingest_static_attributes_with_i18n to preserve i18n metadata (attr.i18n).
+    // Angular's TS passes asMessage(attr.i18n) to createTemplateBinding (ingest.ts line 1497).
+    if !attributes.is_empty() {
         // Hoisted attributes from wrapped element are regular attributes, not Template
-        ingest_static_attributes(job, view_xref, xref, static_attrs, false);
+        ingest_static_attributes_with_i18n(job, view_xref, xref, &attributes, false);
     }
 
     // Process hoisted inputs from the wrapped element (e.g., [class]="..." on <div *ngIf>).

crates/oxc_angular_compiler/src/transform/html_to_r3.rs

@@ -3710,7 +3710,7 @@ impl<'a> HtmlToR3Transform<'a> {
                     source_span: attr.source_span,
                     key_span: attr.key_span,
                     value_span: attr.value_span,
-                    i18n: None,
+                    i18n: attr.i18n.as_ref().map(|meta| meta.clone_in(self.allocator)),
                 });
             }
         }
@@ -3735,7 +3735,7 @@ impl<'a> HtmlToR3Transform<'a> {
                     source_span: input.source_span,
                     key_span: input.key_span,
                     value_span: input.value_span,
-                    i18n: None,
+                    i18n: input.i18n.as_ref().map(|meta| meta.clone_in(self.allocator)),
                 });
             }
         }

crates/oxc_angular_compiler/tests/integration_test.rs

@@ -4221,3 +4221,156 @@ export class TestComponent {
         metadata_section
     );
 }
+
+// ============================================================================
+// Namespace Attribute Const Collection Tests
+// ============================================================================
+
+/// Test that SVG elements with namespace attributes (xmlns:xlink) produce correct consts.
+///
+/// When a static attribute like `xmlns:xlink="..."` is ingested, the name should be
+/// split into namespace="xmlns" and name="xlink" so that the consts array serializes it
+/// as [AttributeMarker.NamespaceUri, "xmlns", "xlink", "..."] instead of
+/// [":xmlns:xlink", "..."].
+///
+/// Without the fix, namespace attributes create duplicate consts entries because
+/// the unsplit `:xmlns:xlink` format doesn't match the properly-split format,
+/// preventing deduplication and shifting all subsequent consts indices.
+#[test]
+fn test_svg_namespace_attribute_consts() {
+    let allocator = Allocator::default();
+    let source = r#"
+import { Component } from '@angular/core';
+
+@Component({
+    selector: 'app-icon',
+    standalone: true,
+    template: '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" data-testid="icon"><use></use></svg>',
+})
+export class IconComponent {}
+"#;
+
+    let result = transform_angular_file(
+        &allocator,
+        "icon.component.ts",
+        source,
+        &ComponentTransformOptions::default(),
+        None,
+    );
+    assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
+    let code = &result.code;
+    eprintln!("OUTPUT:\n{code}");
+
+    // The consts array should NOT contain ":xmlns:xlink" as a raw string.
+    // Instead, namespace attributes should be serialized with the NamespaceUri marker (0).
+    assert!(
+        !code.contains(r#"":xmlns:xlink""#),
+        "Consts should NOT contain raw ':xmlns:xlink' string. Namespace should be split. Output:\n{code}"
+    );
+
+    // The consts array SHOULD contain the proper namespace marker format:
+    // 0 (NamespaceUri marker), "xmlns", "xlink"
+    assert!(
+        code.contains(r#"0,"xmlns","xlink""#),
+        "Consts should contain namespace marker format: 0,\"xmlns\",\"xlink\". Output:\n{code}"
+    );
+}
+
+/// Test that SVG with both namespace attributes and property bindings has correct consts indices.
+///
+/// This reproduces the real-world icon.component pattern where:
+/// - An @if conditional wraps the SVG (creating a template function)
+/// - The SVG has namespace attrs (xmlns:xlink) AND a property binding ([name])
+/// - Both the conditional template and the SVG element need consts entries
+///
+/// Without the fix, a duplicate consts entry is created for the SVG element,
+/// causing the template to reference the wrong consts index.
+#[test]
+fn test_svg_namespace_attrs_with_conditional_and_binding() {
+    let allocator = Allocator::default();
+    let source = r#"
+import { Component, Input } from '@angular/core';
+
+@Component({
+    selector: 'app-icon',
+    standalone: true,
+    template: `@if (showIcon) {<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" data-testid="icon" class="svg"><use></use></svg>}`,
+})
+export class IconComponent {
+    showIcon = true;
+}
+"#;
+
+    let result = transform_angular_file(
+        &allocator,
+        "icon.component.ts",
+        source,
+        &ComponentTransformOptions::default(),
+        None,
+    );
+    assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
+    let code = &result.code;
+    eprintln!("OUTPUT:\n{code}");
+
+    // Should NOT have duplicate consts entries. Count occurrences of "xmlns" in consts.
+    // With the bug, there would be two entries - one with proper namespace format,
+    // one with raw ":xmlns:xlink".
+    assert!(
+        !code.contains(r#"":xmlns:xlink""#),
+        "Consts should NOT contain raw ':xmlns:xlink' string. Output:\n{code}"
+    );
+}
+
+/// When an element has a structural directive (*ngIf) AND an i18n-translated attribute,
+/// the hoisted static attributes must preserve the i18n info. Without this, the literal
+/// text value is used in the consts array instead of the i18n variable reference, causing
+/// incorrect deduplication when multiple similar elements exist.
+///
+/// Ported to match Angular TS behavior: ingestTemplateBindings passes attr.i18n to
+/// createTemplateBinding for hoisted attributes (ingest.ts line 1497).
+#[test]
+fn test_i18n_attribute_on_structural_directive_element() {
+    let allocator = Allocator::default();
+    // Two buttons with *ngIf, both with i18n-cuTooltip but different custom IDs.
+    // Each should get its own i18n variable (i18n_0, i18n_1) in the consts array.
+    // Without the fix, both get literal "Clear date" and are deduplicated into one entry.
+    let source = r#"
+import { Component } from '@angular/core';
+
+@Component({
+    selector: 'app-test',
+    standalone: true,
+    template: `
+        <button *ngIf="showA" cuTooltip="Clear date" i18n-cuTooltip="@@clear-date-a" (click)="clearA()">Clear A</button>
+        <button *ngIf="showB" cuTooltip="Clear date" i18n-cuTooltip="@@clear-date-b" (click)="clearB()">Clear B</button>
+    `,
+})
+export class TestComponent {
+    showA = true;
+    showB = true;
+    clearA() {}
+    clearB() {}
+}
+"#;
+
+    let result = transform_angular_file(
+        &allocator,
+        "test.component.ts",
+        source,
+        &ComponentTransformOptions::default(),
+        None,
+    );
+    assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
+    let code = &result.code;
+    eprintln!("OUTPUT:\n{code}");
+
+    // Both buttons should use i18n variable references, not the literal "Clear date".
+    // The consts array should contain i18n_0 and i18n_1 (or similar), NOT literal "Clear date".
+    assert!(
+        !code.contains(r#""cuTooltip","Clear date""#),
+        "Consts should NOT contain literal 'Clear date' - should use i18n variable reference. Output:\n{code}"
+    );
+    // There should be two distinct i18n entries (i18n_0, i18n_1) for the two different @@ IDs
+    assert!(code.contains("i18n_0"), "Should have i18n_0 variable. Output:\n{code}");
+    assert!(code.contains("i18n_1"), "Should have i18n_1 variable. Output:\n{code}");
+}