fix(angular): fix nested @for track expression with outer-scope variable

crates/oxc_angular_compiler/src/pipeline/phases/reify/mod.rs

@@ -480,6 +480,8 @@ fn reify_create_op<'a>(
                 &repeater.track,
                 repeater.track_fn_name.as_ref(),
                 repeater.uses_component_instance,
+                &mut repeater.track_by_ops,
+                diagnostics,
             );
 
             Some(create_repeater_create_stmt_with_track_expr(
@@ -1171,7 +1173,12 @@ fn reify_host_create_op<'a>(
 /// the track expression as the body, registers it with the constant pool,
 /// and returns the function reference.
 ///
+/// When `track_by_ops` is `Some`, the ops are reified into statements and assembled
+/// into a function body. This handles cases where the track expression needs additional
+/// context variable declarations (e.g., `nextContext()` calls for outer-scope access).
+///
 /// Ported from Angular's `reifyTrackBy()` in `reify.ts`.
+#[allow(clippy::too_many_arguments)]
 fn reify_track_by<'a>(
     allocator: &'a oxc_allocator::Allocator,
     pool: &mut ConstantPool<'a>,
@@ -1180,6 +1187,8 @@ fn reify_track_by<'a>(
     track: &IrExpression<'a>,
     track_fn_name: Option<&Atom<'a>>,
     uses_component_instance: bool,
+    track_by_ops: &mut Option<oxc_allocator::Vec<'a, UpdateOp<'a>>>,
+    diagnostics: &mut Vec<OxcDiagnostic>,
 ) -> OutputExpression<'a> {
     // If the tracking function was already set by optimization phase, return a reference to it
     if let Some(fn_name) = track_fn_name {
@@ -1279,42 +1288,99 @@ fn reify_track_by<'a>(
         ));
     }
 
-    // Convert the track expression to output expression
-    let track_body = convert_ir_expression(allocator, track, expressions, root_xref);
-
     // Create the track function with params ($index, $item)
-    // If uses_component_instance is true, use a regular function expression so `this` is bound correctly.
-    // Otherwise, use an arrow function.
-    // Both are registered with the constant pool via getSharedFunctionReference,
-    // which maintains insertion order in pool.statements.
     let mut params = OxcVec::with_capacity_in(2, allocator);
     params.push(FnParam { name: Atom::from("$index") });
     params.push(FnParam { name: Atom::from("$item") });
 
