diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs b/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs index 59f5345ef..cb9f0f178 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs +++ b/crates/oxc_angular_compiler/src/pipeline/phases/track_fn_optimization.rs @@ -69,7 +69,7 @@ fn optimize_track_expression<'a>( // Check for method call pattern: this.fn($index) or this.fn($index, $item) // These can be passed directly to the repeater runtime. if let Some((method_name, is_root_context)) = - check_track_by_function_call(&rep.track, root_xref, expressions) + check_track_by_function_call(&rep.track, root_xref) { rep.uses_component_instance = true; if is_root_context && view_xref == root_xref { @@ -381,11 +381,8 @@ fn check_ast_for_simple_track_variable( /// Returns the method name (as String) and whether the context is the root view. fn check_track_by_function_call( track: &IrExpression<'_>, - _root_xref: XrefId, - expressions: &crate::pipeline::expression_store::ExpressionStore<'_>, + root_xref: XrefId, ) -> Option<(String, bool)> { - use crate::ast::expression::AngularExpression; - // Handle ResolvedCall expressions (created by resolveNames phase) if let IrExpression::ResolvedCall(rc) = track { // Must have 1 or 2 arguments @@ -411,60 +408,25 @@ fn check_track_by_function_call( return None; } - // Check receiver: must be a ResolvedPropertyRead on Context + // Check receiver: must be a ResolvedPropertyRead on Context whose view is the root view. + // Angular's isTrackByFunctionCall (track_fn_optimization.ts:96-100) requires + // receiver.receiver.view === rootView, rejecting non-root-view contexts. if let IrExpression::ResolvedPropertyRead(rp) = rc.receiver.as_ref() { - if matches!(rp.receiver.as_ref(), IrExpression::Context(_)) { - return Some((rp.name.to_string(), true)); + if let IrExpression::Context(ctx) = rp.receiver.as_ref() { + if ctx.view == root_xref { + return Some((rp.name.to_string(), true)); + } } } return None; } - // Get the AST expression, handling both direct Ast and ExpressionRef - let ast = match track { - IrExpression::Ast(ast) => ast.as_ref(), - IrExpression::ExpressionRef(id) => expressions.get(*id), - _ => return None, - }; - - // Check for Call expression - if let AngularExpression::Call(call) = ast { - // Must have 1 or 2 arguments - if call.args.is_empty() || call.args.len() > 2 { - return None; - } - - // Check arguments: first must be $index, second (if present) must be $item - let first_is_index = matches!(&call.args[0], AngularExpression::PropertyRead(pr) - if matches!(&pr.receiver, AngularExpression::ImplicitReceiver(_)) - && pr.name.as_str() == "$index"); - - if !first_is_index { - return None; - } - - if call.args.len() == 2 { - let second_is_item = matches!(&call.args[1], AngularExpression::PropertyRead(pr) - if matches!(&pr.receiver, AngularExpression::ImplicitReceiver(_)) - && pr.name.as_str() == "$item"); - if !second_is_item { - return None; - } - } - - // Check receiver: must be a property read on the component context - // Pattern: receiver.methodName where receiver is context (this) - if let AngularExpression::PropertyRead(method_read) = &call.receiver { - // The receiver of the property read should be the implicit receiver (this/context) - if matches!(&method_read.receiver, AngularExpression::ImplicitReceiver(_)) { - // This is a method call on the implicit receiver: this.methodName($index, ...) - // In Angular, this is a call on the component context - return Some((method_read.name.to_string(), true)); - } - } - } - + // AST fallback path: After resolve_names (phase 31), track expressions should already + // be ResolvedCall/ResolvedPropertyRead with Context receivers. If we reach here with + // raw AST expressions, ImplicitReceiver lacks a view field so we cannot verify the + // root-view guard that Angular's isTrackByFunctionCall requires. Reject optimization + // to avoid mis-optimizing nested-view track calls. None } @@ -497,3 +459,73 @@ fn is_item_variable(expr: &IrExpression<'_>) -> bool { _ => false, } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ir::expression::{ContextExpr, ResolvedCallExpr, ResolvedPropertyReadExpr}; + use crate::ir::ops::XrefId; + use crate::output::ast::ReadVarExpr; + use oxc_allocator::Allocator; + + /// Build a ResolvedCall IR expression representing `ctx.methodName($index)`, + /// where the Context has the given `context_view`. + fn make_track_method_call<'a>( + alloc: &'a Allocator, + method_name: &'a str, + context_view: XrefId, + ) -> IrExpression<'a> { + let ctx = IrExpression::Context(oxc_allocator::Box::new_in( + ContextExpr { view: context_view, source_span: None }, + alloc, + )); + let prop_read = IrExpression::ResolvedPropertyRead(oxc_allocator::Box::new_in( + ResolvedPropertyReadExpr { + receiver: oxc_allocator::Box::new_in(ctx, alloc), + name: Atom::from(method_name), + source_span: None, + }, + alloc, + )); + let index_arg = IrExpression::OutputExpr(oxc_allocator::Box::new_in( + OutputExpression::ReadVar(oxc_allocator::Box::new_in( + ReadVarExpr { name: Atom::from("$index"), source_span: None }, + alloc, + )), + alloc, + )); + let mut args = oxc_allocator::Vec::new_in(alloc); + args.push(index_arg); + IrExpression::ResolvedCall(oxc_allocator::Box::new_in( + ResolvedCallExpr { + receiver: oxc_allocator::Box::new_in(prop_read, alloc), + args, + source_span: None, + }, + alloc, + )) + } + + #[test] + fn test_root_view_context_is_accepted() { + let alloc = Allocator::default(); + let root_xref = XrefId(0); + let track = make_track_method_call(&alloc, "trackByFn", root_xref); + + let result = check_track_by_function_call(&track, root_xref); + assert_eq!(result, Some(("trackByFn".to_string(), true))); + } + + #[test] + fn test_non_root_view_context_is_rejected() { + let alloc = Allocator::default(); + let root_xref = XrefId(0); + let non_root_xref = XrefId(5); + let track = make_track_method_call(&alloc, "trackByFn", non_root_xref); + + // Before the fix, this would return Some — incorrectly accepting a + // non-root context. After the fix, it returns None. + let result = check_track_by_function_call(&track, root_xref); + assert_eq!(result, None, "non-root context should NOT be optimized"); + } +} diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 4cbbfc904..1131aec37 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -1088,6 +1088,63 @@ fn test_for_track_bare_method_reference() { insta::assert_snapshot!("for_track_bare_method_reference", js); } +/// Tests that `track trackByFn($index)` inside a nested view (e.g. @if) uses +/// `componentInstance().trackByFn` instead of `ctx.trackByFn`. +/// +/// Angular's `isTrackByFunctionCall` (track_fn_optimization.ts:84-115) only optimizes +/// when the context receiver's view is the root view. The main optimization then checks +/// `receiver.receiver.view === unit.xref` to decide between `ctx.method` (root view) +/// and `componentInstance().method` (non-root view). +/// +/// When the @for is inside @if, the repeater is in a non-root view, so Angular uses +/// the `componentInstance().method` path. +#[test] +fn test_for_track_method_call_in_nested_view() { + // @for inside @if: repeater is in a non-root view + let js = compile_template_to_js( + r"@if (showItems) { @for (item of items; track trackByFn($index)) {
{{item.name}}
} }", + "TestComponent", + ); + // In a nested view, Angular uses componentInstance().trackByFn + assert!( + js.contains("componentInstance"), + "Track method call in nested view should use componentInstance().trackByFn. Output:\n{js}" + ); + insta::assert_snapshot!("for_track_method_call_in_nested_view", js); +} + +/// Tests that `track trackByFn($index)` in the root view uses `ctx.trackByFn`. +#[test] +fn test_for_track_method_call_in_root_view() { + // @for directly in root view + let js = compile_template_to_js( + r"@for (item of items; track trackByFn($index)) {
{{item.name}}
}", + "TestComponent", + ); + // In the root view, Angular uses ctx.trackByFn directly + assert!( + js.contains("ctx.trackByFn"), + "Track method call in root view should use ctx.trackByFn. Output:\n{js}" + ); + insta::assert_snapshot!("for_track_method_call_in_root_view", js); +} + +/// Tests that `track trackByFn($index, item)` in the root view uses `ctx.trackByFn`. +/// Note: The loop variable name (e.g. `item`) is used, not the literal `$item`. +/// `generateTrackVariables` converts `item` → `$item` ReadVarExpr, which is then optimizable. +#[test] +fn test_for_track_method_call_with_both_args() { + let js = compile_template_to_js( + r"@for (item of items; track trackByFn($index, item)) {
{{item.name}}
}", + "TestComponent", + ); + assert!( + js.contains("ctx.trackByFn"), + "Track method call with ($index, item) in root view should use ctx.trackByFn. Output:\n{js}" + ); + insta::assert_snapshot!("for_track_method_call_with_both_args", js); +} + #[test] fn test_nested_for_loops() { let js = compile_template_to_js( diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_in_nested_view.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_in_nested_view.snap new file mode 100644 index 000000000..46b71eeb7 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_in_nested_view.snap @@ -0,0 +1,34 @@ +--- +source: crates/oxc_angular_compiler/tests/integration_test.rs +expression: js +--- +function TestComponent_Conditional_0_For_2_Template(rf,ctx) { + if ((rf & 1)) { + i0.ɵɵtext(0," "); + i0.ɵɵelementStart(1,"div"); + i0.ɵɵtext(2); + i0.ɵɵelementEnd(); + i0.ɵɵtext(3," "); + } + if ((rf & 2)) { + const item_r1 = ctx.$implicit; + i0.ɵɵadvance(2); + i0.ɵɵtextInterpolate(item_r1.name); + } +} +function TestComponent_Conditional_0_Template(rf,ctx) { + if ((rf & 1)) { + i0.ɵɵtext(0," "); + i0.ɵɵrepeaterCreate(1,TestComponent_Conditional_0_For_2_Template,4,1,null,null, + i0.ɵɵcomponentInstance().trackByFn,true); + } + if ((rf & 2)) { + const ctx_r1 = i0.ɵɵnextContext(); + i0.ɵɵadvance(); + i0.ɵɵrepeater(ctx_r1.items); + } +} +function TestComponent_Template(rf,ctx) { + if ((rf & 1)) { i0.ɵɵconditionalCreate(0,TestComponent_Conditional_0_Template,3,0); } + if ((rf & 2)) { i0.ɵɵconditional((ctx.showItems? 0: -1)); } +} diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_in_root_view.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_in_root_view.snap new file mode 100644 index 000000000..5238bf645 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_in_root_view.snap @@ -0,0 +1,23 @@ +--- +source: crates/oxc_angular_compiler/tests/integration_test.rs +expression: js +--- +function TestComponent_For_1_Template(rf,ctx) { + if ((rf & 1)) { + i0.ɵɵtext(0," "); + i0.ɵɵelementStart(1,"div"); + i0.ɵɵtext(2); + i0.ɵɵelementEnd(); + i0.ɵɵtext(3," "); + } + if ((rf & 2)) { + const item_r1 = ctx.$implicit; + i0.ɵɵadvance(2); + i0.ɵɵtextInterpolate(item_r1.name); + } +} +function TestComponent_Template(rf,ctx) { + if ((rf & 1)) { i0.ɵɵrepeaterCreate(0,TestComponent_For_1_Template,4,1,null,null,ctx.trackByFn, + true); } + if ((rf & 2)) { i0.ɵɵrepeater(ctx.items); } +} diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_with_both_args.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_with_both_args.snap new file mode 100644 index 000000000..5238bf645 --- /dev/null +++ b/crates/oxc_angular_compiler/tests/snapshots/integration_test__for_track_method_call_with_both_args.snap @@ -0,0 +1,23 @@ +--- +source: crates/oxc_angular_compiler/tests/integration_test.rs +expression: js +--- +function TestComponent_For_1_Template(rf,ctx) { + if ((rf & 1)) { + i0.ɵɵtext(0," "); + i0.ɵɵelementStart(1,"div"); + i0.ɵɵtext(2); + i0.ɵɵelementEnd(); + i0.ɵɵtext(3," "); + } + if ((rf & 2)) { + const item_r1 = ctx.$implicit; + i0.ɵɵadvance(2); + i0.ɵɵtextInterpolate(item_r1.name); + } +} +function TestComponent_Template(rf,ctx) { + if ((rf & 1)) { i0.ɵɵrepeaterCreate(0,TestComponent_For_1_Template,4,1,null,null,ctx.trackByFn, + true); } + if ((rf & 2)) { i0.ɵɵrepeater(ctx.items); } +}