diff --git a/crates/oxc_angular_compiler/src/ir/expression.rs b/crates/oxc_angular_compiler/src/ir/expression.rs index a58538add..b7a6bc106 100644 --- 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 index 4049c70f7..b1cae1b05 100644 --- 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, /// Loading after time. pub loading_after_time: Option, - /// Placeholder config const index (points to [minimumTime] in consts array). - /// Set by defer_configs phase after adding to constant pool. - pub placeholder_config: Option, - /// 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, + /// Placeholder config expression (wraps [minimumTime] as ConstCollectedExpr). + /// Set by defer_configs phase; resolved to ConstReference by const collection phase. + pub placeholder_config: Option>>, + /// Loading config expression (wraps [minimumTime, afterTime] as ConstCollectedExpr). + /// Set by defer_configs phase; resolved to ConstReference by const collection phase. + pub loading_config: Option>>, /// 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 index c250de75e..9d5027f69 100644 --- 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, Option)> = 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>>, + Option>>, + )> = 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 = 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 index ff3eb7c70..8bbb4d9fe 100644 --- 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 index 5f471321b..880a42e57 100644 --- 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#"Hello +@defer (on viewport; prefetch on idle) { +
Deferred content
+} @loading (minimum 100ms) { +
Loading...
+}"#, + "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 new file mode 100644 index 000000000..0491aebeb --- /dev/null +++ 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(); + } +}