Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 19 additions & 97 deletions crates/oxc_angular_compiler/src/pipeline/ingest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>).
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_angular_compiler/src/transform/html_to_r3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
});
}
}
Expand All @@ -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)),
});
}
}
Expand Down
153 changes: 153 additions & 0 deletions crates/oxc_angular_compiler/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
Loading