fix: defer timer consts placed after i18n consts in consts array (#21)

Brooooooklyn · claude · web-flow · commit aabfc01350e6 · 2026-02-10T15:34:56.000+08:00
Defer timer configs (e.g. [100, null] for @Loading minimum) were being added directly to the consts array in Phase 19 (defer_configs), placing them before i18n consts added in Phase 52. This caused all i18n const indices to be off by 1. Match Angular TS's approach: wrap timer configs in ConstCollectedExpr so they are resolved by Phase 53 (collectConstExpressions) which runs after Phase 52 (collectI18nConsts), ensuring correct ordering. Also fixes null vs 0 for missing timer values — Angular uses null for absent loadingAfterTime, not 0. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
diff --git a/crates/oxc_angular_compiler/src/ir/expression.rs b/crates/oxc_angular_compiler/src/ir/expression.rs
@@ -2147,6 +2147,20 @@ pub fn transform_expressions_in_create_op<'a, F>(
                 }
             }
         }
+        // Defer: transform loadingConfig and placeholderConfig expressions.
+        // Matches Angular TS expression.ts lines 1241-1251:
+        //   case OpKind.Defer:
+        //     if (op.loadingConfig !== null) { op.loadingConfig = transform(op.loadingConfig); }
+        //     if (op.placeholderConfig !== null) { op.placeholderConfig = transform(op.placeholderConfig); }
+        //     if (op.resolverFn !== null) { op.resolverFn = transform(op.resolverFn); }
+        CreateOp::Defer(op) => {
+            if let Some(ref mut config) = op.loading_config {
+                transform_expressions_in_expression(config.as_mut(), transform, flags);
+            }
+            if let Some(ref mut config) = op.placeholder_config {
+                transform_expressions_in_expression(config.as_mut(), transform, flags);
+            }
+        }
         // Operations without expressions (expressions are in the UPDATE op now)
         CreateOp::Conditional(_) => {}
         // Operations without expressions
@@ -2162,7 +2176,6 @@ pub fn transform_expressions_in_create_op<'a, F>(
         | CreateOp::EnableBindings(_)
         | CreateOp::Text(_)
         | CreateOp::Pipe(_)
-        | CreateOp::Defer(_)
         | CreateOp::I18nMessage(_)
         | CreateOp::Namespace(_)
         | CreateOp::ProjectionDef(_)
@@ -2329,6 +2342,15 @@ pub fn visit_expressions_in_create_op<'a, F>(
                 }
             }
         }
+        // Defer: visit loadingConfig and placeholderConfig expressions.
+        CreateOp::Defer(op) => {
+            if let Some(ref config) = op.loading_config {
+                visit_expressions_in_expression(config.as_ref(), visitor, flags);
+            }
+            if let Some(ref config) = op.placeholder_config {
+                visit_expressions_in_expression(config.as_ref(), visitor, flags);
+            }
+        }
         // Operations without expressions
         CreateOp::Conditional(_)
         | CreateOp::ListEnd(_)
@@ -2343,7 +2365,6 @@ pub fn visit_expressions_in_create_op<'a, F>(
         | CreateOp::EnableBindings(_)
         | CreateOp::Text(_)
         | CreateOp::Pipe(_)
-        | CreateOp::Defer(_)
         | CreateOp::I18nMessage(_)
         | CreateOp::Namespace(_)
         | CreateOp::ProjectionDef(_)
diff --git a/crates/oxc_angular_compiler/src/ir/ops.rs b/crates/oxc_angular_compiler/src/ir/ops.rs
@@ -1017,12 +1017,12 @@ pub struct DeferOp<'a> {
     pub loading_minimum_time: Option<u32>,
     /// Loading after time.
     pub loading_after_time: Option<u32>,
