fix(core): prevent early exit from leave animations when multiple transitions are present (angular#64225)

Our code ensuring host binding composition for animations was causing the early exit and removal of
elements when multiple transitions were present on the same element. This commit fixes the issue by
ensuring that we properly keep track of all the promise resolvers on the LView and then only
call them once we've properly waited for the longest animation to finish.

fixes: angular#64209

PR Close angular#64225

Author: thePunderWoman

packages/core/src/animation/interfaces.ts

@@ -97,12 +97,17 @@ export interface LongestAnimation {
   duration: number;
 }
 
+export interface NodeAnimations {
+  animateFns: Function[];
+  resolvers?: VoidFunction[];
+}
+
 export interface AnimationLViewData {
   // Enter animations that apply to nodes in this view
-  enter?: Map<number, Function[]>;
+  enter?: Map<number, NodeAnimations>;
 
   // Leave animations that apply to nodes in this view
-  leave?: Map<number, Function[]>;
+  leave?: Map<number, NodeAnimations>;
 
   // Leave animations that apply to nodes in this view
   // We chose to use unknown instead of PromiseSettledResult<void> to avoid requiring the type

packages/core/src/animation/utils.ts

@@ -7,7 +7,7 @@
  */
 
 import {stringify} from '../util/stringify'; // Adjust imports as per actual location
-import {ANIMATIONS_DISABLED, LongestAnimation} from './interfaces';
+import {ANIMATIONS_DISABLED, LongestAnimation, NodeAnimations} from './interfaces';
 import {INJECTOR, LView, DECLARATION_LCONTAINER, ANIMATIONS} from '../render3/interfaces/view';
 import {RuntimeError, RuntimeErrorCode} from '../errors';
 import {Renderer} from '../render3/interfaces/renderer';
@@ -162,17 +162,17 @@ export function trackLeavingNodes(tNode: TNode, el: HTMLElement): void {
 /**
  * Retrieves the list of specified enter animations from the lView
  */
-export function getLViewEnterAnimations(lView: LView): Map<number, Function[]> {
+export function getLViewEnterAnimations(lView: LView): Map<number, NodeAnimations> {
   const animationData = (lView[ANIMATIONS] ??= {});
-  return (animationData.enter ??= new Map<number, Function[]>());
+  return (animationData.enter ??= new Map<number, NodeAnimations>());
 }
 
 /**
  * Retrieves the list of specified leave animations from the lView
  */
-export function getLViewLeaveAnimations(lView: LView): Map<number, Function[]> {
+export function getLViewLeaveAnimations(lView: LView): Map<number, NodeAnimations> {
   const animationData = (lView[ANIMATIONS] ??= {});
-  return (animationData.leave ??= new Map<number, Function[]>());
+  return (animationData.leave ??= new Map<number, NodeAnimations>());
 }
 
 /**
@@ -253,11 +253,30 @@ export function isLongestAnimation(
  * @param fn The animation function to be called later
  */
 export function addAnimationToLView(
-  animations: Map<number, Function[]>,
+  animations: Map<number, NodeAnimations>,
   tNode: TNode,
   fn: Function,
 ) {
-  const animationFns = animations.get(tNode.index) ?? [];
-  animationFns.push(fn);
-  animations.set(tNode.index, animationFns);
+  const nodeAnimations = animations.get(tNode.index) ?? {animateFns: []};
+  nodeAnimations.animateFns.push(fn);
+  animations.set(tNode.index, nodeAnimations);
+}
+
+export function cleanupAfterLeaveAnimations(
+  resolvers: VoidFunction[] | undefined,
+  cleanupFns: Function[],
+): void {
+  if (resolvers) {
+    for (const fn of resolvers) {
+      fn();
+    }
+  }
+  for (const fn of cleanupFns) {
+    fn();
+  }
+}
+
+export function clearLViewNodeAnimationResolvers(lView: LView, tNode: TNode) {
+  const nodeAnimations = getLViewLeaveAnimations(lView).get(tNode.index);
+  if (nodeAnimations) nodeAnimations.resolvers = undefined;
 }

packages/core/src/render3/instructions/animation.ts

@@ -32,8 +32,10 @@ import {
   assertElementNodes,
   cancelAnimationsIfRunning,
   cancelLeavingNodes,
+  cleanupAfterLeaveAnimations,
   cleanupEnterClassData,
   clearLeavingNodes,
+  clearLViewNodeAnimationResolvers,
   enterClassMap,
   getClassListFromValue,
   getLViewEnterAnimations,
@@ -246,7 +248,11 @@ export function ɵɵanimateLeave(value: string | Function): typeof ɵɵanimateLe
   return ɵɵanimateLeave; // For chaining
 }
 
-function runLeaveAnimations(lView: LView, tNode: TNode, value: string | Function): Promise<void> {
+function runLeaveAnimations(
+  lView: LView,
+  tNode: TNode,
+  value: string | Function,
+): {promise: Promise<void>; resolve: VoidFunction} {
   const {promise, resolve} = promiseWithResolvers<void>();
   const nativeElement = getNativeByTNode(tNode, lView) as Element;
 
@@ -255,22 +261,23 @@ function runLeaveAnimations(lView: LView, tNode: TNode, value: string | Function
   const renderer = lView[RENDERER];
   const ngZone = lView[INJECTOR].get(NgZone);
   allLeavingAnimations.add(lView);
+  (getLViewLeaveAnimations(lView).get(tNode.index)!.resolvers ??= []).push(resolve);
 
   const activeClasses = getClassListFromValue(value);
   if (activeClasses && activeClasses.length > 0) {
     animateLeaveClassRunner(
       nativeElement as HTMLElement,
       tNode,
+      lView,
       activeClasses,
       renderer,
       ngZone,
-      resolve,
     );
   } else {
     resolve();
   }
 
-  return promise;
+  return {promise, resolve};
 }
 
 /**
@@ -280,13 +287,14 @@ function runLeaveAnimations(lView: LView, tNode: TNode, value: string | Function
 function animateLeaveClassRunner(
   el: HTMLElement,
   tNode: TNode,
+  lView: LView,
   classList: string[],
   renderer: Renderer,
   ngZone: NgZone,
-  resolver: VoidFunction,
 ) {
   cancelAnimationsIfRunning(el, renderer);
   const cleanupFns: Function[] = [];
+  const resolvers = getLViewLeaveAnimations(lView).get(tNode.index)?.resolvers;
 
   const handleOutAnimationEnd = (event: AnimationEvent | TransitionEvent | CustomEvent) => {
     // this early exit case is to prevent issues with bubbling events that are from child element animations
@@ -308,36 +316,29 @@ function animateLeaveClassRunner(
           renderer.removeClass(el, item);
         }
       }
-    }
-    resolver();
-    for (const fn of cleanupFns) {
-      fn();
+      cleanupAfterLeaveAnimations(resolvers, cleanupFns);
+      clearLViewNodeAnimationResolvers(lView, tNode);
     }
   };
 
   ngZone.runOutsideAngular(() => {
     cleanupFns.push(renderer.listen(el, 'animationend', handleOutAnimationEnd));
     cleanupFns.push(renderer.listen(el, 'transitionend', handleOutAnimationEnd));
   });
-
   trackLeavingNodes(tNode, el);
-
   for (const item of classList) {
     renderer.addClass(el, item);
   }
-
+  // In the case that the classes added have no animations, we need to remove
+  // the element right away. This could happen because someone is intentionally
+  // preventing an animation via selector specificity.
   ngZone.runOutsideAngular(() => {
     requestAnimationFrame(() => {
-      // In the case that the classes added have no animations, we need to remove
-      // the element right away. This could happen because someone is intentionally
-      // preventing an animation via selector specificity.
       determineLongestAnimation(el, longestAnimations, areAnimationSupported);
       if (!longestAnimations.has(el)) {
         clearLeavingNodes(tNode, el);
-        resolver();
-        for (const fn of cleanupFns) {
-          fn();
-        }
+        cleanupAfterLeaveAnimations(resolvers, cleanupFns);
+        clearLViewNodeAnimationResolvers(lView, tNode);
       }
     });
   });
@@ -438,8 +439,8 @@ function queueEnterAnimations(lView: LView) {
   const enterAnimations = lView[ANIMATIONS]?.enter;
   if (enterAnimations) {
     const animationQueue = lView[INJECTOR].get(ANIMATION_QUEUE);
-    for (const [_, animateFns] of enterAnimations) {
-      for (const animateFn of animateFns) {
+    for (const [_, nodeAnimations] of enterAnimations) {
+      for (const animateFn of nodeAnimations.animateFns) {
         animationQueue.queue.add(animateFn);
       }
     }

packages/core/src/render3/node_manipulation.ts

@@ -111,7 +111,7 @@ function maybeQueueEnterAnimation(
   const enterAnimations = parentLView?.[ANIMATIONS]?.enter;
   if (parent !== null && enterAnimations && enterAnimations.has(tNode.index)) {
     const animationQueue = injector.get(ANIMATION_QUEUE);
-    for (const animateFn of enterAnimations.get(tNode.index)!) {
+    for (const animateFn of enterAnimations.get(tNode.index)!.animateFns) {
       animationQueue.queue.add(animateFn);
     }
   }
@@ -416,9 +416,10 @@ function runLeaveAnimationsWithCallback(
       const leaveAnimations = leaveAnimationMap.get(tNode.index);
       const runningAnimations = [];
       if (leaveAnimations) {
-        for (let index = 0; index < leaveAnimations.length; index++) {
-          const animationFn = leaveAnimations[index];
-          runningAnimations.push(animationFn());
+        for (let index = 0; index < leaveAnimations.animateFns.length; index++) {
+          const animationFn = leaveAnimations.animateFns[index];
+          const {promise} = animationFn();
+          runningAnimations.push(promise);
         }
       }
       animations.running = Promise.allSettled(runningAnimations);

packages/core/test/acceptance/animation_spec.ts

@@ -714,6 +714,63 @@ describe('Animation', () => {
       expect(fixture.debugElement.query(By.css('child-cmp'))).toBeNull();
     }));
 
+    it('should await the longest animation when multiple transitions are present', fakeAsync(() => {
+      const multiple = `
+        .slide-out {
+          grid-template-rows: 0fr;
+          overflow: hidden;
+          opacity: 0;
+          translate: 0 -60px;
+          margin-top: -16px;
+          transition: grid-template-rows 1s ease 400ms, margin-top 1s ease 400ms,
+            opacity 400ms ease, translate 400ms ease;
+        }
+      `;
+      @Component({
+        selector: 'test-cmp',
+        styles: multiple,
+        template: '@if (show()) { <div animate.leave="slide-out"><p>Element with text</p></div> }',
+        encapsulation: ViewEncapsulation.None,
+      })
+      class TestComponent {
+        show = signal(true);
+      }
+      TestBed.configureTestingModule({animationsEnabled: true});
+
+      const fixture = TestBed.createComponent(TestComponent);
+      const cmp = fixture.componentInstance;
+      fixture.detectChanges();
+      const div = fixture.debugElement.query(By.css('div'));
+
+      expect(div.nativeElement.className).not.toContain('slide-out');
+      cmp.show.set(false);
+      fixture.detectChanges();
+      tickAnimationFrames(1);
+      expect(cmp.show()).toBeFalsy();
+      fixture.detectChanges();
+      expect(div.nativeElement.className).toContain('slide-out');
+      fixture.detectChanges();
+
+      div.nativeElement.dispatchEvent(
+        new TransitionEvent('transitionend', {propertyName: 'opacity'}),
+      );
+      div.nativeElement.dispatchEvent(
+        new TransitionEvent('transitionend', {propertyName: 'translate'}),
+      );
+
+      expect(fixture.nativeElement.outerHTML).toContain('slide-out');
+
+      div.nativeElement.dispatchEvent(
+        new TransitionEvent('transitionend', {propertyName: 'margin-top'}),
+      );
+      div.nativeElement.dispatchEvent(
+        new TransitionEvent('transitionend', {propertyName: 'grid-template-rows'}),
+      );
+      tick();
+      expect(fixture.nativeElement.outerHTML).not.toContain('slide-out');
+      expect(fixture.debugElement.query(By.css('div'))).toBeNull();
+    }));
+
     describe('legacy animations compatibility', () => {
       beforeAll(() => {
         TestBed.resetTestEnvironment();

packages/core/test/bundling/defer/bundle.golden_symbols.json

@@ -58,6 +58,7 @@
       "shimStylesContent"
     ],
     "lazy": [
+      "DeferComponent",
       "AFTER_RENDER_SEQUENCES_TO_ADD",
       "ANIMATIONS",
       "ANIMATION_QUEUE",
@@ -746,8 +747,7 @@
       "wasLastNodeCreated",
       "writeDirectClass",
       "writeDirectStyle",
-      "writeToDirectiveInput",
-      "DeferComponent"
+      "writeToDirectiveInput"
     ]
   }
 }