Skip to content

Commit fe7dc68

Browse files
committed
fix: _unnamed_N variable references
1 parent 8813cff commit fe7dc68

File tree

4 files changed

+226
-3
lines changed

4 files changed

+226
-3
lines changed

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

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,61 @@ fn collect_op_fences(op: &UpdateOp<'_>) -> Fence {
275275
UpdateOp::Control(ctrl) => collect_fences(&ctrl.expression),
276276
UpdateOp::I18nExpression(i18n) => collect_fences(&i18n.expression),
277277
UpdateOp::DeferWhen(defer_when) => collect_fences(&defer_when.condition),
278-
UpdateOp::Statement(_) => Fence::NONE,
278+
UpdateOp::Statement(stmt) => collect_fences_in_output_statement(&stmt.statement),
279+
_ => Fence::NONE,
280+
}
281+
}
282+
283+
/// Collect fences from an OutputStatement.
284+
fn collect_fences_in_output_statement(stmt: &OutputStatement<'_>) -> Fence {
285+
match stmt {
286+
OutputStatement::Expression(expr_stmt) => {
287+
collect_fences_in_output_expression(&expr_stmt.expr)
288+
}
289+
OutputStatement::If(if_stmt) => {
290+
let mut fences = collect_fences_in_output_expression(&if_stmt.condition);
291+
for s in if_stmt.true_case.iter() {
292+
fences |= collect_fences_in_output_statement(s);
293+
}
294+
for s in if_stmt.false_case.iter() {
295+
fences |= collect_fences_in_output_statement(s);
296+
}
297+
fences
298+
}
299+
OutputStatement::Return(ret) => collect_fences_in_output_expression(&ret.value),
300+
_ => Fence::NONE,
301+
}
302+
}
303+
304+
/// Collect fences from an OutputExpression.
305+
fn collect_fences_in_output_expression(expr: &OutputExpression<'_>) -> Fence {
306+
match expr {
307+
OutputExpression::WrappedIrNode(wrapped) => collect_fences(&wrapped.node),
308+
OutputExpression::BinaryOperator(bin) => {
309+
collect_fences_in_output_expression(&bin.lhs)
310+
| collect_fences_in_output_expression(&bin.rhs)
311+
}
312+
OutputExpression::Conditional(cond) => {
313+
let mut fences = collect_fences_in_output_expression(&cond.condition);
314+
fences |= collect_fences_in_output_expression(&cond.true_case);
315+
if let Some(ref false_case) = cond.false_case {
316+
fences |= collect_fences_in_output_expression(false_case);
317+
}
318+
fences
319+
}
320+
OutputExpression::InvokeFunction(call) => {
321+
let mut fences = collect_fences_in_output_expression(&call.fn_expr);
322+
for arg in call.args.iter() {
323+
fences |= collect_fences_in_output_expression(arg);
324+
}
325+
fences
326+
}
327+
OutputExpression::Not(unary) => collect_fences_in_output_expression(&unary.condition),
328+
OutputExpression::ReadProp(prop) => collect_fences_in_output_expression(&prop.receiver),
329+
OutputExpression::ReadKey(keyed) => {
330+
collect_fences_in_output_expression(&keyed.receiver)
331+
| collect_fences_in_output_expression(&keyed.index)
332+
}
279333
_ => Fence::NONE,
280334
}
281335
}
@@ -1224,6 +1278,9 @@ where
12241278
UpdateOp::DeferWhen(defer_when) => {
12251279
visit_all_expressions(&defer_when.condition, &mut visitor)
12261280
}
1281+
UpdateOp::Statement(stmt) => {
1282+
visit_ir_expressions_in_output_statement(&stmt.statement, &mut visitor);
1283+
}
12271284
_ => {}
12281285
}
12291286
}
@@ -1356,6 +1413,86 @@ where
13561413
}
13571414
}
13581415

