Skip to content

Commit c83c17d

Browse files
fix: compile animation trigger host bindings to ɵɵsyntheticHostProperty (#208)
* fix: compile animation trigger bindings in host property to ɵɵsyntheticHostProperty convert_animations_for_host was unconditionally converting all AnimationBindingOp entries to CreateOp::Animation (ɵɵanimateEnter/ɵɵanimateLeave), which is only correct for animate.enter/animate.leave bindings. Host [@trigger] bindings (AnimationBindingKind::Value) need to remain in the update list so they are reified as ɵɵsyntheticHostProperty in the rf & 2 block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle animation trigger bindings in directive host property The directive's parse_host_property_name was missing the @ animation check, causing host: { '[@slidein]': 'state' } on directives to emit ɵɵdomProperty instead of ɵɵsyntheticHostProperty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 581f4ed commit c83c17d

File tree

4 files changed

+188
-60
lines changed

4 files changed

+188
-60
lines changed

crates/oxc_angular_compiler/src/directive/compiler.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,9 @@ fn parse_host_property_name(name: &str) -> (BindingType, &str, Option<&str>) {
743743
}
744744
} else if let Some(rest) = name.strip_prefix("attr.") {
745745
(BindingType::Attribute, rest, None)
746+
} else if name.starts_with('@') {
747+
// Animation binding like @triggerName
748+
(BindingType::Animation, name, None)
746749
} else {
747750
(BindingType::Property, name, None)
748751
}

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