-    let fn_expr = if uses_component_instance {
-        // Regular function expression: function($index, $item) { return track_body; }
-        // Angular uses o.fn(params, [new o.ReturnStatement(op.track)])
-        let mut stmts = OxcVec::with_capacity_in(1, allocator);
-        stmts.push(OutputStatement::Return(Box::new_in(
-            ReturnStatement { value: track_body, source_span: None },
-            allocator,
-        )));
+    let fn_expr = if let Some(track_ops) = track_by_ops {
+        // Complex case: track_by_ops is present (set by track_fn_optimization phase).
+        // This happens when the track expression needs additional ops like context
+        // variable declarations (e.g., `const group_r2 = nextContext().$implicit`).
+        //
+        // Ported from Angular's reify.ts lines 884-904:
+        //   reifyUpdateOperations(unit, op.trackByOps);
+        //   const statements = [...]; // from trackByOps
+        //   fn = op.usesComponentInstance || statements.length !== 1 || !(statements[0] instanceof ReturnStatement)
+        //     ? o.fn(params, statements)
+        //     : o.arrowFn(params, statements[0].value);
 
-        OutputExpression::Function(Box::new_in(
-            FunctionExpr { name: None, params, statements: stmts, source_span: None },
-            allocator,
-        ))
+        // Reify each op in track_by_ops into output statements
+        let mut statements = OxcVec::new_in(allocator);
+        for track_op in track_ops.iter() {
+            if let Some(stmt) = reify_update_op(
+                allocator,
+                track_op,
+                expressions,
+                root_xref,
+                TemplateCompilationMode::Full,
+                diagnostics,
+            ) {
+                statements.push(stmt);
+            }
+        }
+
+        // Determine whether to use function or arrow:
+        // Angular uses function when:
+        //   - usesComponentInstance is true, OR
+        //   - there are multiple statements, OR
+        //   - the single statement is not a ReturnStatement
+        let use_function = uses_component_instance
+            || statements.len() != 1
+            || !matches!(statements.first(), Some(OutputStatement::Return(_)));
+
+        if use_function {
+            OutputExpression::Function(Box::new_in(
+                FunctionExpr { name: None, params, statements, source_span: None },
+                allocator,
+            ))
+        } else {
+            // Single return statement → extract value for arrow function body
+            // Clone the return value since we can't move out of the Box
+            let return_value = if let Some(OutputStatement::Return(ret)) = statements.first() {
+                ret.value.clone_in(allocator)
+            } else {
+                unreachable!("checked above that there's exactly one Return statement");
+            };
+
+            OutputExpression::ArrowFunction(Box::new_in(
+                ArrowFunctionExpr {
+                    params,
+                    body: ArrowFunctionBody::Expression(Box::new_in(return_value, allocator)),
+                    source_span: None,
+                },
+                allocator,
+            ))
+        }
     } else {
-        // Arrow function: ($index, $item) => track_body
-        // Angular uses o.arrowFn(params, op.track)
-        OutputExpression::ArrowFunction(Box::new_in(
-            ArrowFunctionExpr {
-                params,
-                body: ArrowFunctionBody::Expression(Box::new_in(track_body, allocator)),
-                source_span: None,
-            },
-            allocator,
-        ))
+        // Simple case: no track_by_ops. Wrap the raw track expression.
+        // Ported from Angular's reify.ts lines 878-883:
+        //   fn = op.usesComponentInstance
+        //     ? o.fn(params, [new o.ReturnStatement(op.track)])
+        //     : o.arrowFn(params, op.track);
+        let track_body = convert_ir_expression(allocator, track, expressions, root_xref);
+
+        if uses_component_instance {
+            let mut stmts = OxcVec::with_capacity_in(1, allocator);
+            stmts.push(OutputStatement::Return(Box::new_in(
+                ReturnStatement { value: track_body, source_span: None },
+                allocator,
+            )));
+
+            OutputExpression::Function(Box::new_in(
+                FunctionExpr { name: None, params, statements: stmts, source_span: None },
+                allocator,
+            ))
+        } else {
+            OutputExpression::ArrowFunction(Box::new_in(
+                ArrowFunctionExpr {
+                    params,
+                    body: ArrowFunctionBody::Expression(Box::new_in(track_body, allocator)),
+                    source_span: None,
+                },
+                allocator,
+            ))
+        }
     };
 
     // Register with constant pool as a shared function

crates/oxc_angular_compiler/src/pipeline/phases/resolve_contexts.rs

@@ -42,6 +42,15 @@ pub fn resolve_contexts(job: &mut ComponentCompilationJob<'_>) {
     for view_xref in view_xrefs {
         let is_root = view_xref == root_xref;
         if let Some(view) = job.view_mut(view_xref) {
+            // Process arrow functions' ops first, matching Angular's resolve_contexts.ts:
+            //   for (const expr of unit.functions) { processLexicalScope(unit, expr.ops); }
+            for fn_ptr in view.functions.iter() {
+                // SAFETY: The pointer is valid because it was populated by generate_arrow_functions
+                // and the allocator keeps the data alive.
+                let arrow_fn = unsafe { &mut **fn_ptr };
+                process_lexical_scope_update_vec(allocator, view_xref, is_root, &mut arrow_fn.ops);
+            }
+
             process_lexical_scope_create(allocator, view_xref, is_root, &mut view.create);
             process_lexical_scope_update(allocator, view_xref, is_root, &mut view.update);
         }
@@ -79,7 +88,8 @@ fn process_lexical_scope_create<'a>(
     }
 
     // Second pass: Transform ContextExpr to the appropriate expression
-    // Also recursively process listener handler_ops (per resolve_contexts.ts lines 45-49)
+    // Also recursively process listener handler_ops and RepeaterCreate track_by_ops
+    // (per resolve_contexts.ts lines 40-61)
     for op in ops.iter_mut() {
         match op {
             CreateOp::Listener(listener) => {
@@ -122,6 +132,26 @@ fn process_lexical_scope_create<'a>(
                     &mut None,
                 );
             }
+            CreateOp::RepeaterCreate(repeater) => {
+                // Process track_by_ops with their own scope first, matching Angular's
+                // resolve_contexts.ts lines 55-58:
+                //   case ir.OpKind.RepeaterCreate:
+                //     if (op.trackByOps !== null) { processLexicalScope(view, op.trackByOps); }
+                if let Some(ref mut track_by_ops) = repeater.track_by_ops {
+                    process_lexical_scope_update_vec(allocator, view_xref, is_root, track_by_ops);
+                }
+                // Also transform expressions in the RepeaterCreate op itself (e.g., rep.track)
+                // using the parent scope. Note: track_by_ops have already been processed above
+                // so re-visiting them via transform_expressions_in_create_op is a no-op
+                // (ContextExpr nodes in track_by_ops are already resolved).
+                transform_expressions_in_create_op(
+                    op,
+                    &|expr, _flags| {
+                        transform_context_expr(allocator, expr, &scope);
+                    },
+                    VisitorContextFlag::NONE,
+                );
+            }
             _ => {
                 transform_expressions_in_create_op(
                     op,
@@ -251,6 +281,50 @@ fn process_lexical_scope_update<'a>(
     }
 }
 
+/// Process update operations in a Vec (used for arrow function ops and track_by_ops).
+///
+/// This is the same logic as `process_lexical_scope_update` but works with `Vec<UpdateOp>`
+/// instead of `UpdateOpList`. Needed for `ArrowFunctionExpr.ops` and `RepeaterCreate.track_by_ops`.
+fn process_lexical_scope_update_vec<'a>(
+    allocator: &'a oxc_allocator::Allocator,
+    view_xref: XrefId,
+    is_root: bool,
+    ops: &mut oxc_allocator::Vec<'a, UpdateOp<'a>>,
+) {
+    // Track how to access each view's context by its XrefId.
+    let mut scope: FxHashMap<XrefId, ContextAccess> = FxHashMap::default();
+
+    // The current view's context is accessible via the `ctx` parameter.
+    scope.insert(view_xref, ContextAccess::CtxParameter);
+
+    // First pass: Build scope from Variable operations
+    for op in ops.iter() {
+        if let UpdateOp::Variable(var_op) = op {
+            if var_op.kind == SemanticVariableKind::Context {
+                if let Some(target_view) = var_op.view {
+                    scope.insert(target_view, ContextAccess::ReadVariable(var_op.xref));
+                }
+            }
+        }
+    }
+
+    // If this is the root view, prefer `ctx` over any variables
+    if is_root {
+        scope.insert(view_xref, ContextAccess::CtxParameter);
+    }
+
+    // Second pass: Transform ContextExpr to the appropriate expression
+    for op in ops.iter_mut() {
+        transform_expressions_in_update_op(
+            op,
+            &|expr, _flags| {
+                transform_context_expr(allocator, expr, &scope);
+            },
+            VisitorContextFlag::NONE,
+        );
+    }
+}
+
 /// Transforms a ContextExpr based on the scope.
 ///
 /// - For current view context: Keep as ContextExpr (reify handles ctx)

crates/oxc_angular_compiler/src/pipeline/phases/resolve_names.rs

@@ -50,25 +50,19 @@ pub fn resolve_names(job: &mut ComponentCompilationJob<'_>) {
     // Collect expression store data we need first
     let expression_store_ptr = &job.expressions as *const ExpressionStore<'_>;
 
-    // Process each view's create and update operation lists
-    // IMPORTANT: We need to build scope from BOTH create and update ops before processing listeners,
-    // because listener handler expressions need access to variables from both phases.
+    // Process each view's create and update operation lists.
+    // Per Angular's TypeScript (resolve_names.ts lines 25-26), create and update ops are
+    // processed as SEPARATE lexical scopes — each builds its own scope/localDefinitions
+    // from its own Variable ops. This is important because variables from update ops
+    // (like @for loop items: $implicit, $index) should NOT be visible when resolving
+    // expressions in create ops (like RepeaterCreate.track). The generateVariables phase
+    // already prepends all necessary variables to each op list.
     for view in job.all_views_mut() {
         // SAFETY: We're only reading from expression_store, not modifying it
         let expressions = unsafe { &*expression_store_ptr };
 
-        // Build scope from update ops first (contains context variables like $implicit, $index, etc.)
-        let update_scope = build_scope_from_update_ops(&view.update);
-
-        // Process create ops with the update scope available for listener handler expressions
-        process_lexical_scope_create(
-            root_xref,
-            &mut view.create,
-            None,
-            &update_scope,
-            allocator,
-            expressions,
-        );
+        // Process create ops with their own scope (no update scope merged in)
+        process_lexical_scope_create(root_xref, &mut view.create, None, allocator, expressions);
         process_lexical_scope_update(root_xref, &mut view.update, None, allocator, expressions);
     }
 
@@ -139,50 +133,17 @@ where
     maps
 }
 
-/// Build scope maps from update operations (for variables like context reads).
-fn build_scope_from_update_ops<'a>(ops: &crate::ir::list::UpdateOpList<'a>) -> ScopeMaps<'a> {
-    let mut maps = ScopeMaps::default();
-    for op in ops.iter() {
-        if let UpdateOp::Variable(var_op) = op {
-            match var_op.kind {
-                SemanticVariableKind::Identifier => {
-                    // Check if this is a local variable (@let declaration)
-                    if var_op.local {
-                        if !maps.local_definitions.contains_key(&var_op.name) {
-                            maps.local_definitions.insert(var_op.name.clone(), var_op.xref);
-                        }
-                    } else if !maps.scope.contains_key(&var_op.name) {
-                        maps.scope.insert(var_op.name.clone(), var_op.xref);
-                    }
-                    // Also add to scope for non-local (always)
-                    if !maps.scope.contains_key(&var_op.name) {
-                        maps.scope.insert(var_op.name.clone(), var_op.xref);
-                    }
-                }
-                SemanticVariableKind::Alias => {
-                    if !maps.scope.contains_key(&var_op.name) {
-                        maps.scope.insert(var_op.name.clone(), var_op.xref);
-                    }
-                }
-                _ => {}
-            }
-        }
-    }
-    maps
-}
-
 /// Process create operations to build scope and resolve names.
 fn process_lexical_scope_create<'a>(
     root_xref: XrefId,
     ops: &mut crate::ir::list::CreateOpList<'a>,
     saved_view: Option<SavedView>,
