fix: i18n ICU expression ordering with pipes (#5)

R3Icu used HashMap for vars and placeholders, which does not preserve
insertion order. JavaScript objects (used in Angular's ngtsc) do preserve
insertion order. When ICU cases share interpolation expressions, the
HashMap's non-deterministic iteration caused pipe expressions to be
emitted before plain expressions in the i18nExp chain.

Changed R3Icu.vars and R3Icu.placeholders from HashMap to ordered
Vec<(Atom, ...)> with deduplication helpers matching JS object semantics.

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

Author: Brooooooklyn

crates/oxc_angular_compiler/src/ast/r3.rs

@@ -1361,10 +1361,10 @@ pub struct R3HostElement<'a> {
 /// An ICU message for internationalization.
 #[derive(Debug)]
 pub struct R3Icu<'a> {
-    /// Variable expressions.
-    pub vars: HashMap<'a, Atom<'a>, R3BoundText<'a>>,
-    /// Placeholder expressions.
-    pub placeholders: HashMap<'a, Atom<'a>, R3IcuPlaceholder<'a>>,
+    /// Variable expressions (ordered: must preserve insertion order like JS objects).
+    pub vars: Vec<'a, (Atom<'a>, R3BoundText<'a>)>,
+    /// Placeholder expressions (ordered: must preserve insertion order like JS objects).
+    pub placeholders: Vec<'a, (Atom<'a>, R3IcuPlaceholder<'a>)>,
     /// Source span.
     pub source_span: Span,
     /// i18n metadata.

crates/oxc_angular_compiler/src/transform/html_to_r3.rs

@@ -99,6 +99,34 @@ pub struct TransformOptions {
     pub collect_comment_nodes: bool,
 }
 
+/// Inserts or updates a var entry in an ordered Vec, preserving first-insertion order.
+/// This matches JS object semantics where reassigning an existing key keeps its position.
+fn ordered_insert_var<'a>(
+    vec: &mut Vec<'a, (Atom<'a>, R3BoundText<'a>)>,
+    key: Atom<'a>,
+    value: R3BoundText<'a>,
+) {
+    if let Some(existing) = vec.iter_mut().find(|(k, _)| *k == key) {
+        existing.1 = value;
+    } else {
+        vec.push((key, value));
+    }
+}
+
+/// Inserts or updates a placeholder entry in an ordered Vec, preserving first-insertion order.
+/// This matches JS object semantics where reassigning an existing key keeps its position.
+fn ordered_insert_placeholder<'a>(
+    vec: &mut Vec<'a, (Atom<'a>, R3IcuPlaceholder<'a>)>,
+    key: Atom<'a>,
+    value: R3IcuPlaceholder<'a>,
+) {
+    if let Some(existing) = vec.iter_mut().find(|(k, _)| *k == key) {
+        existing.1 = value;
+    } else {
+        vec.push((key, value));
+    }
+}
+
 /// Transforms HTML AST to R3 AST.
 pub struct HtmlToR3Transform<'a> {
     allocator: &'a Allocator,
@@ -1250,7 +1278,7 @@ impl<'a> HtmlToR3Transform<'a> {
         };
 
         // Create variable for the switch value (using VAR_* placeholder name)
-        let mut vars = HashMap::new_in(self.allocator);
+        let mut vars = Vec::new_in(self.allocator);
         let switch_value_str = expansion.switch_value.as_str();
         let switch_value_span = expansion.switch_value_span;
 
