refactor(core): Ensure determineLongestAnimation is run synchronously after style applies

thePunderWoman · mattrbeck · commit 318ade062e63 · 2026-03-13T13:03:11.000-06:00
This adds a setTimeout, which guarantees that we call getAnimations one frame after a reflow is finished. This means getAnimations will return data, avoiding needing the expensive fallback of getComputedStyles. It also updates the cleanup to prevent a potential memory leak if the component is destroyed before the timeout runs.
diff --git a/packages/core/src/render3/instructions/animation.ts b/packages/core/src/render3/instructions/animation.ts
@@ -136,6 +136,11 @@ export function runEnterAnimation(
 
   // We only need to add these event listeners if there are actual classes to apply
   if (activeClasses && activeClasses.length > 0) {
+    let isCleanedUp = false;
+    cleanupFns.push(() => {
+      isCleanedUp = true;
+    });
+
     ngZone.runOutsideAngular(() => {
       cleanupFns.push(renderer.listen(nativeElement, 'animationstart', handleEnterAnimationStart));
       cleanupFns.push(renderer.listen(nativeElement, 'transitionstart', handleEnterAnimationStart));
@@ -152,14 +157,16 @@ export function runEnterAnimation(
     // preventing an animation via selector specificity.
     ngZone.runOutsideAngular(() => {
       requestAnimationFrame(() => {
-        if (hasCompleted) return;
-        determineLongestAnimation(nativeElement, longestAnimations, areAnimationSupported);
-        if (!longestAnimations.has(nativeElement)) {
-          for (const klass of activeClasses) {
-            renderer.removeClass(nativeElement, klass);
+        setTimeout(() => {
+          if (hasCompleted || isCleanedUp) return;
+          determineLongestAnimation(nativeElement, longestAnimations, areAnimationSupported);
+          if (!longestAnimations.has(nativeElement)) {
+            for (const klass of activeClasses) {
+              renderer.removeClass(nativeElement, klass);
+            }
+            cleanupEnterClassData(nativeElement);
           }
-          cleanupEnterClassData(nativeElement);
-        }
+        });
       });
     });
   }
@@ -326,6 +333,14 @@ function animateLeaveClassRunner(
   const componentResolvers = getLViewLeaveAnimations(lView).get(tNode.index)?.resolvers;
   let fallbackTimeoutId: number | undefined;
   let hasCompleted = false;
+  let isCleanedUp = false;
+
+  cleanupFns.push(() => {
+    isCleanedUp = true;
+    if (fallbackTimeoutId !== undefined) {
+      clearTimeout(fallbackTimeoutId);
+    }
+  });
 
   const handleOutAnimationEnd = (event: AnimationEvent | TransitionEvent | CustomEvent) => {
     const target = getEventTarget(event as Event);
@@ -378,21 +393,22 @@ function animateLeaveClassRunner(
   // preventing an animation via selector specificity.
   ngZone.runOutsideAngular(() => {
     requestAnimationFrame(() => {
-      if (hasCompleted) return;
-      determineLongestAnimation(el, longestAnimations, areAnimationSupported);
-      const longest = longestAnimations.get(el);
-      if (!longest) {
-        clearLeavingNodes(tNode, el);
-        cleanupAfterLeaveAnimations(componentResolvers, cleanupFns);
-        clearLViewNodeAnimationResolvers(lView, tNode);
-      } else {
-        // Fallback cleanup if the browser drops the transitionend/animationend event
-        // entirely due to off-screen optimizations or rapid DOM teardown.
-        fallbackTimeoutId = setTimeout(() => {
-          handleOutAnimationEnd(new CustomEvent('animation-fallback'));
-        }, longest.duration + 50) as unknown as number;
-        cleanupFns.push(() => clearTimeout(fallbackTimeoutId));
-      }
+      setTimeout(() => {
+        if (hasCompleted || isCleanedUp) return;
+        determineLongestAnimation(el, longestAnimations, areAnimationSupported);
+        const longest = longestAnimations.get(el);
+        if (!longest) {
+          clearLeavingNodes(tNode, el);
+          cleanupAfterLeaveAnimations(componentResolvers, cleanupFns);
+          clearLViewNodeAnimationResolvers(lView, tNode);
+        } else {
+          // Fallback cleanup if the browser drops the transitionend/animationend event
+          // entirely due to off-screen optimizations or rapid DOM teardown.
+          fallbackTimeoutId = setTimeout(() => {
+            handleOutAnimationEnd(new CustomEvent('animation-fallback'));
+          }, longest.duration + 50) as unknown as number;
+        }
+      });
     });
   });
 }
diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts
@@ -31,7 +31,7 @@ import {
   ViewChild,
   ViewContainerRef,
 } from '@angular/core';
-import {fakeAsync, TestBed, tick} from '@angular/core/testing';
+import {fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {isNode} from '@angular/private/testing';
 import {tickAnimationFrames} from '../animation_utils/tick_animation_frames';
@@ -171,6 +171,68 @@ describe('Animation', () => {
       expect(cmp.el).toBeUndefined();
     }));
 
+    it('should fire the fallback timer and clean up if the animationend event is not dispatched', fakeAsync(() => {
+      @Component({
+        selector: 'test-cmp',
+        styles: styles,
+        template: '<div>@if (show()) {<p animate.leave="fade" #el>I should fade</p>}</div>',
+        encapsulation: ViewEncapsulation.None,
+      })
+      class TestComponent {
+        show = signal(true);
+        @ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
+      }
+      TestBed.configureTestingModule({animationsEnabled: true});
+
+      const fixture = TestBed.createComponent(TestComponent);
+      const cmp = fixture.componentInstance;
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.innerHTML).toContain('I should fade');
+      cmp.show.set(false);
+      fixture.detectChanges();
+
+      tickAnimationFrames(1);
+
+      // Now we step the tick so the macro-task setup timeout executes and queues our fallback.
+      tick(1);
+
+      // Wait for duration + 50ms buffer time for fallback
+      tick(10 + 50);
+
+      // Assert that cleanup occurred successfully without needing to artificially dispatch the event
+      expect(fixture.nativeElement.innerHTML).not.toContain('I should fade');
+      expect(cmp.el).toBeUndefined();
+    }));
+
+    it('should prevent leaking fallback timeout if view is destroyed early', fakeAsync(() => {
+      @Component({
+        selector: 'test-cmp',
+        styles: styles,
+        template: '<div>@if (show()) {<p animate.leave="fade" #el>I should fade</p>}</div>',
+        encapsulation: ViewEncapsulation.None,
+      })
+      class TestComponent {
+        show = signal(true);
+        @ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
+      }
+      TestBed.configureTestingModule({animationsEnabled: true});
+
+      const fixture = TestBed.createComponent(TestComponent);
+      const cmp = fixture.componentInstance;
+      fixture.detectChanges();
+
+      cmp.show.set(false);
+      fixture.detectChanges();
+
+      // We explicitly destroy the fixture right after triggering the leave,
+      // imitating the situation where the view goes away before the next macro-task.
+      fixture.destroy();
+
+      // Flush all tasks, expecting no errors with our cleanup flag guarding the timeout.
+      flush();
+    }));
+
     it('should support string arrays', fakeAsync(() => {
       const multiple = `
         .slide-out {
