fix: listener handler_expression should generate tmp variable declarations in handler scope (#8)

* fix: listener handler_expression should generate tmp variable declarations in handler scope

handler_expression was being processed without the IN_CHILD_OPERATION flag,
causing temporary variables to be scoped to the parent create block instead
of the listener function. This meant `let tmp_N_0;` declarations were missing
inside listener functions that use safe navigation on function call receivers
(e.g., `getPopover()?.close()`).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: process handler_expression after handler_ops to avoid duplicate tmp declarations

handler_expression was being processed inside the per-op loop, causing it
to be visited/transformed once per handler_op. When handler_ops had multiple
entries (e.g., restoreView + nextContext), this produced spurious declarations
(let tmp_0_0; let tmp_1_0;) and wrong variable names.

Now handler_expression is processed once after the loop with op_count set to
handler_ops.len(), matching Angular TS where the return expression is the last
entry in handlerOps. For a listener with 2 handler_ops, this correctly
generates tmp_2_0 instead of tmp_1_0.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

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

Author: Brooooooklyn

crates/oxc_angular_compiler/src/ir/expression.rs

@@ -2092,12 +2092,13 @@ pub fn transform_expressions_in_create_op<'a, F>(
             transform_expressions_in_expression(&mut op.initializer, transform, flags);
         }
         CreateOp::Listener(op) => {
-            // Process handler expression if present
+            // Process handler ops and handler_expression as child operations.
+            // handler_expression is the return expression of the listener function and
+            // must be treated as part of the handler scope (not the parent scope).
+            let child_flags = flags.union(VisitorContextFlag::IN_CHILD_OPERATION);
             if let Some(handler_expr) = &mut op.handler_expression {
-                transform_expressions_in_expression(handler_expr, transform, flags);
+                transform_expressions_in_expression(handler_expr, transform, child_flags);
             }
-            // Process handler ops in the listener
-            let child_flags = flags.union(VisitorContextFlag::IN_CHILD_OPERATION);
             for handler_op in op.handler_ops.iter_mut() {
                 transform_expressions_in_update_op(handler_op, transform, child_flags);
             }
@@ -2279,10 +2280,10 @@ pub fn visit_expressions_in_create_op<'a, F>(
             visit_expressions_in_expression(&op.initializer, visitor, flags);
         }
         CreateOp::Listener(op) => {
+            let child_flags = flags.union(VisitorContextFlag::IN_CHILD_OPERATION);
             if let Some(handler_expr) = &op.handler_expression {
-                visit_expressions_in_expression(handler_expr, visitor, flags);
+                visit_expressions_in_expression(handler_expr, visitor, child_flags);
             }
-            let child_flags = flags.union(VisitorContextFlag::IN_CHILD_OPERATION);
             for handler_op in op.handler_ops.iter() {
                 visit_expressions_in_update_op(handler_op, visitor, child_flags);
             }

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

@@ -39,7 +39,8 @@ use rustc_hash::{FxHashMap, FxHashSet};
 
 use crate::ir::expression::{
     IrExpression, VisitorContextFlag, transform_expressions_in_create_op,
-    transform_expressions_in_update_op, visit_expressions_in_create_op,
+    transform_expressions_in_expression, transform_expressions_in_update_op,
+    visit_expressions_in_create_op, visit_expressions_in_expression,
     visit_expressions_in_update_op,
 };
 use crate::ir::list::{CreateOpList, UpdateOpList};
@@ -132,8 +133,16 @@ fn generate_temporaries_for_create<'a>(
         // for nested ops lists and the results are prepended to those lists.
         match op {
             CreateOp::Listener(listener) => {
-                let mut handler_stmts =
-                    generate_temporaries_for_handler_ops(&mut listener.handler_ops, allocator);
+                // Process handler_ops and handler_expression together.
+                // In Angular TS, the handler_expression is part of handlerOps.
+                // In our IR, it's a separate field, so we must include it in
+                // temporary variable processing to generate the correct `let tmp_N_0;`
+                // declarations inside the listener function.
+                let mut handler_stmts = generate_temporaries_for_handler_ops_with_expression(
+                    &mut listener.handler_ops,
+                    &mut listener.handler_expression,
+                    allocator,
+                );
                 prepend_update_ops(&mut listener.handler_ops, &mut handler_stmts, allocator);
             }
             CreateOp::AnimationListener(listener) => {
@@ -219,6 +228,74 @@ fn generate_temporaries_for_handler_ops<'a>(
     generated_statements
 }
 
+/// Generates temporary variable declarations for handler_ops AND handler_expression together.
+///
+/// In Angular TS, the handler_expression is part of handlerOps (there is no separate field).
+/// In our IR, handler_expression is a separate field on ListenerOp. We must include it in
+/// temporary variable processing so that `let tmp_N_0;` declarations are generated inside
+/// the listener function scope rather than the parent create scope.
+///
+/// The handler_ops are processed first (same as `generate_temporaries_for_handler_ops`),
+/// then handler_expression is processed once AFTER the loop with op_count equal to
+/// handler_ops.len(). This matches Angular TS where the return expression is the last
+/// entry in handlerOps at index N.
+fn generate_temporaries_for_handler_ops_with_expression<'a>(
+    ops: &mut [UpdateOp<'a>],
+    handler_expression: &mut Option<oxc_allocator::Box<'a, IrExpression<'a>>>,
+    allocator: &'a Allocator,
+) -> Vec<UpdateOp<'a>> {
+    // First, process handler_ops exactly like generate_temporaries_for_handler_ops
+    let mut generated_statements = generate_temporaries_for_handler_ops(ops, allocator);
+
+    // Then process handler_expression once, with op_count = ops.len().
+    // In Angular TS, the return expression is the last entry in handlerOps,
+    // so its op index equals the number of preceding ops.
+    if let Some(handler_expr) = handler_expression.as_mut() {
+        let op_count = ops.len();
+
+        // Pass 1: Count reads per xref to determine final reads
+        let read_counts = RefCell::new(FxHashMap::<XrefId, usize>::default());
+        visit_expressions_in_expression(
+            handler_expr,
+            &|expr, flags| {
+                if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
+                    return;
+                }
+                if let IrExpression::ReadTemporary(read) = expr {
+                    *read_counts.borrow_mut().entry(read.xref).or_insert(0) += 1;
+                }
+            },
+            VisitorContextFlag::NONE,
+        );
+        let total_reads = read_counts.into_inner();
+
+        // Pass 2: Assign names with reuse when final read is encountered
+        let tracker = RefCell::new(TempVarTracker::new(op_count, total_reads));
+        transform_expressions_in_expression(
+            handler_expr,
+            &|expr, flags| {
+                if flags.contains(VisitorContextFlag::IN_CHILD_OPERATION) {
+                    return;
+                }
+                assign_temp_names(expr, &tracker, allocator);
+            },
+            VisitorContextFlag::NONE,
+        );
+
+        // Collect unique names and create declarations
+        let defs = tracker.into_inner().defs;
+        for name in collect_unique_names(&defs) {
+            let stmt = create_declare_var_statement(allocator, &name);
+            generated_statements.push(UpdateOp::Statement(StatementOp {
+                base: UpdateOpBase::default(),
+                statement: stmt,
+            }));
+        }
+    }
+
+    generated_statements
+}
+
 /// Prepends generated statement ops to a handler_ops Vec.
 /// Since handler_ops is a Vec (not OpList), we need to handle prepending manually.
 fn prepend_update_ops<'a>(