-    /// Placeholder config const index (points to [minimumTime] in consts array).
-    /// Set by defer_configs phase after adding to constant pool.
-    pub placeholder_config: Option<u32>,
-    /// Loading config const index (points to [minimumTime, afterTime] in consts array).
-    /// Set by defer_configs phase after adding to constant pool.
-    pub loading_config: Option<u32>,
+    /// Placeholder config expression (wraps [minimumTime] as ConstCollectedExpr).
+    /// Set by defer_configs phase; resolved to ConstReference by const collection phase.
+    pub placeholder_config: Option<Box<'a, IrExpression<'a>>>,
+    /// Loading config expression (wraps [minimumTime, afterTime] as ConstCollectedExpr).
+    /// Set by defer_configs phase; resolved to ConstReference by const collection phase.
+    pub loading_config: Option<Box<'a, IrExpression<'a>>>,
     /// Resolver function expression (after constant pool processing).
     /// This is the shared function reference created by resolve_defer_deps_fns.
     /// Corresponds to `resolverFn` in Angular TS.
diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/defer_configs.rs b/crates/oxc_angular_compiler/src/pipeline/phases/defer_configs.rs
@@ -3,18 +3,25 @@
 //! Configures @defer block instructions with trigger and dependency information.
 //!
 //! This phase processes DeferOp and DeferOnOp to:
-//! 1. Collect timing configs into the constant pool
+//! 1. Wrap timing configs in ConstCollectedExpr (resolved to consts later by Phase 53)
 //! 2. Link defer triggers to their target defer blocks
 //! 3. Set up main, placeholder, loading, and error template slots
 //! 4. Configure timing parameters (minimum time, loading after, etc.)
 //!
 //! Ported from Angular's `template/pipeline/src/phases/defer_configs.ts`.
+//!
+//! Key difference from old approach: instead of calling `job.add_const()` here (which
+//! places timer configs before i18n consts), we create ConstCollectedExpr wrappers.
+//! These are resolved by Phase 53 (collectConstExpressions) which runs AFTER Phase 52
+//! (collectI18nConsts), ensuring correct const array ordering.
 
-use oxc_allocator::Vec as OxcVec;
+use oxc_allocator::{Box, Vec as OxcVec};
 
+use crate::ast::expression::{AbsoluteSourceSpan, LiteralPrimitive, LiteralValue, ParseSpan};
 use crate::ir::enums::DeferOpModifierKind;
+use crate::ir::expression::{ConstCollectedExpr, IrExpression, IrLiteralArrayExpr};
 use crate::ir::ops::{CreateOp, UpdateOp, XrefId};
-use crate::pipeline::compilation::{ComponentCompilationJob, ConstValue};
+use crate::pipeline::compilation::ComponentCompilationJob;
 
 /// Collected timing config for a defer block.
 #[derive(Clone)]
@@ -28,7 +35,7 @@ struct DeferTimingConfig {
 /// Configures defer instructions with trigger and dependency information.
 ///
 /// This phase:
-/// 1. Collects timing configs and adds them to the constant pool
+/// 1. Collects timing configs and wraps them in ConstCollectedExpr for later resolution
 /// 2. Collects all DeferOp blocks and their associated DeferOnOp triggers
 /// 3. Links triggers to their target defer blocks
 /// 4. Validates timing parameters
@@ -54,51 +61,123 @@ pub fn configure_defer_instructions(job: &mut ComponentCompilationJob<'_>) {
         })
         .collect();
 
