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
13 changes: 7 additions & 6 deletions crates/oxc_angular_compiler/src/ir/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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>(
Expand Down
35 changes: 35 additions & 0 deletions crates/oxc_angular_compiler/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ============================================================================
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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)); }
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading