From 697f6a55a57461f57d4b596d2746130a512294aa Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Thu, 9 Jan 2020 18:35:53 +0100
Subject: [PATCH] fix(animations): not waiting for child animations to finish
 when removing parent in Ivy (#34702)

In #28162 we introduced an extra `removeNode` call for host elements which can cause the parent element to be removed before all child animations have finished. The issue is only in Ivy, because that the only place where we pass in the `isHostElement` flag. These changes fix the issue by not re-triggering the removal logic if the element has in-progress animations.

Fixes #33597.

PR Close #34702
---
 .../src/render/transition_animation_engine.ts | 13 +++--
 .../animation/animation_integration_spec.ts   | 47 +++++++++++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts
index b3ec5bc052..f3ef0f80bc 100644
--- a/packages/animations/browser/src/render/transition_animation_engine.ts
+++ b/packages/animations/browser/src/render/transition_animation_engine.ts
@@ -437,11 +437,14 @@ export class AnimationTransitionNamespace {
     if (containsPotentialParentTransition) {
       engine.markElementAsRemoved(this.id, element, false, context);
     } else {
-      // we do this after the flush has occurred such
-      // that the callbacks can be fired
-      engine.afterFlush(() => this.clearElementCache(element));
-      engine.destroyInnerAnimations(element);
-      engine._onRemovalComplete(element, context);
+      const removalFlag = element[REMOVAL_FLAG];
+      if (!removalFlag || removalFlag === NULL_REMOVAL_STATE) {
+        // we do this after the flush has occurred such
+        // that the callbacks can be fired
+        engine.afterFlush(() => this.clearElementCache(element));
+        engine.destroyInnerAnimations(element);
+        engine._onRemovalComplete(element, context);
+      }
     }
   }
 
diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts
index 4087eff1f4..d2034fb6a2 100644
--- a/packages/core/test/animation/animation_integration_spec.ts
+++ b/packages/core/test/animation/animation_integration_spec.ts
@@ -932,6 +932,53 @@ const DEFAULT_COMPONENT_ID = '1';
              expect(fixture.debugElement.nativeElement.children.length).toBe(0);
            }));
 
+        it('should wait for child animations before removing parent', fakeAsync(() => {
+             @Component({
+               template: '<child-cmp *ngIf="exp" @parentTrigger></child-cmp>',
+               animations: [trigger(
+                   'parentTrigger', [transition(':leave', [group([query('@*', animateChild())])])])]
+             })
+             class ParentCmp {
+               exp = true;
+             }
+
+             @Component({
+               selector: 'child-cmp',
+               template: '<p @childTrigger>Hello there</p>',
+               animations: [trigger(
+                   'childTrigger',
+                   [transition(
+                       ':leave', [style({opacity: 1}), animate('200ms', style({opacity: 0}))])])]
+             })
+             class ChildCmp {
+             }
+
+             TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]});
+             const engine = TestBed.inject(ɵAnimationEngine);
+             const fixture = TestBed.createComponent(ParentCmp);
+
+             fixture.detectChanges();
+             engine.flush();
+             expect(getLog().length).toBe(0);
+
+             fixture.componentInstance.exp = false;
+             fixture.detectChanges();
+             expect(fixture.nativeElement.children.length).toBe(1);
+
+             engine.flush();
+             expect(getLog().length).toBe(1);
+
+             const player = getLog()[0];
+             expect(player.keyframes).toEqual([
+               {opacity: '1', offset: 0},
+               {opacity: '0', offset: 1},
+             ]);
+
+             player.finish();
+             flushMicrotasks();
+             expect(fixture.nativeElement.children.length).toBe(0);
+           }));
+
         // animationRenderer => nonAnimationRenderer
         it('should trigger a leave animation when the outer components element binding updates on the host component element',
            fakeAsync(() => {