Skip to content

Commit c14c90b

Browse files
Brooooooklynclaude
andauthored
fix: skip optimize_save_restore_view for Animation handler_ops (#3)
Angular's ngtsc keeps restoreView/resetView in animation callbacks even when the return value doesn't reference the view context (e.g., `return "animate-in"`). OXC was incorrectly removing these via optimize_save_restore_view, causing 37 ClickUp component mismatches. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 78b7a27 commit c14c90b

File tree

4 files changed

+148
-1
lines changed

4 files changed

+148
-1
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1749,7 +1749,11 @@ fn optimize_listener_handler_ops<'a>(job: &mut ComponentCompilationJob<'a>) {
17491749
}
17501750
CreateOp::Animation(animation) => {
17511751
optimize_handler_ops(&mut animation.handler_ops, None, allocator);
1752-
optimize_save_restore_view(&mut animation.handler_ops, allocator);
1752+
// Note: We intentionally do NOT call optimize_save_restore_view on
1753+
// Animation handler_ops. Angular's ngtsc output keeps restoreView/resetView
1754+
// in animation callbacks even when the return value doesn't reference the
1755+
// view context (e.g., `return "animate-in"`). Skipping this optimization
1756+
// for Animation handlers matches the observed Angular output.
17531757
}
17541758
_ => {}
17551759
}

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3325,6 +3325,90 @@ fn test_animation_in_for_with_listener_variable_naming() {
33253325
insta::assert_snapshot!("animation_in_for_with_listener", js);
33263326
}
33273327