-    // Create const pool entries for timing configs
-    // Map: xref -> (placeholder_config_index, loading_config_index)
-    let mut config_indices: std::vec::Vec<(XrefId, Option<u32>, Option<u32>)> = Vec::new();
+    // Build ConstCollectedExpr wrappers for each defer block's timing configs.
+    // These wrap a LiteralArray (e.g. [100, null]) in ConstCollectedExpr so that
+    // Phase 53 (collectConstExpressions) will lift them to consts AFTER i18n consts.
+    //
+    // This matches Angular TS's defer_configs.ts which uses:
+    //   op.loadingConfig = new ir.ConstCollectedExpr(literalOrArrayLiteral([...]))
+    let mut config_exprs: std::vec::Vec<(
+        XrefId,
+        Option<Box<'_, IrExpression<'_>>>,
+        Option<Box<'_, IrExpression<'_>>>,
+    )> = Vec::new();
+
+    let span = ParseSpan { start: 0, end: 0 };
+    let source_span = AbsoluteSourceSpan { start: 0, end: 0 };
 
     for config in &timing_configs {
-        let mut placeholder_config_idx = None;
-        let mut loading_config_idx = None;
+        let mut placeholder_config_expr = None;
+        let mut loading_config_expr = None;
 
         // Create loading config: [minimumTime, afterTime]
-        // Note: Angular processes loadingConfig before placeholderConfig in transformExpressionsInOp
-        // (see ir/src/expression.ts lines 1177-1186), so we must add loading config first
-        // to get the correct const pool index order.
+        // Angular processes loadingConfig before placeholderConfig in transformExpressionsInOp
+        // (see ir/src/expression.ts lines 1241-1251), so we create loading config first.
+        // Angular uses `literalOrArrayLiteral([op.loadingMinimumTime, op.loadingAfterTime])`
+        // which emits `null` for missing values, not `0`.
         if config.loading_minimum_time.is_some() || config.loading_after_time.is_some() {
-            let min_time = config.loading_minimum_time.unwrap_or(0);
-            let after_time = config.loading_after_time.unwrap_or(0);
-            let mut entries = OxcVec::new_in(allocator);
-            entries.push(ConstValue::Number(min_time as f64));
-            entries.push(ConstValue::Number(after_time as f64));
-            let const_value = ConstValue::Array(entries);
-            loading_config_idx = Some(job.add_const(const_value));
+            let mut elements = OxcVec::with_capacity_in(2, allocator);
+
+            // minimumTime: number or null
+            let min_val = match config.loading_minimum_time {
+                Some(t) => LiteralValue::Number(t as f64),
+                None => LiteralValue::Null,
+            };
+            elements.push(IrExpression::Ast(Box::new_in(
+                crate::ast::expression::AngularExpression::LiteralPrimitive(Box::new_in(
+                    LiteralPrimitive { span, source_span, value: min_val },
+                    allocator,
+                )),
+                allocator,
+            )));
+
+            // afterTime: number or null
+            let after_val = match config.loading_after_time {
+                Some(t) => LiteralValue::Number(t as f64),
+                None => LiteralValue::Null,
+            };
+            elements.push(IrExpression::Ast(Box::new_in(
+                crate::ast::expression::AngularExpression::LiteralPrimitive(Box::new_in(
+                    LiteralPrimitive { span, source_span, value: after_val },
+                    allocator,
+                )),
+                allocator,
+            )));
+
+            let array_expr = IrExpression::LiteralArray(Box::new_in(
+                IrLiteralArrayExpr { elements, source_span: None },
+                allocator,
+            ));
+
+            loading_config_expr = Some(Box::new_in(
+                IrExpression::ConstCollected(Box::new_in(
+                    ConstCollectedExpr {
+                        expr: Box::new_in(array_expr, allocator),
+                        source_span: None,
+                    },
+                    allocator,
+                )),
+                allocator,
+            ));
         }
 
         // Create placeholder config: [minimumTime]
         if let Some(min_time) = config.placeholder_minimum_time {
-            let mut entries = OxcVec::new_in(allocator);
-            entries.push(ConstValue::Number(min_time as f64));
-            let const_value = ConstValue::Array(entries);
-            placeholder_config_idx = Some(job.add_const(const_value));
+            let mut elements = OxcVec::with_capacity_in(1, allocator);
+            elements.push(IrExpression::Ast(Box::new_in(
+                crate::ast::expression::AngularExpression::LiteralPrimitive(Box::new_in(
+                    LiteralPrimitive {
+                        span,
+                        source_span,
+                        value: LiteralValue::Number(min_time as f64),
+                    },
+                    allocator,
+                )),
+                allocator,
+            )));
+
+            let array_expr = IrExpression::LiteralArray(Box::new_in(
+                IrLiteralArrayExpr { elements, source_span: None },
+                allocator,
+            ));
+
+            placeholder_config_expr = Some(Box::new_in(
+                IrExpression::ConstCollected(Box::new_in(
+                    ConstCollectedExpr {
+                        expr: Box::new_in(array_expr, allocator),
+                        source_span: None,
+                    },
+                    allocator,
+                )),
+                allocator,
+            ));
         }
 
-        config_indices.push((config.xref, placeholder_config_idx, loading_config_idx));
+        config_exprs.push((config.xref, placeholder_config_expr, loading_config_expr));
     }
 
-    // Update DeferOp with config indices
+    // Update DeferOp with config expressions
     let view_xrefs: std::vec::Vec<XrefId> = job.all_views().map(|v| v.xref).collect();
     for view_xref in view_xrefs {
         if let Some(view) = job.view_mut(view_xref) {
             for op in view.create.iter_mut() {
                 if let CreateOp::Defer(defer) = op {
-                    // Find the config indices for this defer block
-                    if let Some((_, placeholder_idx, loading_idx)) =
-                        config_indices.iter().find(|(xref, _, _)| *xref == defer.xref)
+                    // Find the config expressions for this defer block
+                    if let Some((_, placeholder_expr, loading_expr)) =
+                        config_exprs.iter_mut().find(|(xref, _, _)| *xref == defer.xref)
                     {
-                        defer.placeholder_config = *placeholder_idx;
-                        defer.loading_config = *loading_idx;
+                        defer.placeholder_config = placeholder_expr.take();
+                        defer.loading_config = loading_expr.take();
                     }
                 }
             }
diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs b/crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs
@@ -507,8 +507,23 @@ fn reify_create_op<'a>(
         CreateOp::Defer(defer) => {
             // Emit defer instruction for @defer
             let slot = defer.slot.map(|s| s.0).unwrap_or(0);
-            // Take ownership of resolver_fn since OutputExpression doesn't implement Clone
-            // Use config indices (const pool slots) instead of raw timing values
+            // Extract const indices from resolved ConstReference expressions.
+            // By this point, Phase 53 (collectConstExpressions) has replaced
+            // ConstCollectedExpr with ConstReference(index).
+            let loading_config = defer.loading_config.as_ref().and_then(|expr| {
+                if let crate::ir::expression::IrExpression::ConstReference(cr) = expr.as_ref() {
+                    Some(cr.index)
+                } else {
+                    None
+                }
+            });
+            let placeholder_config = defer.placeholder_config.as_ref().and_then(|expr| {
+                if let crate::ir::expression::IrExpression::ConstReference(cr) = expr.as_ref() {
+                    Some(cr.index)
+                } else {
+                    None
+                }
+            });
             Some(create_defer_stmt(
                 allocator,
                 slot,
@@ -517,8 +532,8 @@ fn reify_create_op<'a>(
                 defer.loading_slot.map(|s| s.0),
                 defer.placeholder_slot.map(|s| s.0),
                 defer.error_slot.map(|s| s.0),
-                defer.loading_config,
-                defer.placeholder_config,
+                loading_config,
+                placeholder_config,
                 defer.flags,
             ))
         }
diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs
@@ -4619,3 +4619,49 @@ fn test_i18n_nested_icu_with_interpolations_inside_elements() {
 
     insta::assert_snapshot!("i18n_nested_icu_with_interpolations_inside_elements", js);
 }
+
+/// Tests that @defer loading timer consts are ordered AFTER i18n consts in the consts array.
+///
+/// Angular's TS compiler wraps defer timer configs in ConstCollectedExpr (phase 19), which are
+/// resolved later in collectConstExpressions (phase 53) — AFTER i18n consts are added (phase 52).
+/// This means i18n consts always appear before defer timer consts in the consts array.
+///
+/// Previously, OXC directly called job.add_const() in the defer_configs phase, placing the timer
+/// const [100, null] at the front of the array and shifting all i18n indices by +1.
+///
+/// The template pattern: an i18n message + @defer with @loading(minimum 100ms).
+/// Expected consts ordering: [i18n_0, [100, null], ...]
+/// Bug consts ordering:      [[100, 0], i18n_0, ...]
+#[test]
+fn test_defer_loading_timer_consts_after_i18n_consts() {
+    let js = compile_template_to_js(
+        r#"<span i18n="@@my-label">Hello</span>
+@defer (on viewport; prefetch on idle) {
+  <div>Deferred content</div>
+} @loading (minimum 100ms) {
+  <div>Loading...</div>
+}"#,
+        "TestComponent",
+    );
+
+    // The i18n message should reference const index 0 (i18n_0 is first in consts array)
+    // The @defer instruction should reference the timer config at a later index
+    //
+    // NG expected output has:
+    //   consts: [...] => return [i18n_0, [100, null], ...]
+    //   i18n(N, 0)       — i18n at const index 0
+    //   defer(M, ..., 1, ..., timerScheduling)  — timer config at const index 1
+    //
+    // The bug would produce:
+    //   consts: [...] => return [[100, 0], i18n_0, ...]
+    //   i18n(N, 1)       — i18n at const index 1 (wrong!)
+    //   defer(M, ..., 0, ..., timerScheduling)  — timer config at const index 0 (wrong!)
+
+    // Check that i18n references const index 0 (not 1)
+    assert!(
+        js.contains("i18n(1,0)") || js.contains("i18n(1, 0)"),
+        "i18n should reference const index 0 (before defer timer const). Output:\n{js}"
+    );
+
+    insta::assert_snapshot!("defer_loading_timer_consts_after_i18n_consts", js);
+}
diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__defer_loading_timer_consts_after_i18n_consts.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__defer_loading_timer_consts_after_i18n_consts.snap
@@ -0,0 +1,35 @@
+---
+source: crates/oxc_angular_compiler/tests/integration_test.rs
+expression: js
+---
+function TestComponent_Defer_3_Template(rf,ctx) {
+  if ((rf & 1)) {
+    i0.ɵɵtext(0,"\n  ");
+    i0.ɵɵelementStart(1,"div");
+    i0.ɵɵtext(2,"Deferred content");
+    i0.ɵɵelementEnd();
+    i0.ɵɵtext(3,"\n");
+  }
+}
+function TestComponent_DeferLoading_4_Template(rf,ctx) {
+  if ((rf & 1)) {
+    i0.ɵɵtext(0,"\n  ");
+    i0.ɵɵelementStart(1,"div");
+    i0.ɵɵtext(2,"Loading...");
+    i0.ɵɵelementEnd();
+    i0.ɵɵtext(3,"\n");
+  }
+}
+function TestComponent_Template(rf,ctx) {
+  if ((rf & 1)) {
+    i0.ɵɵelementStart(0,"span");
+    i0.ɵɵi18n(1,0);
+    i0.ɵɵelementEnd();
+    i0.ɵɵtext(2,"\n");
+    i0.ɵɵdomTemplate(3,TestComponent_Defer_3_Template,4,0)(4,TestComponent_DeferLoading_4_Template,
+        4,0);
+    i0.ɵɵdefer(5,3,null,4,null,null,1,null,i0.ɵɵdeferEnableTimerScheduling);
+    i0.ɵɵdeferOnViewport();
+    i0.ɵɵdeferPrefetchOnIdle();
+  }
+}
