Skip to content

Commit 33c8948

Browse files
matskoIgorMinar
authored andcommitted
refactor(animations): ensure animation data-structures are created only when used
Closes angular#12250
1 parent 606e518 commit 33c8948

File tree

7 files changed

+129
-87
lines changed

7 files changed

+129
-87
lines changed

modules/@angular/compiler/src/animation/animation_compiler.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export class AnimationCompiler {
3232
var _ANIMATION_FACTORY_ELEMENT_VAR = o.variable('element');
3333
var _ANIMATION_DEFAULT_STATE_VAR = o.variable('defaultStateStyles');
3434
var _ANIMATION_FACTORY_VIEW_VAR = o.variable('view');
35+
var _ANIMATION_FACTORY_VIEW_CONTEXT = _ANIMATION_FACTORY_VIEW_VAR.prop('animationContext');
3536
var _ANIMATION_FACTORY_RENDERER_VAR = _ANIMATION_FACTORY_VIEW_VAR.prop('renderer');
3637
var _ANIMATION_CURRENT_STATE_VAR = o.variable('currentState');
3738
var _ANIMATION_NEXT_STATE_VAR = o.variable('nextState');
@@ -186,7 +187,7 @@ class _AnimationBuilder implements AnimationAstVisitor {
186187
context.stateMap.registerState(DEFAULT_STATE, {});
187188

188189
var statements: o.Statement[] = [];
189-
statements.push(_ANIMATION_FACTORY_VIEW_VAR
190+
statements.push(_ANIMATION_FACTORY_VIEW_CONTEXT
190191
.callMethod(
191192
'cancelActiveAnimation',
192193
[
@@ -263,13 +264,20 @@ class _AnimationBuilder implements AnimationAstVisitor {
263264
.toStmt()])])
264265
.toStmt());
265266

266-
statements.push(_ANIMATION_FACTORY_VIEW_VAR
267+
var transitionParams = o.literalMap([
268+
['toState', _ANIMATION_NEXT_STATE_VAR], ['fromState', _ANIMATION_CURRENT_STATE_VAR],
269+
['totalTime', _ANIMATION_TIME_VAR]
270+
]);
271+
272+
var transitionEvent = o.importExpr(resolveIdentifier(Identifiers.AnimationTransitionEvent))
273+
.instantiate([transitionParams]);
274+
275+
statements.push(_ANIMATION_FACTORY_VIEW_CONTEXT
267276
.callMethod(
268277
'queueAnimation',
269278
[
270279
_ANIMATION_FACTORY_ELEMENT_VAR, o.literal(this.animationName),
271-
_ANIMATION_PLAYER_VAR, _ANIMATION_TIME_VAR,
272-
_ANIMATION_CURRENT_STATE_VAR, _ANIMATION_NEXT_STATE_VAR
280+
_ANIMATION_PLAYER_VAR, transitionEvent
273281
])
274282
.toStmt());
275283

modules/@angular/compiler/src/identifiers.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ANALYZE_FOR_ENTRY_COMPONENTS, ChangeDetectionStrategy, ChangeDetectorRef, ComponentFactory, ComponentFactoryResolver, ElementRef, Injector, LOCALE_ID as LOCALE_ID_, NgModuleFactory, QueryList, RenderComponentType, Renderer, SecurityContext, SimpleChange, TRANSLATIONS_FORMAT as TRANSLATIONS_FORMAT_, TemplateRef, ViewContainerRef, ViewEncapsulation} from '@angular/core';
9+
import {ANALYZE_FOR_ENTRY_COMPONENTS, AnimationTransitionEvent, ChangeDetectionStrategy, ChangeDetectorRef, ComponentFactory, ComponentFactoryResolver, ElementRef, Injector, LOCALE_ID as LOCALE_ID_, NgModuleFactory, QueryList, RenderComponentType, Renderer, SecurityContext, SimpleChange, TRANSLATIONS_FORMAT as TRANSLATIONS_FORMAT_, TemplateRef, ViewContainerRef, ViewEncapsulation} from '@angular/core';
1010

1111
import {CompileIdentifierMetadata, CompileTokenMetadata} from './compile_metadata';
1212
import {AnimationGroupPlayer, AnimationKeyframe, AnimationSequencePlayer, AnimationStyles, AppElement, AppView, ChangeDetectorStatus, CodegenComponentFactoryResolver, DebugAppView, DebugContext, EMPTY_ARRAY, EMPTY_MAP, NgModuleInjector, NoOpAnimationPlayer, StaticNodeDebugInfo, TemplateRef_, UNINITIALIZED, ValueUnwrapper, ViewType, ViewUtils, balanceAnimationKeyframes, castByValue, checkBinding, clearStyles, collectAndResolveStyles, devModeEqual, flattenNestedViewRenderNodes, interpolate, prepareFinalAnimationStyles, pureProxy1, pureProxy10, pureProxy2, pureProxy3, pureProxy4, pureProxy5, pureProxy6, pureProxy7, pureProxy8, pureProxy9, reflector, registerModuleFactory, renderStyles} from './private_import_core';
@@ -266,6 +266,11 @@ export class Identifiers {
266266
moduleUrl: assetUrl('core', 'i18n/tokens'),
267267
runtime: TRANSLATIONS_FORMAT_
268268
};
269+
static AnimationTransitionEvent: IdentifierSpec = {
270+
name: 'AnimationTransitionEvent',
271+
moduleUrl: assetUrl('core', 'animation/animation_transition_event'),
272+
runtime: AnimationTransitionEvent
273+
};
269274
}
270275

271276
export function resolveIdentifier(identifier: IdentifierSpec) {

modules/@angular/compiler/src/view_compiler/event_binder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,9 @@ export class CompileEventListener {
119119
[o.THIS_EXPR.prop(this._methodName).callMethod(o.BuiltinMethod.Bind, [o.THIS_EXPR])]);
120120

121121
// tie the property callback method to the view animations map
122-
var stmt = o.THIS_EXPR
122+
var stmt = o.THIS_EXPR.prop('animationContext')
123123
.callMethod(
124-
'registerAnimationOutput',
124+
'registerOutputHandler',
125125
[
126126
this.compileElement.renderNode, o.literal(this.eventName),
127127
o.literal(this.eventPhase), outputListener

modules/@angular/core/src/animation/view_animation_map.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ export class ViewAnimationMap {
1515
private _map = new Map<any, {[key: string]: AnimationPlayer}>();
1616
private _allPlayers: AnimationPlayer[] = [];
1717

18-
get length(): number { return this.getAllPlayers().length; }
19-
2018
find(element: any, animationName: string): AnimationPlayer {
2119
var playersByAnimation = this._map.get(element);
2220
if (isPresent(playersByAnimation)) {
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
import {AnimationGroupPlayer} from '../animation/animation_group_player';
9+
import {AnimationPlayer} from '../animation/animation_player';
10+
import {queueAnimation as queueAnimationGlobally} from '../animation/animation_queue';
11+
import {AnimationTransitionEvent} from '../animation/animation_transition_event';
12+
import {ViewAnimationMap} from '../animation/view_animation_map';
13+
14+
export class AnimationViewContext {
15+
private _players = new ViewAnimationMap();
16+
private _listeners = new Map<any, _AnimationOutputHandler[]>();
17+
18+
onAllActiveAnimationsDone(callback: () => any): void {
19+
var activeAnimationPlayers = this._players.getAllPlayers();
20+
// we check for the length to avoid having GroupAnimationPlayer
21+
// issue an unnecessary microtask when zero players are passed in
22+
if (activeAnimationPlayers.length) {
23+
new AnimationGroupPlayer(activeAnimationPlayers).onDone(() => callback());
24+
} else {
25+
callback();
26+
}
27+
}
28+
29+
queueAnimation(
30+
element: any, animationName: string, player: AnimationPlayer,
31+
event: AnimationTransitionEvent): void {
32+
queueAnimationGlobally(player);
33+
34+
this._players.set(element, animationName, player);
35+
player.onDone(() => {
36+
// TODO: add codegen to remove the need to store these values
37+
this._triggerOutputHandler(element, animationName, 'done', event);
38+
this._players.remove(element, animationName);
39+
});
40+
41+
player.onStart(() => this._triggerOutputHandler(element, animationName, 'start', event));
42+
}
43+
44+
cancelActiveAnimation(element: any, animationName: string, removeAllAnimations: boolean = false):
45+
void {
46+
if (removeAllAnimations) {
47+
this._players.findAllPlayersByElement(element).forEach(player => player.destroy());
48+
} else {
49+
var player = this._players.find(element, animationName);
50+
if (player) {
51+
player.destroy();
52+
}
53+
}
54+
}
55+
56+
registerOutputHandler(
57+
element: any, eventName: string, eventPhase: string, eventHandler: Function): void {
58+
var animations = this._listeners.get(element);
59+
if (!animations) {
60+
this._listeners.set(element, animations = []);
61+
}
62+
animations.push(new _AnimationOutputHandler(eventName, eventPhase, eventHandler));
63+
}
64+
65+
private _triggerOutputHandler(
66+
element: any, animationName: string, phase: string, event: AnimationTransitionEvent): void {
67+
const listeners = this._listeners.get(element);
68+
if (listeners && listeners.length) {
69+
for (let i = 0; i < listeners.length; i++) {
70+
let listener = listeners[i];
71+
// we check for both the name in addition to the phase in the event
72+
// that there may be more than one @trigger on the same element
73+
if (listener.eventName === animationName && listener.eventPhase === phase) {
74+
listener.handler(event);
75+
break;
76+
}
77+
}
78+
}
79+
}
80+
}
81+
82+
class _AnimationOutputHandler {
83+
constructor(public eventName: string, public eventPhase: string, public handler: Function) {}
84+
}

modules/@angular/core/src/linker/view.ts

Lines changed: 15 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,14 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AnimationGroupPlayer} from '../animation/animation_group_player';
10-
import {AnimationPlayer} from '../animation/animation_player';
11-
import {queueAnimation} from '../animation/animation_queue';
12-
import {AnimationTransitionEvent} from '../animation/animation_transition_event';
13-
import {ViewAnimationMap} from '../animation/view_animation_map';
149
import {ChangeDetectorRef, ChangeDetectorStatus} from '../change_detection/change_detection';
1510
import {Injector} from '../di/injector';
1611
import {ListWrapper} from '../facade/collection';
1712
import {isPresent} from '../facade/lang';
1813
import {WtfScopeFn, wtfCreateScope, wtfLeave} from '../profile/profile';
1914
import {RenderComponentType, RenderDebugInfo, Renderer} from '../render/api';
2015

16+
import {AnimationViewContext} from './animation_view_context';
2117
import {DebugContext, StaticNodeDebugInfo} from './debug_context';
2218
import {AppElement} from './element';
2319
import {ElementInjector} from './element_injector';
@@ -49,10 +45,7 @@ export abstract class AppView<T> {
4945
renderer: Renderer;
5046

5147
private _hasExternalHostElement: boolean;
52-
53-
public animationPlayers = new ViewAnimationMap();
54-
55-
private _animationListeners = new Map<any, _AnimationOutputHandler[]>();
48+
private _animationContext: AnimationViewContext;
5649

5750
public context: T;
5851

@@ -68,60 +61,14 @@ export abstract class AppView<T> {
6861
}
6962
}
7063

71-
get destroyed(): boolean { return this.cdMode === ChangeDetectorStatus.Destroyed; }
72-
73-
cancelActiveAnimation(element: any, animationName: string, removeAllAnimations: boolean = false) {
74-
if (removeAllAnimations) {
75-
this.animationPlayers.findAllPlayersByElement(element).forEach(player => player.destroy());
76-
} else {
77-
var player = this.animationPlayers.find(element, animationName);
78-
if (isPresent(player)) {
79-
player.destroy();
80-
}
81-
}
82-
}
83-
84-
queueAnimation(
85-
element: any, animationName: string, player: AnimationPlayer, totalTime: number,
86-
fromState: string, toState: string): void {
87-
queueAnimation(player);
88-
var event = new AnimationTransitionEvent(
89-
{'fromState': fromState, 'toState': toState, 'totalTime': totalTime});
90-
this.animationPlayers.set(element, animationName, player);
91-
92-
player.onDone(() => {
93-
// TODO: make this into a datastructure for done|start
94-
this.triggerAnimationOutput(element, animationName, 'done', event);
95-
this.animationPlayers.remove(element, animationName);
96-
});
97-
98-
player.onStart(() => { this.triggerAnimationOutput(element, animationName, 'start', event); });
99-
}
100-
101-
triggerAnimationOutput(
102-
element: any, animationName: string, phase: string, event: AnimationTransitionEvent) {
103-
var listeners = this._animationListeners.get(element);
104-
if (isPresent(listeners) && listeners.length) {
105-
for (let i = 0; i < listeners.length; i++) {
106-
let listener = listeners[i];
107-
// we check for both the name in addition to the phase in the event
108-
// that there may be more than one @trigger on the same element
109-
if (listener.eventName === animationName && listener.eventPhase === phase) {
110-
listener.handler(event);
111-
break;
112-
}
113-
}
64+
get animationContext(): AnimationViewContext {
65+
if (!this._animationContext) {
66+
this._animationContext = new AnimationViewContext();
11467
}
68+
return this._animationContext;
11569
}
11670

117-
registerAnimationOutput(
118-
element: any, eventName: string, eventPhase: string, eventHandler: Function): void {
119-
var animations = this._animationListeners.get(element);
120-
if (!isPresent(animations)) {
121-
this._animationListeners.set(element, animations = []);
122-
}
123-
animations.push(new _AnimationOutputHandler(eventName, eventPhase, eventHandler));
124-
}
71+
get destroyed(): boolean { return this.cdMode === ChangeDetectorStatus.Destroyed; }
12572

12673
create(context: T, givenProjectableNodes: Array<any|any[]>, rootSelectorOrNode: string|any):
12774
AppElement {
@@ -234,11 +181,11 @@ export abstract class AppView<T> {
234181
this.destroyInternal();
235182
this.dirtyParentQueriesInternal();
236183

237-
if (this.animationPlayers.length == 0) {
238-
this.renderer.destroyView(hostElement, this.allNodes);
184+
if (this._animationContext) {
185+
this._animationContext.onAllActiveAnimationsDone(
186+
() => this.renderer.destroyView(hostElement, this.allNodes));
239187
} else {
240-
var player = new AnimationGroupPlayer(this.animationPlayers.getAllPlayers());
241-
player.onDone(() => { this.renderer.destroyView(hostElement, this.allNodes); });
188+
this.renderer.destroyView(hostElement, this.allNodes);
242189
}
243190
}
244191

@@ -254,11 +201,11 @@ export abstract class AppView<T> {
254201

255202
detach(): void {
256203
this.detachInternal();
257-
if (this.animationPlayers.length == 0) {
258-
this.renderer.detachView(this.flatRootNodes);
204+
if (this._animationContext) {
205+
this._animationContext.onAllActiveAnimationsDone(
206+
() => this.renderer.detachView(this.flatRootNodes));
259207
} else {
260-
var player = new AnimationGroupPlayer(this.animationPlayers.getAllPlayers());
261-
player.onDone(() => { this.renderer.detachView(this.flatRootNodes); });
208+
this.renderer.detachView(this.flatRootNodes);
262209
}
263210
}
264211

@@ -466,7 +413,3 @@ function _findLastRenderNode(node: any): any {
466413
}
467414
return lastNode;
468415
}
469-
470-
class _AnimationOutputHandler {
471-
constructor(public eventName: string, public eventPhase: string, public handler: Function) {}
472-
}

modules/@angular/core/test/animation/active_animations_players_map_spec.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,25 @@ export function main() {
3636
expect(playersMap.find(elementNode, animationName)).toBe(player);
3737
expect(playersMap.findAllPlayersByElement(elementNode)).toEqual([player]);
3838
expect(playersMap.getAllPlayers()).toEqual([player]);
39-
expect(playersMap.length).toEqual(1);
39+
expect(countPlayers(playersMap)).toEqual(1);
4040
});
4141

4242
it('should remove a registered player when remove() is called', () => {
4343
var player = new MockAnimationPlayer();
4444
playersMap.set(elementNode, animationName, player);
4545
expect(playersMap.find(elementNode, animationName)).toBe(player);
46-
expect(playersMap.length).toEqual(1);
46+
expect(countPlayers(playersMap)).toEqual(1);
4747
playersMap.remove(elementNode, animationName);
4848
expect(playersMap.find(elementNode, animationName)).not.toBe(player);
49-
expect(playersMap.length).toEqual(0);
49+
expect(countPlayers(playersMap)).toEqual(0);
5050
});
5151

5252
it('should allow multiple players to be registered on the same element', () => {
5353
var player1 = new MockAnimationPlayer();
5454
var player2 = new MockAnimationPlayer();
5555
playersMap.set(elementNode, 'myAnimation1', player1);
5656
playersMap.set(elementNode, 'myAnimation2', player2);
57-
expect(playersMap.length).toEqual(2);
57+
expect(countPlayers(playersMap)).toEqual(2);
5858
expect(playersMap.findAllPlayersByElement(elementNode)).toEqual([player1, player2]);
5959
});
6060

@@ -63,10 +63,14 @@ export function main() {
6363
var player2 = new MockAnimationPlayer();
6464
playersMap.set(elementNode, animationName, player1);
6565
expect(playersMap.find(elementNode, animationName)).toBe(player1);
66-
expect(playersMap.length).toEqual(1);
66+
expect(countPlayers(playersMap)).toEqual(1);
6767
playersMap.set(elementNode, animationName, player2);
6868
expect(playersMap.find(elementNode, animationName)).toBe(player2);
69-
expect(playersMap.length).toEqual(1);
69+
expect(countPlayers(playersMap)).toEqual(1);
7070
});
7171
});
7272
}
73+
74+
function countPlayers(map: ViewAnimationMap): number {
75+
return map.getAllPlayers().length;
76+
}

0 commit comments

Comments
 (0)