3328+
#[test]
3329+
fn test_animation_enter_string_literal_in_embedded_view() {
3330+
// Reproduces the edit-long-text-custom-field-value.component.ts issue:
3331+
// An animation handler that returns a string literal in an embedded view (@if)
3332+
// should still have getCurrentView/restoreView/resetView generated.
3333+
//
3334+
// Angular always adds restoreView/resetView to animation handlers in embedded views,
3335+
// even when the handler expression is a simple string literal.
3336+
// The variable_optimization phase may later optimize away the restoreView if the
3337+
// handler doesn't actually access outer context, but the getCurrentView at the
3338+
// view level must survive if there are other listeners that reference it.
3339+
//
3340+
// NG output pattern for animation returning string literal in embedded view:
3341+
// const _r1 = i0.ɵɵgetCurrentView();
3342+
// i0.ɵɵelementStart(0, "div", 0);
3343+
// i0.ɵɵanimateEnter(function ..._cb() {
3344+
// i0.ɵɵrestoreView(_r1);
3345+
// const ctx_r1 = i0.ɵɵnextContext();
3346+
// return i0.ɵɵresetView(ctx_r1.onClick());
3347+
// });
3348+
// i0.ɵɵlistener("click", function ..._listener() { ... });
3349+
//
3350+
// OXC bug: missing getCurrentView because the animation handler's restoreView
3351+
// gets optimized away (string literal doesn't reference outer context), and if
3352+
// the SavedView variable optimization also removes the getCurrentView, other
3353+
// listeners in the same view lose their restoreView target.
3354+
let js = compile_template_to_js(
3355+
r#"@if (show) {
3356+
<div [animate.enter]="'animate-in'" (click)="onClick()">
3357+
{{label}}
3358+
</div>
3359+
}"#,
3360+
"TestComponent",
3361+
);
3362+
assert!(
3363+
!js.contains("_unnamed_"),
3364+
"Generated JS contains _unnamed_ references.\nGenerated JS:\n{js}"
3365+
);
3366+
assert!(
3367+
js.contains("getCurrentView"),
3368+
"Embedded view with animation and listener should have getCurrentView.\nGenerated JS:\n{js}"
3369+
);
3370+
assert!(
3371+
js.contains("restoreView"),
3372+
"Listener in embedded view should have restoreView.\nGenerated JS:\n{js}"
3373+
);
3374+
insta::assert_snapshot!("animation_enter_string_literal_embedded_view", js);
3375+
}
3376+
3377+
#[test]
3378+
fn test_animation_enter_string_literal_only_in_embedded_view() {
3379+
// Tests the case where an animation handler returning a string literal is the ONLY
3380+
// listener-like op in an embedded view. Angular's ngtsc always keeps restoreView/resetView
3381+
// in animation handler callbacks in embedded views, even when the return value is a simple
3382+
// string literal that doesn't reference the view context.
3383+
//
3384+
// Expected NG output pattern:
3385+
// i0.ɵɵanimateEnter(function ...() {
3386+
// i0.ɵɵrestoreView(_r1);
3387+
// return i0.ɵɵresetView("animate-in");
3388+
// });
3389+
let js = compile_template_to_js(
3390+
r#"@if (show) {
3391+
<div [animate.enter]="'animate-in'">
3392+
{{label}}
3393+
</div>
3394+
}"#,
3395+
"TestComponent",
3396+
);
3397+
assert!(
3398+
!js.contains("_unnamed_"),
3399+
"Generated JS contains _unnamed_ references.\nGenerated JS:\n{js}"
3400+
);
3401+
assert!(
3402+
js.contains("restoreView"),
3403+
"Animation handler in embedded view should keep restoreView.\nGenerated JS:\n{js}"
3404+
);
3405+
assert!(
3406+
js.contains("resetView"),
3407+
"Animation handler in embedded view should keep resetView.\nGenerated JS:\n{js}"
3408+
);
3409+
insta::assert_snapshot!("animation_enter_string_literal_only_embedded_view", js);
3410+
}
3411+
33283412
/// Test that implicit standalone components (no `standalone` in decorator) use Full mode.
33293413
///
33303414
/// Angular 19+ defaults `standalone` to `true` when not specified. However, OXC performs
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
source: crates/oxc_angular_compiler/tests/integration_test.rs
3+
expression: js
4+
---
5+
function TestComponent_Conditional_0_Template(rf,ctx) {
6+
if ((rf & 1)) {
7+
const _r1 = i0.ɵɵgetCurrentView();
8+
i0.ɵɵtext(0,"\n ");
9+
i0.ɵɵelementStart(1,"div",0);
10+
i0.ɵɵanimateEnter(function TestComponent_Conditional_0_Template_animateenter_cb() {
11+
i0.ɵɵrestoreView(_r1);
12+
return i0.ɵɵresetView("animate-in");
13+
});
14+
i0.ɵɵlistener("click",function TestComponent_Conditional_0_Template_div_click_1_listener() {
15+
i0.ɵɵrestoreView(_r1);
16+
const ctx_r1 = i0.ɵɵnextContext();
17+
return i0.ɵɵresetView(ctx_r1.onClick());
18+
});
19+
i0.ɵɵtext(2);
20+
i0.ɵɵelementEnd();
21+
i0.ɵɵtext(3,"\n ");
22+
}
23+
if ((rf & 2)) {
24+
const ctx_r1 = i0.ɵɵnextContext();
25+
i0.ɵɵadvance(2);
26+
i0.ɵɵtextInterpolate1("\n ",ctx_r1.label,"\n ");
27+
}
28+
}
29+
function TestComponent_Template(rf,ctx) {
30+
if ((rf & 1)) { i0.ɵɵconditionalCreate(0,TestComponent_Conditional_0_Template,4,1); }
31+
if ((rf & 2)) { i0.ɵɵconditional((ctx.show? 0: -1)); }
32+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
source: crates/oxc_angular_compiler/tests/integration_test.rs
3+
expression: js
4+
---
5+
function TestComponent_Conditional_0_Template(rf,ctx) {
6+
if ((rf & 1)) {
7+
const _r1 = i0.ɵɵgetCurrentView();
8+
i0.ɵɵtext(0,"\n ");
9+
i0.ɵɵelementStart(1,"div");
10+
i0.ɵɵanimateEnter(function TestComponent_Conditional_0_Template_animateenter_cb() {
11+
i0.ɵɵrestoreView(_r1);
12+
return i0.ɵɵresetView("animate-in");
13+
});
14+
i0.ɵɵtext(2);
15+
i0.ɵɵelementEnd();
16+
i0.ɵɵtext(3,"\n ");
17+
}
18+
if ((rf & 2)) {
19+
const ctx_r1 = i0.ɵɵnextContext();
20+
i0.ɵɵadvance(2);
21+
i0.ɵɵtextInterpolate1("\n ",ctx_r1.label,"\n ");
22+
}
23+
}
24+
function TestComponent_Template(rf,ctx) {
25+
if ((rf & 1)) { i0.ɵɵconditionalCreate(0,TestComponent_Conditional_0_Template,4,1); }
26+
if ((rf & 2)) { i0.ɵɵconditional((ctx.show? 0: -1)); }
27+
}

0 commit comments

Comments
 (0)