refactor(animations): always remove elements right away unless they have a leave animation

modules/@angular/platform-browser/animations/src/render/dom_animation_engine.ts

@@ -69,8 +69,21 @@ export class DomAnimationEngine {
   }
 
   onRemove(element: any, domFn: () => any): void {
+    let lookupRef = this._elementTriggerStates.get(element);
+    if (lookupRef) {
+      const possibleTriggers = Object.keys(lookupRef);
+      const hasRemoval = possibleTriggers.some(triggerName => {
+        const oldValue = lookupRef[triggerName];
+        const instruction = this._triggers[triggerName].matchTransition(oldValue, 'void');
+        return !!instruction;
+      });
+      if (hasRemoval) {
         element[MARKED_FOR_REMOVAL] = true;
         this._queuedRemovals.set(element, domFn);
+        return;
+      }
+    }
+    domFn();
   }
 
   setProperty(element: any, property: string, value: any): void {

modules/@angular/platform-browser/animations/src/render/web_animations/web_animations_player.ts

@@ -90,7 +90,8 @@ export class WebAnimationsPlayer implements AnimationPlayer {
     }
 
     this._player = this._triggerWebAnimation(this.element, keyframes, this.options);
-    this._finalKeyframe = _copyKeyframeStyles(keyframes[keyframes.length - 1]);
+    this._finalKeyframe =
+        keyframes.length ? _copyKeyframeStyles(keyframes[keyframes.length - 1]) : {};
 
     // this is required so that the player doesn't start to animate right away
     this._resetDomPlayerState();

modules/@angular/platform-browser/animations/test/engine/dom_animation_engine_spec.ts

@@ -626,56 +626,6 @@ export function main() {
         expect(container.contains(child1)).toBe(true);
         expect(container.contains(child2)).toBe(true);
       });
-
-      it('should queue up all `remove` DOM operations until all animations are complete', () => {
-        let container = <any>el('<div></div>');
-        let targetContainer = <any>el('<div></div>');
-        let otherContainer = <any>el('<div></div>');
-        let child1 = <any>el('<div></div>');
-        let child2 = <any>el('<div></div>');
-        container.appendChild(targetContainer);
-        container.appendChild(otherContainer);
-        targetContainer.appendChild(child1);
-        targetContainer.appendChild(child2);
-
-        /*----------------*
-         container
-         /    \
-         target   other
-         / \
-         c1 c2
-         *----------------*/
-
-        expect(container.contains(otherContainer)).toBe(true);
-
-        const engine = makeEngine();
-        engine.onRemove(child1, () => targetContainer.removeChild(child1));
-        engine.onRemove(child2, () => targetContainer.removeChild(child2));
-        engine.onRemove(otherContainer, () => container.removeChild(otherContainer));
-
-        expect(container.contains(child1)).toBe(true);
-        expect(container.contains(child2)).toBe(true);
-        expect(container.contains(otherContainer)).toBe(true);
-
-        const instructions =
-            buildAnimationKeyframes([style({height: 0}), animate(1000, style({height: 100}))]);
-
-        const player = engine.animateTimeline(targetContainer, instructions);
-
-        expect(container.contains(child1)).toBe(true);
-        expect(container.contains(child2)).toBe(true);
-        expect(container.contains(otherContainer)).toBe(true);
-
-        engine.flush();
-        expect(container.contains(child1)).toBe(true);
-        expect(container.contains(child2)).toBe(true);
-        expect(container.contains(otherContainer)).toBe(false);
-
-        player.finish();
-        expect(container.contains(child1)).toBe(false);
-        expect(container.contains(child2)).toBe(false);
-        expect(container.contains(otherContainer)).toBe(false);
-      });
     });
   });
 }

modules/@angular/platform-browser/test/animation/animation_renderer_spec.ts

@@ -7,7 +7,7 @@
  */
 import {AnimationTriggerMetadata, animate, state, style, transition, trigger} from '@angular/animations';
 import {USE_VIEW_ENGINE} from '@angular/compiler/src/config';
-import {Component, Injectable, RendererFactoryV2, RendererTypeV2, ViewChild} from '@angular/core';
+import {AnimationPlayer, Component, Injectable, RendererFactoryV2, RendererTypeV2, ViewChild} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {BrowserAnimationModule, ɵAnimationEngine, ɵAnimationRendererFactory} from '@angular/platform-browser/animations';
 
@@ -200,6 +200,76 @@ export function main() {
              });
            });
          });
+
+      it('should only queue up dom removals if the element itself contains a valid leave animation',
+         () => {
+           @Component({
+             selector: 'my-cmp',
+             template: `
+               <div #elm1 *ngIf="exp1"></div>
+               <div #elm2 @animation1 *ngIf="exp2"></div>
+               <div #elm3 @animation2 *ngIf="exp3"></div>
+            `,
+             animations: [
+               trigger('animation1', [transition('a => b', [])]),
+               trigger('animation2', [transition(':leave', [])]),
+             ]
+           })
+           class Cmp {
+             exp1: any = true;
+             exp2: any = true;
+             exp3: any = true;
+
+             @ViewChild('elm1') public elm1: any;
+
+             @ViewChild('elm2') public elm2: any;
+
+             @ViewChild('elm3') public elm3: any;
+           }
+
+           TestBed.configureTestingModule({
+             providers: [{provide: ɵAnimationEngine, useClass: InjectableAnimationEngine}],
+             declarations: [Cmp]
+           });
+
+           const engine = TestBed.get(ɵAnimationEngine);
+           const fixture = TestBed.createComponent(Cmp);
+           const cmp = fixture.componentInstance;
+
+           fixture.detectChanges();
+           const elm1 = cmp.elm1;
+           const elm2 = cmp.elm2;
+           const elm3 = cmp.elm3;
+           assertHasParent(elm1);
+           assertHasParent(elm2);
+           assertHasParent(elm3);
+           engine.flush();
+           finishPlayers(engine.activePlayers);
+
+           cmp.exp1 = false;
+           fixture.detectChanges();
+           assertHasParent(elm1, false);
+           assertHasParent(elm2);
+           assertHasParent(elm3);
+           engine.flush();
+           expect(engine.activePlayers.length).toEqual(0);
+
+           cmp.exp2 = false;
+           fixture.detectChanges();
+           assertHasParent(elm1, false);
+           assertHasParent(elm2, false);
+           assertHasParent(elm3);
+           engine.flush();
+           expect(engine.activePlayers.length).toEqual(0);
+
+           cmp.exp3 = false;
+           fixture.detectChanges();
+           assertHasParent(elm1, false);
+           assertHasParent(elm2, false);
+           assertHasParent(elm3);
+           engine.flush();
+           expect(engine.activePlayers.length).toEqual(1);
+         });
     });
   });
 }
@@ -233,3 +303,17 @@ class MockAnimationEngine extends ɵAnimationEngine {
 
   flush() {}
 }
+
+
+function assertHasParent(element: any, yes: boolean = true) {
+  const parent = element.nativeElement.parentNode;
+  if (yes) {
+    expect(parent).toBeTruthy();
+  } else {
+    expect(parent).toBeFalsy();
+  }
+}
+
+function finishPlayers(players: AnimationPlayer[]) {
+  players.forEach(player => player.finish());
+}