-    update_scope: &ScopeMaps<'a>,
     allocator: &'a oxc_allocator::Allocator,
     expressions: &ExpressionStore<'a>,
 ) {
     // Maps variable names to their XrefIds
-    // Start with update_scope to include context variables (like @for loop items)
-    let mut scope: ScopeMaps<'a> = update_scope.clone();
+    // Per Angular's TypeScript, each processLexicalScope call starts with a fresh scope.
+    let mut scope: ScopeMaps<'a> = ScopeMaps::default();
 
     // Track saved view for RestoreView expressions
     let mut current_saved_view = saved_view;
@@ -335,6 +296,12 @@ fn process_lexical_scope_create<'a>(
                 }
             }
             _ => {
+                // Note: RepeaterCreate falls through here. Angular's resolve_names.ts first
+                // pass (lines 88-92) processes trackByOps with its own scope, but trackByOps
+                // is always None at this phase (phase 31) — it's created later by
+                // track_fn_optimization (phase 34). The second pass processes all ops
+                // (including RepeaterCreate) via transformExpressionsInOp with the parent
+                // scope, which is what transform_expressions_in_create_op does here.
                 transform_expressions_in_create_op(
                     op,
                     &|expr, _flags| {
@@ -1521,18 +1488,8 @@ pub fn resolve_names_for_host(job: &mut HostBindingCompilationJob<'_>) {
     // SAFETY: We're only reading from expression_store, not modifying it
     let expressions = unsafe { &*expression_store_ptr };
 
-    // Build scope from update ops
-    let update_scope = build_scope_from_update_ops(&job.root.update);
-
-    // Process create ops with the update scope available
-    process_lexical_scope_create(
-        root_xref,
-        &mut job.root.create,
-        None,
-        &update_scope,
-        allocator,
-        expressions,
-    );
+    // Process create ops with their own scope (no update scope merged in)
+    process_lexical_scope_create(root_xref, &mut job.root.create, None, allocator, expressions);
     process_lexical_scope_update(root_xref, &mut job.root.update, None, allocator, expressions);
 
     // Verify no LexicalRead expressions remain after resolution.