fix(animations): do not crash animations if a nested component fires …

…CD during CD Closes angular#18193
matsko · Jul 18, 2017 · 4e5a0b3 · 4e5a0b3
1 parent b399cb2
commit 4e5a0b3
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 7 deletions.
diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts
@@ -8,7 +8,7 @@
 import {AUTO_STYLE, AnimationEvent, AnimationOptions, animate, animateChild, group, keyframes, query, state, style, transition, trigger} from '@angular/animations';
 import {AnimationDriver, ɵAnimationEngine, ɵNoopAnimationDriver} from '@angular/animations/browser';
 import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/browser/testing';
-import {Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core';
+import {ChangeDetectorRef, Component, HostBinding, HostListener, RendererFactory2, ViewChild} from '@angular/core';
 import {ɵDomRendererFactory2} from '@angular/platform-browser';
 import {BrowserAnimationsModule} from '@angular/platform-browser/animations';
 import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
@@ -1413,6 +1413,65 @@ export function main() {
         ]);
       });
 
+      it('should not flush animations twice when an inner component runs change detection', () => {
+        @Component({
+          selector: 'outer-cmp',
+          template: `
+            <div *ngIf="exp" @outer></div>
+            <inner-cmp #inner></inner-cmp>
+          `,
+          animations: [trigger(
+              'outer',
+              [transition(':enter', [style({opacity: 0}), animate('1s', style({opacity: 1}))])])]
+        })
+        class OuterCmp {
+          @ViewChild('inner') public inner: any;
+          public exp: any = null;
+
+          update() { this.exp = 'go'; }
+
+          ngDoCheck() {
+            if (this.exp == 'go') {
+              this.inner.update();
+            }
+          }
+        }
+
+        @Component({
+          selector: 'inner-cmp',
+          template: `
+            <div *ngIf="exp" @inner></div>
+          `,
+          animations: [trigger('inner', [transition(
+                                            ':enter',
+                                            [
+                                              style({opacity: 0}),
+                                              animate('1s', style({opacity: 1})),
+                                            ])])]
+        })
+        class InnerCmp {
+          public exp: any;
+          constructor(private _ref: ChangeDetectorRef) {}
+          update() {
+            this.exp = 'go';
+            this._ref.detectChanges();
+          }
+        }
+
+        TestBed.configureTestingModule({declarations: [OuterCmp, InnerCmp]});
+
+        const engine = TestBed.get(ɵAnimationEngine);
+        const fixture = TestBed.createComponent(OuterCmp);
+        const cmp = fixture.componentInstance;
+        fixture.detectChanges();
+        expect(getLog()).toEqual([]);
+
+        cmp.update();
+        fixture.detectChanges();
+
+        const players = getLog();
+        expect(players.length).toEqual(2);
+      });
     });
 
     describe('animation listeners', () => {
@@ -1815,12 +1874,12 @@ export function main() {
           selector: 'my-cmp',
           template: `
               <div class="parent" [@parent]="exp" (@parent.done)="cb('all','done', $event)">
-                <div *ngFor="let item of items" 
+                <div *ngFor="let item of items"
                      class="item item-{{ item }}"
                      @child
                      (@child.start)="cb('c-' + item, 'start', $event)"
                      (@child.done)="cb('c-' + item, 'done', $event)">
-                  {{ item }} 
+                  {{ item }}
                 </div>
               </div>
             `,

diff --git a/packages/platform-browser/animations/src/animation_renderer.ts b/packages/platform-browser/animations/src/animation_renderer.ts
@@ -15,6 +15,7 @@ export class AnimationRendererFactory implements RendererFactory2 {
   private _microtaskId: number = 1;
   private _animationCallbacksBuffer: [(e: any) => any, any][] = [];
   private _rendererCache = new Map<Renderer2, BaseAnimationRenderer>();
+  private _cdRecurDepth = 0;
 
   constructor(
       private delegate: RendererFactory2, private engine: AnimationEngine, private _zone: NgZone) {
@@ -58,6 +59,7 @@ export class AnimationRendererFactory implements RendererFactory2 {
   }
 
   begin() {
+    this._cdRecurDepth++;
     if (this.delegate.begin) {
       this.delegate.begin();
     }
@@ -90,10 +92,16 @@ export class AnimationRendererFactory implements RendererFactory2 {
   }
 
   end() {
-    this._zone.runOutsideAngular(() => {
-      this._scheduleCountTask();
-      this.engine.flush(this._microtaskId);
-    });
+    this._cdRecurDepth--;
+
+    // this is to prevent animations from running twice when an inner
+    // component does CD when a parent component insted has inserted it
+    if (this._cdRecurDepth == 0) {
+      this._zone.runOutsideAngular(() => {
+        this._scheduleCountTask();
+        this.engine.flush(this._microtaskId);
+      });
+    }
     if (this.delegate.end) {
       this.delegate.end();
     }