fix(angular): enforce root-view guard in @for track method-call optimization (#38)

Angular's `isTrackByFunctionCall` (track_fn_optimization.ts:96-100) requires
`receiver.receiver.view === rootView`, rejecting non-root-view contexts from
the optimized path. The Rust port ignored `root_xref` and accepted any Context
receiver, which could mis-optimize nested-view track calls.

- Check `ctx.view == root_xref` in the ResolvedCall path
- Remove the AST fallback path entirely (dead code after resolve_names phase,
  and ImplicitReceiver lacks a view field to verify the guard)
- Add unit tests that directly verify the root-view guard
- Add integration tests for root-view vs nested-view track method calls

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

Author: Brooooooklyn

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");
+    }
+}

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)) { <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(

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)); }
+}

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); }
+}

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); }
+}