Lines changed: 17 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -273,19 +273,23 @@ fn create_placeholder_expression<'a>(
273273
pub fn convert_animations_for_host(job: &mut HostBindingCompilationJob<'_>) {
274274
let allocator = job.allocator;
275275

276-
// First pass: collect all AnimationBindingOp pointers
276+
// First pass: collect all AnimationBindingOp pointers that need conversion.
277+
// Skip AnimationBindingKind::Value ops — these are [@trigger] host bindings that
278+
// should remain in the update list and be reified as ɵɵsyntheticHostProperty.
279+
// Only AnimationBindingKind::String ops (animate.enter/animate.leave) are converted.
277280
let binding_ptrs: Vec<std::ptr::NonNull<UpdateOp<'_>>> = {
278281
let mut ptrs = Vec::new();
279282
for op in job.root.update.iter() {
280-
if matches!(op, UpdateOp::AnimationBinding(_)) {
281-
ptrs.push(std::ptr::NonNull::from(op));
283+
if let UpdateOp::AnimationBinding(binding) = op {
284+
if matches!(binding.kind, AnimationBindingKind::String) {
285+
ptrs.push(std::ptr::NonNull::from(op));
286+
}
282287
}
283288
}
284289
ptrs
285290
};
286291

287-
// Second pass: process each AnimationBindingOp
288-
let mut animations_to_create: Vec<AnimationInfo<'_>> = Vec::new();
292+
// Second pass: process each AnimationBindingOp (String kind only)
289293
let mut strings_to_create: Vec<AnimationStringInfo<'_>> = Vec::new();
290294

291295
for ptr in binding_ptrs {
@@ -295,7 +299,6 @@ pub fn convert_animations_for_host(job: &mut HostBindingCompilationJob<'_>) {
295299
let target = binding.target;
296300
let source_span = binding.base.source_span;
297301
let name = binding.name.clone();
298-
let kind = binding.kind;
299302
let animation_kind = get_animation_kind(name.as_str());
300303

301304
// Extract expression by replacing with placeholder
@@ -316,63 +319,17 @@ pub fn convert_animations_for_host(job: &mut HostBindingCompilationJob<'_>) {
316319
// SAFETY: ptr was obtained from this list
317320
unsafe { job.root.update.remove(ptr) };
318321

319-
match kind {
320-
AnimationBindingKind::String => {
321-
strings_to_create.push(AnimationStringInfo {
322-
target,
323-
name,
324-
animation_kind,
325-
expression,
326-
source_span,
327-
});
328-
}
329-
AnimationBindingKind::Value => {
330-
// Create handler_ops with a return statement containing the expression
331-
let mut handler_ops = OxcVec::new_in(allocator);
332-
333-
let wrapped_expr = OutputExpression::WrappedIrNode(Box::new_in(
334-
WrappedIrExpr { node: expression, source_span },
335-
allocator,
336-
));
337-
338-
let return_stmt = OutputStatement::Return(Box::new_in(
339-
ReturnStatement { value: wrapped_expr, source_span: None },
340-
allocator,
341-
));
342-
343-
handler_ops.push(UpdateOp::Statement(StatementOp {
344-
base: UpdateOpBase { source_span, ..Default::default() },
345-
statement: return_stmt,
346-
}));
347-
348-
animations_to_create.push(AnimationInfo {
349-
target,
350-
name,
351-
animation_kind,
352-
handler_ops,
353-
source_span,
354-
});
355-
}
356-
}
322+
strings_to_create.push(AnimationStringInfo {
323+
target,
324+
name,
325+
animation_kind,
326+
expression,
327+
source_span,
328+
});
357329
}
358330
}
359331

360-
// Third pass: add Animation CreateOps to create list
361-
// Host bindings don't have element targets, so we just push to the create list
362-
for info in animations_to_create {
363-
job.root.create.push(CreateOp::Animation(AnimationOp {
364-
base: CreateOpBase { source_span: info.source_span, ..Default::default() },
365-
target: info.target,
366-
name: info.name,
367-
animation_kind: info.animation_kind,
368-
handler_ops: info.handler_ops,
369-
handler_fn_name: None,
370-
i18n_message: None,
371-
security_context: SecurityContext::None,
372-
sanitizer: None,
373-
}));
374-
}
375-
332+
// Third pass: add AnimationString CreateOps to create list
376333
for info in strings_to_create {
377334
job.root.create.push(CreateOp::AnimationString(AnimationStringOp {
378335
base: CreateOpBase { source_span: info.source_span, ..Default::default() },

crates/oxc_angular_compiler/tests/integration_test.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2299,6 +2299,84 @@ fn test_animate_enter_and_leave_together() {
22992299
);
23002300
}
23012301

2302+
#[test]
2303+
fn test_host_animation_trigger_binding() {
2304+
// Component with animation trigger in host property should emit ɵɵsyntheticHostProperty
2305+
let source = r#"
2306+
import { Component } from '@angular/core';
2307+
import { trigger, transition, style, animate } from '@angular/animations';
2308+
2309+
@Component({
2310+
selector: 'app-slide',
2311+
template: '<ng-content></ng-content>',
2312+
animations: [trigger('slideIn', [transition(':enter', [style({ width: 0 }), animate('200ms')])])],
2313+
host: {
2314+
'[@slideIn]': 'animationState',
2315+
}
2316+
})
2317+
export class SlideComponent {
2318+
animationState = 'active';
2319+
}
2320+
"#;
2321+
let allocator = Allocator::default();
2322+
let result = transform_angular_file(&allocator, "slide.component.ts", source, None, None);
2323+
assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
2324+
2325+
let code = &result.code;
2326+
2327+
// Should have ɵɵsyntheticHostProperty in the hostBindings update block
2328+
assert!(
2329+
code.contains("syntheticHostProperty"),
2330+
"Expected ɵɵsyntheticHostProperty for host animation trigger.\nGot:\n{code}"
2331+
);
2332+
assert!(
2333+
code.contains(r#"syntheticHostProperty("@slideIn""#),
2334+
"Expected syntheticHostProperty with @slideIn name.\nGot:\n{code}"
2335+
);
2336+
2337+
// Should NOT have ɵɵanimateEnter/ɵɵanimateLeave for [@trigger] bindings
2338+
assert!(
2339+
!code.contains("animateEnter") && !code.contains("animateLeave"),
2340+
"Host [@trigger] bindings should not use animateEnter/animateLeave.\nGot:\n{code}"
2341+
);
2342+
}
2343+
2344+
#[test]
2345+
fn test_directive_host_animation_trigger_binding() {
2346+
// Directive with animation trigger in host property should emit ɵɵsyntheticHostProperty
2347+
let source = r#"
2348+
import { Directive } from '@angular/core';
2349+
import { trigger, transition, style, animate } from '@angular/animations';
2350+
2351+
@Directive({
2352+
selector: '[appSlide]',
2353+
host: {
2354+
'[@slideIn]': 'animationState',
2355+
}
2356+
})
2357+
export class SlideDirective {
2358+
animationState = 'active';
2359+
}
2360+
"#;
2361+
let allocator = Allocator::default();
2362+
let result = transform_angular_file(&allocator, "slide.directive.ts", source, None, None);
2363+
assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
2364+
2365+
let code = &result.code;
2366+
2367+
// Should have ɵɵsyntheticHostProperty in the hostBindings update block
2368+
assert!(
2369+
code.contains(r#"syntheticHostProperty("@slideIn""#),
2370+
"Expected syntheticHostProperty with @slideIn name for directive.\nGot:\n{code}"
2371+
);
2372+
2373+
// Should NOT use regular hostProperty for animation triggers
2374+
assert!(
2375+
!code.contains(r#"hostProperty("@slideIn""#),
2376+
"Should not use hostProperty for animation triggers.\nGot:\n{code}"
2377+
);
2378+
}
2379+
23022380
/// Test that multiple components with host bindings in the same file have unique constant names.
23032381
///
23042382
/// This test simulates the real-world scenario from Material Angular's fab.ts where

napi/angular-compiler/e2e/compare/fixtures/animations/animation-host.fixture.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,74 @@
44
import type { Fixture } from '../types.js'
55

66
export const fixtures: Fixture[] = [
7+
{
8+
name: 'animation-host-property-trigger',
9+
category: 'animations',
10+
description: 'Animation trigger binding in host property',
11+
className: 'AnimationHostTriggerComponent',
12+
type: 'full-transform',
13+
sourceCode: `
14+
import { Component } from '@angular/core';
15+
import { trigger, transition, style, animate } from '@angular/animations';
16+
17+
@Component({
18+
selector: 'app-animation-host-trigger',
19+
standalone: true,
20+
template: \`<ng-content></ng-content>\`,
21+
animations: [
22+
trigger('slideIn', [
23+
transition(':enter', [
24+
style({ width: 0, opacity: 0 }),
25+
animate('200ms ease-out', style({ width: '*', opacity: 1 })),
26+
]),
27+
transition(':leave', [
28+
animate('200ms ease-in', style({ width: 0, opacity: 0 })),
29+
]),
30+
]),
31+
],
32+
host: {
33+
'[@slideIn]': 'animationState',
34+
}
35+
})
36+
export class AnimationHostTriggerComponent {
37+
animationState = 'active';
38+
}
39+
`.trim(),
40+
expectedFeatures: ['ɵɵsyntheticHostProperty'],
41+
},
42+
{
43+
name: 'animation-host-property-trigger-with-style',
44+
category: 'animations',
45+
description: 'Animation trigger binding combined with style binding in host property',
46+
className: 'AnimationHostTriggerWithStyleComponent',
47+
type: 'full-transform',
48+
sourceCode: `
49+
import { Component } from '@angular/core';
50+
import { trigger, transition, style, animate } from '@angular/animations';
51+
52+
@Component({
53+
selector: 'app-animation-host-trigger-with-style',
54+
standalone: true,
55+
template: \`<ng-content></ng-content>\`,
56+
animations: [
57+
trigger('slideIn', [
58+
transition(':enter', [
59+
style({ opacity: 0 }),
60+
animate('200ms ease-out', style({ opacity: 1 })),
61+
]),
62+
]),
63+
],
64+
host: {
65+
'[@slideIn]': 'animationState',
66+
'[style.overflow]': '"hidden"',
67+
}
68+
})
69+
export class AnimationHostTriggerWithStyleComponent {
70+
animationState = 'active';
71+
}
72+
`.trim(),
73+
expectedFeatures: ['ɵɵsyntheticHostProperty', 'ɵɵstyleProp'],
74+
},
775
{
876
name: 'animation-on-component',
977
category: 'animations',
@@ -67,6 +135,28 @@ import { Component } from '@angular/core';
67135
})
68136
export class AnimationParamsComponent {
69137
state = 'initial';
138+
}
139+
`.trim(),
140+
expectedFeatures: ['ɵɵsyntheticHostProperty'],
141+
},
142+
{
143+
name: 'animation-directive-host-property-trigger',
144+
category: 'animations',
145+
description: 'Animation trigger binding in directive host property',
146+
className: 'SlideDirective',
147+
type: 'full-transform',
148+
sourceCode: `
149+
import { Directive } from '@angular/core';
150+
import { trigger, transition, style, animate } from '@angular/animations';
151+
152+
@Directive({
153+
selector: '[appSlide]',
154+
host: {
155+
'[@slideIn]': 'animationState',
156+
}
157+
})
158+
export class SlideDirective {
159+
animationState = 'active';
70160
}
71161
`.trim(),
72162
expectedFeatures: ['ɵɵsyntheticHostProperty'],

0 commit comments

Comments
 (0)