diff --git a/crates/oxc_angular_compiler/src/pipeline/ingest.rs b/crates/oxc_angular_compiler/src/pipeline/ingest.rs index ae9428f1a..273bacb10 100644 --- a/crates/oxc_angular_compiler/src/pipeline/ingest.rs +++ b/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., `
`) 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
). diff --git a/crates/oxc_angular_compiler/src/transform/html_to_r3.rs b/crates/oxc_angular_compiler/src/transform/html_to_r3.rs index 532f09672..e6d2f9042 100644 --- a/crates/oxc_angular_compiler/src/transform/html_to_r3.rs +++ b/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)), }); } } diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 61156d9d0..3365bccbe 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/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: '', +}) +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) {}`, +}) +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: ` + + + `, +}) +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}"); +}