fix(ivy): ensure that super/sub classes can both update an animation binding in harmony

matsko · matsko · commit ab6b89a12e27 · 2019-04-21T20:16:37.000-07:00
In ivy, because metadata is inherited, this may cause super and sub host bindings on
two components to update an animation binding in parallel. This patch ensures that
the animation transition code can handle this.

Jira-Issue: FW-1220
diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts
@@ -183,8 +183,9 @@ export class AnimationTransitionNamespace {
     return trigger;
   }
 
-  trigger(element: any, triggerName: string, value: any, defaultToFallback: boolean = true):
-      TransitionAnimationPlayer|undefined {
+  trigger(
+      element: any, triggerName: string, value: any, defaultToFallback: boolean = true,
+      isFinal: boolean = false): TransitionAnimationPlayer|undefined {
     const trigger = this._getTrigger(triggerName);
     const player = new TransitionAnimationPlayer(this.id, triggerName, element);
 
@@ -203,21 +204,17 @@ export class AnimationTransitionNamespace {
       toState.absorbOptions(fromState.options);
     }
 
-    triggersWithStates[triggerName] = toState;
-
     if (!fromState) {
       fromState = DEFAULT_STATE_VALUE;
     }
 
-    const isRemoval = toState.value === VOID_VALUE;
-
     // normally this isn't reached by here, however, if an object expression
     // is passed in then it may be a new object each time. Comparing the value
     // is important since that will stay the same despite there being a new object.
     // The removal arc here is special cased because the same element is triggered
     // twice in the event that it contains animations on the outer/inner portions
     // of the host container
-    if (!isRemoval && fromState.value === toState.value) {
+    if (fromState.value === toState.value) {
       // this means that despite the value not changing, some inner params
       // have changed which means that the animation final styles need to be applied
       if (!objEquals(fromState.params, toState.params)) {
@@ -236,6 +233,10 @@ export class AnimationTransitionNamespace {
       return;
     }
 
+    if (!isFinal) {
+      triggersWithStates[triggerName] = toState;
+    }
+
     const playersOnElement: TransitionAnimationPlayer[] =
         getOrSetAsInMap(this._engine.playersByElement, element, []);
     playersOnElement.forEach(player => {
@@ -336,7 +337,7 @@ export class AnimationTransitionNamespace {
         // this check is here in the event that an element is removed
         // twice (both on the host level and the component level)
         if (this._triggers[triggerName]) {
-          const player = this.trigger(element, triggerName, VOID_VALUE, defaultToFallback);
+          const player = this.trigger(element, triggerName, VOID_VALUE, defaultToFallback, true);
           if (player) {
             players.push(player);
           }
diff --git a/packages/core/test/acceptance/BUILD.bazel b/packages/core/test/acceptance/BUILD.bazel
@@ -9,13 +9,15 @@ ts_library(
         ["**/*.ts"],
     ),
     deps = [
+        "//packages/animations",
         "//packages/common",
         "//packages/compiler",
         "//packages/compiler/testing",
         "//packages/core",
         "//packages/core/testing",
         "//packages/platform-browser",
         "//packages/platform-browser-dynamic",
+        "//packages/platform-browser/animations",
         "//packages/platform-browser/testing",
         "//packages/private/testing",
         "@npm//zone.js",
diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts
@@ -0,0 +1,115 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import {AnimationEvent, animate, style, transition, trigger} from '@angular/animations';
+import {Component, ViewChild} from '@angular/core';
+import {TestBed, fakeAsync, flushMicrotasks} from '@angular/core/testing';
+import {expect} from '@angular/platform-browser/testing/src/matchers';
+import {onlyInIvy} from '@angular/private/testing';
+
+describe('acceptance integration tests', () => {
+  onlyInIvy('animation and host metadata is inherited only in ivy')
+      .fit(
+          'animation trigger events should only fire once in both super class and sub class components',
+          fakeAsync(() => {
+            const sharedAnimations = [
+              trigger(
+                  'foo',
+                  [
+                    transition(
+                        ':increment',
+                        [
+                          style({opacity: 0}),
+                          animate('1s', style({opacity: 1})),
+                        ]),
+                  ]),
+            ];
+
+            @Component({
+              selector: 'super-comp',
+              animations: sharedAnimations,
+              template: '...',
+              host: {
+                '[@foo]': 'foo1Value',
+                '(@foo.start)': 'foo1Start($event)',
+                '(@foo.done)': 'foo1Done($event)',
+              }
+            })
+            class SuperClassComp {
+              foo1Value = 0;
+              log1: any[] = [];
+
+              foo1Start(event: AnimationEvent) { this.log1.push('start-parent', event.toState); }
+
+              foo1Done(event: AnimationEvent) { this.log1.push('done-parent', event.toState); }
+
+              fire() { this.foo1Value++; }
+            }
+
+            @Component({
+              selector: 'sub-comp',
+              animations: sharedAnimations,
+              template: '...',
+              host: {
+                '[@foo]': 'foo2Value',
+                '(@foo.start)': 'foo2Start($event)',
+                '(@foo.done)': 'foo2Done($event)',
+              }
+            })
+            class SubClassComp extends SuperClassComp {
+              foo2Value = 0;
+              log2: any[] = [];
+
+              foo2Start(event: AnimationEvent) { this.log2.push('start-child', event.toState); }
+
+              foo2Done(event: AnimationEvent) { this.log2.push('done-child', event.toState); }
+
+              fire() {
+                super.fire();
+                this.foo2Value++;
+              }
+            }
+
+            @Component({
+              selector: 'app',
+              template: `
+            <sub-comp #sub></sub-comp>
+          `
+            })
+            class AppComp {
+              @ViewChild('sub')
+              subComp: SubClassComp|null = null;
+            }
+
+            TestBed.configureTestingModule({declarations: [SubClassComp, SuperClassComp, AppComp]});
+            const fixture = TestBed.createComponent(AppComp);
+            fixture.detectChanges();
+            flushMicrotasks();
+
+            const comp = fixture.componentInstance.subComp !;
+            comp.log1.length = 0;
+            comp.log2.length = 0;
+
+            comp.fire();
+            fixture.detectChanges();
+            flushMicrotasks();
+
+            expect(comp.log1).toEqual([
+              'start-parent',
+              1,
+              'done-parent',
+              1,
+            ]);
+
+            expect(comp.log2).toEqual([
+              'start-child',
+              1,
+              'done-child',
+              1,
+            ]);
+          }));
+});
