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
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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");
}
}
57 changes: 57 additions & 0 deletions crates/oxc_angular_compiler/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) { <div>{{item.name}}</div> } }",
"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)) { <div>{{item.name}}</div> }",
"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)) { <div>{{item.name}}</div> }",
"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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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)); }
}
Original file line number Diff line number Diff line change
@@ -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); }
}
Original file line number Diff line number Diff line change
@@ -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); }
}
Loading