diff --git a/crates/oxc_angular_compiler/src/ir/expression.rs b/crates/oxc_angular_compiler/src/ir/expression.rs index 5f1b6b10d..a58538add 100644 --- a/crates/oxc_angular_compiler/src/ir/expression.rs +++ b/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); } diff --git a/crates/oxc_angular_compiler/src/pipeline/phases/temporary_variables.rs b/crates/oxc_angular_compiler/src/pipeline/phases/temporary_variables.rs index 0111fe8c2..6c2bced42 100644 --- a/crates/oxc_angular_compiler/src/pipeline/phases/temporary_variables.rs +++ b/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>>, + allocator: &'a Allocator, +) -> Vec> { + // 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::::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>( diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index a1b339e9c..dbc37245c 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/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#""#, + "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#""#, + "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) { }"#, + "TestComponent", + ); + insta::assert_snapshot!("safe_call_in_listener_inside_conditional", js); +} + // ============================================================================ // Event Modifier Tests // ============================================================================ diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener.snap new file mode 100644 index 000000000..462f7b737 --- /dev/null +++ b/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(); + } +} diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener_inside_conditional.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_call_in_listener_inside_conditional.snap new file mode 100644 index 000000000..93c69bf78 --- /dev/null +++ b/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)); } +} diff --git a/crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_property_read_with_call_receiver_in_listener.snap b/crates/oxc_angular_compiler/tests/snapshots/integration_test__safe_property_read_with_call_receiver_in_listener.snap new file mode 100644 index 000000000..e5643ec44 --- /dev/null +++ b/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(); + } +}