1416+
/// Visit IR expressions inside an OutputStatement.
1417+
///
1418+
/// Statement ops are created by `convert_variables_to_statements` when unused
1419+
/// side-effectful variables (like StoreLet) are converted from Variable ops to
1420+
/// standalone expression statements. The IR expressions inside these statements
1421+
/// still contain ReadVariable references that must be counted for correct
1422+
/// context variable inlining decisions.
1423+
fn visit_ir_expressions_in_output_statement<F>(stmt: &OutputStatement<'_>, visitor: &mut F)
1424+
where
1425+
F: FnMut(&IrExpression<'_>),
1426+
{
1427+
match stmt {
1428+
OutputStatement::Expression(expr_stmt) => {
1429+
visit_ir_expressions_in_output_expression(&expr_stmt.expr, visitor);
1430+
}
1431+
OutputStatement::If(if_stmt) => {
1432+
visit_ir_expressions_in_output_expression(&if_stmt.condition, visitor);
1433+
for s in if_stmt.true_case.iter() {
1434+
visit_ir_expressions_in_output_statement(s, visitor);
1435+
}
1436+
for s in if_stmt.false_case.iter() {
1437+
visit_ir_expressions_in_output_statement(s, visitor);
1438+
}
1439+
}
1440+
OutputStatement::Return(ret) => {
1441+
visit_ir_expressions_in_output_expression(&ret.value, visitor);
1442+
}
1443+
_ => {}
1444+
}
1445+
}
1446+
1447+
/// Visit IR expressions inside an OutputExpression.
1448+
///
1449+
/// Traverses OutputExpression tree to find WrappedIrNode nodes which contain
1450+
/// actual IR expressions that may reference variables.
1451+
fn visit_ir_expressions_in_output_expression<F>(expr: &OutputExpression<'_>, visitor: &mut F)
1452+
where
1453+
F: FnMut(&IrExpression<'_>),
1454+
{
1455+
match expr {
1456+
OutputExpression::WrappedIrNode(wrapped) => {
1457+
visit_all_expressions(&wrapped.node, visitor);
1458+
}
1459+
OutputExpression::BinaryOperator(bin) => {
1460+
visit_ir_expressions_in_output_expression(&bin.lhs, visitor);
1461+
visit_ir_expressions_in_output_expression(&bin.rhs, visitor);
1462+
}
1463+
OutputExpression::Conditional(cond) => {
1464+
visit_ir_expressions_in_output_expression(&cond.condition, visitor);
1465+
visit_ir_expressions_in_output_expression(&cond.true_case, visitor);
1466+
if let Some(ref false_case) = cond.false_case {
1467+
visit_ir_expressions_in_output_expression(false_case, visitor);
1468+
}
1469+
}
1470+
OutputExpression::InvokeFunction(call) => {
1471+
visit_ir_expressions_in_output_expression(&call.fn_expr, visitor);
1472+
for arg in call.args.iter() {
1473+
visit_ir_expressions_in_output_expression(arg, visitor);
1474+
}
1475+
}
1476+
OutputExpression::Not(unary) => {
1477+
visit_ir_expressions_in_output_expression(&unary.condition, visitor);
1478+
}
1479+
OutputExpression::Instantiate(inst) => {
1480+
visit_ir_expressions_in_output_expression(&inst.class_expr, visitor);
1481+
for arg in inst.args.iter() {
1482+
visit_ir_expressions_in_output_expression(arg, visitor);
1483+
}
1484+
}
1485+
OutputExpression::ReadProp(prop) => {
1486+
visit_ir_expressions_in_output_expression(&prop.receiver, visitor);
1487+
}
1488+
OutputExpression::ReadKey(keyed) => {
1489+
visit_ir_expressions_in_output_expression(&keyed.receiver, visitor);
1490+
visit_ir_expressions_in_output_expression(&keyed.index, visitor);
1491+
}
1492+
_ => {}
1493+
}
1494+
}
1495+
13591496
/// Result of collecting variables that need to be processed.
13601497
struct VariableActions {
13611498
/// Variables that should be completely removed (unused, not side-effectful).

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,40 @@ fn test_let_used_in_ngif_child_view() {
542542
insta::assert_snapshot!("let_used_in_ngif_child_view", js);
543543
}
544544

545+
#[test]
546+
fn test_let_inside_for_if_with_component_method_call() {
547+
// Reproduces the _unnamed_N bug from ClickUp's stylesheet-viewer.component.html.
548+
//
549+
// Pattern: @for → @if → TWO @let declarations calling component methods
550+
//
551+
// When there are two @let expressions that both need the component context,
552+
// the context variable can't be inlined (used twice) and must be extracted
553+
// as a named variable like `const ctx_rN = i0.ɵɵnextContext()`.
554+
//
555+
// Expected (Angular):
556+
// const item_rN = i0.ɵɵnextContext().$implicit;
557+
// const ctx_rN = i0.ɵɵnextContext();
558+
// i0.ɵɵstoreLet(ctx_rN.computeA(item_rN.id));
559+
// ...
560+
// const b_rN = i0.ɵɵstoreLet(ctx_rN.computeB(item_rN.text));
561+
//
562+
// Actual (Oxc bug):
563+
// const item_rN = i0.ɵɵnextContext().$implicit;
564+
// i0.ɵɵstoreLet(_unnamed_N.computeA(item_rN.id));
565+
// ...
566+
// const b_rN = i0.ɵɵstoreLet(i0.ɵɵnextContext().computeB(item_rN.text));
567+
let js = compile_template_to_js(
568+
r#"@for (item of items; track item) { @if (showDetail()) { @let a = computeA(item.id); @let b = computeB(item.text); @if (a > 0) { <div>{{b}}</div> } } }"#,
569+
"TestComponent",
570+
);
571+
// The output must NOT contain _unnamed_ - all variables should be properly named
572+
assert!(
573+
!js.contains("_unnamed_"),
574+
"Generated JS contains _unnamed_ references, indicating a naming bug.\nGenerated JS:\n{js}"
575+
);
576+
insta::assert_snapshot!("let_inside_for_if_with_component_method_call", js);
577+
}
578+
545579
// ============================================================================
546580
// ng-content Tests
547581
// ============================================================================
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
source: crates/oxc_angular_compiler/tests/integration_test.rs
3+
assertion_line: 576
4+
expression: js
5+
---
6+
function TestComponent_For_1_Conditional_1_Conditional_4_Template(rf,ctx) {
7+
if ((rf & 1)) {
8+
i0.ɵɵtext(0," ");
9+
i0.ɵɵelementStart(1,"div");
10+
i0.ɵɵtext(2);
11+
i0.ɵɵelementEnd();
12+
i0.ɵɵtext(3," ");
13+
}
14+
if ((rf & 2)) {
15+
i0.ɵɵnextContext();
16+
const b_r1 = i0.ɵɵreadContextLet(2);
17+
i0.ɵɵadvance(2);
18+
i0.ɵɵtextInterpolate(b_r1);
19+
}
20+
}
21+
function TestComponent_For_1_Conditional_1_Template(rf,ctx) {
22+
if ((rf & 1)) {
23+
i0.ɵɵtext(0," ");
24+
i0.ɵɵtext(1," ");
25+
i0.ɵɵdeclareLet(2);
26+
i0.ɵɵtext(3," ");
27+
i0.ɵɵconditionalCreate(4,TestComponent_For_1_Conditional_1_Conditional_4_Template,
28+
4,1);
29+
}
30+
if ((rf & 2)) {
31+
const item_r2 = i0.ɵɵnextContext().$implicit;
32+
const ctx_r2 = i0.ɵɵnextContext();
33+
const a_r4 = ctx_r2.computeA(item_r2.id);
34+
i0.ɵɵadvance(2);
35+
i0.ɵɵstoreLet(ctx_r2.computeB(item_r2.text));
36+
i0.ɵɵadvance(2);
37+
i0.ɵɵconditional(((a_r4 > 0)? 4: -1));
38+
}
39+
}
40+
function TestComponent_For_1_Template(rf,ctx) {
41+
if ((rf & 1)) {
42+
i0.ɵɵtext(0," ");
43+
i0.ɵɵconditionalCreate(1,TestComponent_For_1_Conditional_1_Template,5,2);
44+
}
45+
if ((rf & 2)) {
46+
const ctx_r2 = i0.ɵɵnextContext();
47+
i0.ɵɵadvance();
48+
i0.ɵɵconditional((ctx_r2.showDetail()? 1: -1));
49+
}
50+
}
51+
function TestComponent_Template(rf,ctx) {
52+
if ((rf & 1)) { i0.ɵɵrepeaterCreate(0,TestComponent_For_1_Template,2,1,null,null,i0.ɵɵrepeaterTrackByIdentity); }
53+
if ((rf & 2)) { i0.ɵɵrepeater(ctx.items); }
54+
}

napi/angular-compiler/e2e/compare/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
"compare:clickup": "NODE_OPTIONS='--max-old-space-size=32768' oxnode src/index.ts --preset clickup --full-file",
1111
"compare:clickup:fast": "NODE_OPTIONS='--max-old-space-size=16384' oxnode src/index.ts --preset clickup --full-file --ng-baseline ./ng-baseline-clickup.json",
1212
"compare:clickup:save-baseline": "NODE_OPTIONS='--max-old-space-size=32768' oxnode src/index.ts --preset clickup --full-file --generate-ng-baseline --save-ng-baseline ./ng-baseline-clickup.json",
13-
"compare:clickup-core": "NODE_OPTIONS='--max-old-space-size=16384' oxnode src/index.ts --preset clickup-core --full-file",
14-
"compare:clickup-client": "NODE_OPTIONS='--max-old-space-size=16384' oxnode src/index.ts --preset clickup-client --full-file",
1513
"show-diff": "oxnode src/show-diff.ts",
1614
"test": "vitest run"
1715
},

0 commit comments

Comments
 (0)