@@ -1262,7 +1290,7 @@ impl<'a> HtmlToR3Transform<'a> {
         // This matches Angular's visitExpansion behavior where nested ICUs are visited first,
         // and their VAR_* placeholders are added before the outer ICU's VAR_*.
         // Ported from Angular's i18n_parser.ts:137-159
-        let mut placeholders = HashMap::new_in(self.allocator);
+        let mut placeholders = Vec::new_in(self.allocator);
         for case in expansion.cases.iter() {
             self.extract_placeholders_from_nodes(&case.expansion, &mut placeholders, &mut vars);
         }
@@ -1271,7 +1299,8 @@ impl<'a> HtmlToR3Transform<'a> {
         // This ensures the correct order: nested ICU vars first, then outer ICU var.
         // Use the unique VAR_* placeholder name as the key, matching Angular's behavior.
         // The expression_placeholder was already generated above with getUniquePlaceholder.
-        vars.insert(
+        ordered_insert_var(
+            &mut vars,
             expression_placeholder.clone(),
             R3BoundText { value: parse_result.ast, source_span: switch_value_span, i18n: None },
         );
@@ -1290,8 +1319,8 @@ impl<'a> HtmlToR3Transform<'a> {
     fn extract_placeholders_from_nodes(
         &mut self,
         nodes: &[HtmlNode<'a>],
-        placeholders: &mut HashMap<'a, Atom<'a>, R3IcuPlaceholder<'a>>,
-        vars: &mut HashMap<'a, Atom<'a>, R3BoundText<'a>>,
+        placeholders: &mut Vec<'a, (Atom<'a>, R3IcuPlaceholder<'a>)>,
+        vars: &mut Vec<'a, (Atom<'a>, R3BoundText<'a>)>,
     ) {
         for node in nodes {
             match node {
@@ -1335,10 +1364,11 @@ impl<'a> HtmlToR3Transform<'a> {
                     // Use the unique VAR_* placeholder name as the key, not the raw switch value.
                     // This is critical: when multiple nested ICUs have the same switch value
                     // (e.g., same pipe expression), they MUST have separate entries in the vars
-                    // HashMap. Angular uses unique placeholder names (VAR_SELECT, VAR_SELECT_1,
+                    // collection. Angular uses unique placeholder names (VAR_SELECT, VAR_SELECT_1,
                     // VAR_SELECT_2) to ensure each nested ICU creates its own TextOp with its
                     // own pipe slot allocation.
-                    vars.insert(
+                    ordered_insert_var(
+                        vars,
                         var_placeholder_name,
                         R3BoundText {
                             value: parse_result.ast,
@@ -1357,7 +1387,7 @@ impl<'a> HtmlToR3Transform<'a> {
         &mut self,
         text: &str,
         base_span: Span,
-        placeholders: &mut HashMap<'a, Atom<'a>, R3IcuPlaceholder<'a>>,
+        placeholders: &mut Vec<'a, (Atom<'a>, R3IcuPlaceholder<'a>)>,
     ) {
         // Use default Angular interpolation markers
         let start_marker = "{{";
@@ -1395,7 +1425,11 @@ impl<'a> HtmlToR3Transform<'a> {
                         source_span: interp_span,
                         i18n: None,
                     };
-                    placeholders.insert(placeholder_key, R3IcuPlaceholder::BoundText(bound_text));
+                    ordered_insert_placeholder(
+                        placeholders,
+                        placeholder_key,
+                        R3IcuPlaceholder::BoundText(bound_text),
+                    );
 
                     pos = abs_end;
                 } else {

crates/oxc_angular_compiler/tests/integration_test.rs

@@ -19,8 +19,8 @@ use oxc_span::Atom;
 fn compile_template_to_js(template: &str, component_name: &str) -> String {
     let allocator = Allocator::default();
 
-    // Stage 1: Parse HTML
-    let parser = HtmlParser::new(&allocator, template, "test.html");
+    // Stage 1: Parse HTML (with expansion forms enabled for ICU/plural support)
+    let parser = HtmlParser::with_expansion_forms(&allocator, template, "test.html");
     let html_result = parser.parse();
 
     // Check for parse errors
@@ -3817,6 +3817,58 @@ export class TestComponent {
     );
 }
 
+/// Tests that i18n expressions with pipes maintain the correct template order.
+/// When a text node inside an i18n span contains both plain expressions and pipe expressions,
+/// the expressions must be emitted in their original template order.
+/// Ported from Angular compliance test: r3_view_compiler_i18n/multiple_pipes.ts
+#[test]
+fn test_i18n_expression_ordering_with_pipes() {
+    let js = compile_template_to_js(
+        r#"<span i18n>{{ a }} and {{ b }} and {{ c }} and {{ b | uppercase }}</span>"#,
+        "TestComponent",
+    );
+
+    // Debug: print full output
+    // The i18nExp calls should follow template order:
+    // ctx.a, ctx.b, ctx.c, pipeBind1(..., ctx.b)
+    assert!(
+        js.contains("i18nExp(ctx.a)(ctx.b)(ctx.c)"),
+        "Expressions should be in template order. Full output:\n{js}"
+    );
+}
+
+/// Tests i18n expression ordering with ICU plural containing both plain and pipe expressions.
+/// The credits-tooltip pattern: an i18n block with text interpolation + ICU plural where
+/// the "other" case has a pipe expression that must come AFTER the plain expression.
+///
+/// Expected expression order (matching Angular ngtsc):
+///   i18nExp(ctx.name)(ctx.count)(ctx.amount)(pipeBind1(..., ctx.count))
+///
+/// Bug: OXC was emitting pipeBind1 before ctx.amount (swapping expressions 2 and 3).
+#[test]
+fn test_i18n_expression_ordering_icu_plural_with_pipe() {
+    let js = compile_template_to_js(
+        r#"<div i18n>{{ name }} {count, plural, =1 {({{ amount }} credits x 1 user)} other {({{ amount }} credits x {{ count | number }} users)}}</div>"#,
+        "TestComponent",
+    );
+
+    // Extract the update block to check i18nExp ordering
+    let update_start = js.find("if ((rf & 2))").expect("should have update block");
+    let update_block = &js[update_start..];
+
+    // The plain expression (ctx.amount) must come BEFORE the pipe expression (pipeBind1)
+    // in the i18nExp chain. This matches Angular ngtsc behavior.
+    let amount_pos = update_block.find("ctx.amount").expect("should have ctx.amount in i18nExp");
+    let pipe_pos = update_block.find("pipeBind1").expect("should have pipeBind1 in i18nExp");
+
+    assert!(
+        amount_pos < pipe_pos,
+        "ctx.amount (plain expression) must come before pipeBind1 (pipe expression) in i18nExp chain.\n\
+         amount_pos={amount_pos}, pipe_pos={pipe_pos}\n\
+         Update block:\n{update_block}"
+    );
+}
+
 #[test]
 fn test_nested_if_listener_ctx_reference() {
     // Test: nested @if where a listener in the inner @if accesses component properties.