crates/oxc_angular_compiler/tests/integration_test.rs

@@ -1040,6 +1040,41 @@ fn test_safe_call() {
     insta::assert_snapshot!("safe_call", js);
 }
 
+#[test]
+fn test_safe_call_in_listener() {
+    // When a listener handler contains `fn()?.method()`, Angular generates a temporary variable
+    // because `fn()` is a function call that shouldn't be evaluated twice.
+    // Expected output should contain `let tmp_N_0;` declaration inside the listener function.
+    let js = compile_template_to_js(
+        r#"<button (click)="getPopover()?.close()">Close</button>"#,
+        "TestComponent",
+    );
+    insta::assert_snapshot!("safe_call_in_listener", js);
+}
+
+#[test]
+fn test_safe_property_read_with_call_receiver_in_listener() {
+    // Pattern: `fn()?.prop` in listener — receiver is a call, needs tmp variable
+    let js = compile_template_to_js(
+        r#"<button (click)="getDialog()?.visible">Toggle</button>"#,
+        "TestComponent",
+    );
+    insta::assert_snapshot!("safe_property_read_with_call_receiver_in_listener", js);
+}
+
+#[test]
+fn test_safe_call_in_listener_inside_conditional() {
+    // When a listener is inside an embedded view (e.g., @if), handler_ops contains
+    // restoreView and nextContext statements. The handler_expression (return value)
+    // must be processed AFTER those ops, not once per op, to get the correct
+    // tmp variable name (tmp_2_0, matching the op index after restoreView and nextContext).
+    let js = compile_template_to_js(
+        r#"@if (show) { <button (click)="getPopover()?.close()">Close</button> }"#,
+        "TestComponent",
+    );
+    insta::assert_snapshot!("safe_call_in_listener_inside_conditional", js);
+}
+
 // ============================================================================
 // Event Modifier Tests
 // ============================================================================

crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener.snap

@@ -0,0 +1,15 @@
+---
+source: crates/oxc_angular_compiler/tests/integration_test.rs
+expression: js
+---
+function TestComponent_Template(rf,ctx) {
+  if ((rf & 1)) {
+    i0.ɵɵelementStart(0,"button",0);
+    i0.ɵɵlistener("click",function TestComponent_Template_button_click_0_listener() {
+      let tmp_0_0;
+      return (((tmp_0_0 = ctx.getPopover()) == null)? null: tmp_0_0.close());
+    });
+    i0.ɵɵtext(1,"Close");
+    i0.ɵɵelementEnd();
+  }
+}

crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener_inside_conditional.snap

@@ -0,0 +1,24 @@
+---
+source: crates/oxc_angular_compiler/tests/integration_test.rs
+expression: js
+---
+function TestComponent_Conditional_0_Template(rf,ctx) {
+  if ((rf & 1)) {
+    const _r1 = i0.ɵɵgetCurrentView();
+    i0.ɵɵtext(0," ");
+    i0.ɵɵelementStart(1,"button",0);
+    i0.ɵɵlistener("click",function TestComponent_Conditional_0_Template_button_click_1_listener() {
+      let tmp_2_0;
+      i0.ɵɵrestoreView(_r1);
+      const ctx_r1 = i0.ɵɵnextContext();
+      return i0.ɵɵresetView((((tmp_2_0 = ctx_r1.getPopover()) == null)? null: tmp_2_0.close()));
+    });
+    i0.ɵɵtext(2,"Close");
+    i0.ɵɵelementEnd();
+    i0.ɵɵtext(3," ");
+  }
+}
+function TestComponent_Template(rf,ctx) {
+  if ((rf & 1)) { i0.ɵɵconditionalCreate(0,TestComponent_Conditional_0_Template,4,0); }
+  if ((rf & 2)) { i0.ɵɵconditional((ctx.show? 0: -1)); }
+}

crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_property_read_with_call_receiver_in_listener.snap

@@ -0,0 +1,15 @@
+---
+source: crates/oxc_angular_compiler/tests/integration_test.rs
+expression: js
+---
+function TestComponent_Template(rf,ctx) {
+  if ((rf & 1)) {
+    i0.ɵɵelementStart(0,"button",0);
+    i0.ɵɵlistener("click",function TestComponent_Template_button_click_0_listener() {
+      let tmp_0_0;
+      return (((tmp_0_0 = ctx.getDialog()) == null)? null: tmp_0_0.visible);
+    });
+    i0.ɵɵtext(1,"Toggle");
+    i0.ɵɵelementEnd();
+  }
+}