Skip to content

Commit 3661037

Browse files
Brooooooklynclaude
andauthored
fix: generate tmp variables for safe navigation on binary/ternary/unary expressions (#16)
needs_temporary_in_safe_access() in expand_safe_reads was missing match arms for IrExpression::Binary, Ternary, Not, and Unary. When a safe property read like (pipeBind(...) || fallback)?.name had a Binary receiver, the function fell through to `_ => false` instead of recursing into operands to detect the pipe binding. This caused the pipe to be evaluated twice with inflated binding slot indices. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cf75ff7 commit 3661037

3 files changed

Lines changed: 46 additions & 0 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,20 @@ fn needs_temporary_in_safe_access(expr: &IrExpression<'_>) -> bool {
174174
needs_temporary_in_safe_access(&keyed.receiver)
175175
|| needs_temporary_in_safe_access(&keyed.key)
176176
}
177+
// Binary operators need to check both operands
178+
IrExpression::Binary(bin) => {
179+
needs_temporary_in_safe_access(&bin.lhs) || needs_temporary_in_safe_access(&bin.rhs)
180+
}
181+
// Ternary needs to check all branches
182+
IrExpression::Ternary(ternary) => {
183+
needs_temporary_in_safe_access(&ternary.condition)
184+
|| needs_temporary_in_safe_access(&ternary.true_expr)
185+
|| needs_temporary_in_safe_access(&ternary.false_expr)
186+
}
187+
// Not expression needs to check operand
188+
IrExpression::Not(not) => needs_temporary_in_safe_access(&not.expr),
189+
// Unary operator needs to check operand
190+
IrExpression::Unary(unary) => needs_temporary_in_safe_access(&unary.expr),
177191
// Check AST expressions for function calls
178192
IrExpression::Ast(ast_expr) => needs_temporary_in_ast_expression(ast_expr),
179193
// Parenthesized expressions need to check their inner expression

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,20 @@ fn test_safe_call_in_listener_inside_conditional() {
11561156
insta::assert_snapshot!("safe_call_in_listener_inside_conditional", js);
11571157
}
11581158

1159+
#[test]
1160+
fn test_pipe_in_binary_with_safe_property_read() {
1161+
// Pattern from EmailCommentComponent: (comment$ | async || comment)?.new_mentioned_thread_count
1162+
// When a pipe binding is inside a binary expression that is the receiver of a safe property read,
1163+
// the compiler must generate a temporary variable to avoid evaluating the pipe twice.
1164+
// TypeScript Angular compiler produces: (tmp = pipeBind(...) || fallback) == null ? null : tmp.prop
1165+
// Without the fix, OXC duplicates the pipe call in both the guard and the access expression.
1166+
let js = compile_template_to_js(
1167+
r#"<div>{{ ((data$ | async) || fallback)?.name }}</div>"#,
1168+
"TestComponent",
1169+
);
1170+
insta::assert_snapshot!("pipe_in_binary_with_safe_property_read", js);
1171+
}
1172+
11591173
// ============================================================================
11601174
// Event Modifier Tests
11611175
// ============================================================================
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
source: crates/oxc_angular_compiler/tests/integration_test.rs
3+
assertion_line: 1170
4+
expression: js
5+
---
6+
function TestComponent_Template(rf,ctx) {
7+
if ((rf & 1)) {
8+
i0.ɵɵelementStart(0,"div");
9+
i0.ɵɵtext(1);
10+
i0.ɵɵpipe(2,"async");
11+
i0.ɵɵelementEnd();
12+
}
13+
if ((rf & 2)) {
14+
let tmp_0_0;
15+
i0.ɵɵadvance();
16+
i0.ɵɵtextInterpolate((((tmp_0_0 = (i0.ɵɵpipeBind1(2,1,ctx.data$) || ctx.fallback)) == null)? null: tmp_0_0.name));
17+
}
18+
}

0 commit comments

Comments
 (0)