fix(animations): properly cache renderer and namespace triggers (#14703)

- Don’t use the animation renderer if a component
  used style encapsulation but no animations.
- The `AnimationRenderer` should be cached in the same
  lifecycle as its delegate.
- Trigger names need to be namespaced per component type.
This commit is contained in:
Tobias Bosch 2017-02-24 12:10:19 -08:00 committed by Igor Minar
parent 5094aef8fd
commit 436a179552
12 changed files with 73 additions and 23 deletions

View File

@ -123,7 +123,7 @@ export interface RendererTypeV2 {
id: string; id: string;
encapsulation: ViewEncapsulation; encapsulation: ViewEncapsulation;
styles: (string|any[])[]; styles: (string|any[])[];
data: {[kind: string]: any[]}; data: {[kind: string]: any};
} }
/** /**
@ -137,6 +137,12 @@ export abstract class RendererFactoryV2 {
* @experimental * @experimental
*/ */
export abstract class RendererV2 { export abstract class RendererV2 {
/**
* This field can be used to store arbitrary data on this renderer instance.
* This is useful for renderers that delegate to other renderers.
*/
abstract get data(): {[key: string]: any};
abstract destroy(): void; abstract destroy(): void;
abstract createElement(name: string, namespace?: string): any; abstract createElement(name: string, namespace?: string): any;
abstract createComment(value: string): any; abstract createComment(value: string): any;

View File

@ -427,6 +427,8 @@ class DebugRendererFactoryV2 implements RendererFactoryV2 {
class DebugRendererV2 implements RendererV2 { class DebugRendererV2 implements RendererV2 {
constructor(private delegate: RendererV2) {} constructor(private delegate: RendererV2) {}
get data() { return this.delegate.data; }
destroyNode(node: any) { destroyNode(node: any) {
removeDebugNodeFromIndex(getDebugNode(node)); removeDebugNodeFromIndex(getDebugNode(node));
if (this.delegate.destroyNode) { if (this.delegate.destroyNode) {

View File

@ -79,6 +79,22 @@ function declareTests({useJit}: {useJit: boolean}) {
]); ]);
}); });
it('should not throw an error if a trigger with the same name exists in separate components',
() => {
@Component({selector: 'cmp1', template: '...', animations: [trigger('trig', [])]})
class Cmp1 {
}
@Component({selector: 'cmp2', template: '...', animations: [trigger('trig', [])]})
class Cmp2 {
}
TestBed.configureTestingModule({declarations: [Cmp1, Cmp2]});
const cmp1 = TestBed.createComponent(Cmp1);
const cmp2 = TestBed.createComponent(Cmp2);
});
it('should trigger a state change animation from void => state on the component host element', it('should trigger a state change animation from void => state on the component host element',
() => { () => {
@Component({ @Component({

View File

@ -8,7 +8,7 @@
import {AnimationPlayer, AnimationTriggerMetadata} from '@angular/animations'; import {AnimationPlayer, AnimationTriggerMetadata} from '@angular/animations';
export abstract class AnimationEngine { export abstract class AnimationEngine {
abstract registerTrigger(trigger: AnimationTriggerMetadata): void; abstract registerTrigger(trigger: AnimationTriggerMetadata, name?: string): void;
abstract onInsert(element: any, domFn: () => any): void; abstract onInsert(element: any, domFn: () => any): void;
abstract onRemove(element: any, domFn: () => any): void; abstract onRemove(element: any, domFn: () => any): void;
abstract setProperty(element: any, property: string, value: any): void; abstract setProperty(element: any, property: string, value: any): void;

View File

@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be * 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 * found in the LICENSE file at https://angular.io/license
*/ */
import {AnimationTriggerMetadata} from '@angular/animations'; import {AnimationEvent, AnimationTriggerMetadata} from '@angular/animations';
import {Injectable, NgZone, RendererFactoryV2, RendererTypeV2, RendererV2} from '@angular/core'; import {Injectable, NgZone, RendererFactoryV2, RendererTypeV2, RendererV2} from '@angular/core';
import {AnimationEngine} from '../animation_engine'; import {AnimationEngine} from '../animation_engine';
@ -18,15 +18,18 @@ export class AnimationRendererFactory implements RendererFactoryV2 {
createRenderer(hostElement: any, type: RendererTypeV2): RendererV2 { createRenderer(hostElement: any, type: RendererTypeV2): RendererV2 {
let delegate = this.delegate.createRenderer(hostElement, type); let delegate = this.delegate.createRenderer(hostElement, type);
if (!hostElement || !type) return delegate; if (!hostElement || !type || !type.data || !type.data['animation']) return delegate;
let animationRenderer = type.data['__animationRenderer__'] as any as AnimationRenderer; let animationRenderer = delegate.data['animationRenderer'];
if (animationRenderer && delegate == animationRenderer.delegate) { if (!animationRenderer) {
return animationRenderer; const namespaceId = type.id;
const animationTriggers = type.data['animation'] as AnimationTriggerMetadata[];
animationTriggers.forEach(
trigger =>
this._engine.registerTrigger(trigger, namespaceify(namespaceId, trigger.name)));
animationRenderer = new AnimationRenderer(delegate, this._engine, this._zone, namespaceId);
delegate.data['animationRenderer'] = animationRenderer;
} }
const animationTriggers = type.data['animation'] as AnimationTriggerMetadata[];
animationRenderer = (type.data as any)['__animationRenderer__'] =
new AnimationRenderer(delegate, this._engine, this._zone, animationTriggers);
return animationRenderer; return animationRenderer;
} }
} }
@ -37,13 +40,12 @@ export class AnimationRenderer implements RendererV2 {
constructor( constructor(
public delegate: RendererV2, private _engine: AnimationEngine, private _zone: NgZone, public delegate: RendererV2, private _engine: AnimationEngine, private _zone: NgZone,
_triggers: AnimationTriggerMetadata[] = null) { private _namespaceId: string) {
this.destroyNode = this.delegate.destroyNode ? (n) => delegate.destroyNode(n) : null; this.destroyNode = this.delegate.destroyNode ? (n) => delegate.destroyNode(n) : null;
if (_triggers) {
_triggers.forEach(trigger => _engine.registerTrigger(trigger));
}
} }
get data() { return this.delegate.data; }
destroy(): void { this.delegate.destroy(); } destroy(): void { this.delegate.destroy(); }
createElement(name: string, namespace?: string): any { createElement(name: string, namespace?: string): any {
@ -102,7 +104,7 @@ export class AnimationRenderer implements RendererV2 {
setProperty(el: any, name: string, value: any): void { setProperty(el: any, name: string, value: any): void {
if (name.charAt(0) == '@') { if (name.charAt(0) == '@') {
this._engine.setProperty(el, name.substr(1), value); this._engine.setProperty(el, namespaceify(this._namespaceId, name.substr(1)), value);
this._queueFlush(); this._queueFlush();
} else { } else {
this.delegate.setProperty(el, name, value); this.delegate.setProperty(el, name, value);
@ -115,7 +117,13 @@ export class AnimationRenderer implements RendererV2 {
const element = resolveElementFromTarget(target); const element = resolveElementFromTarget(target);
const [name, phase] = parseTriggerCallbackName(eventName.substr(1)); const [name, phase] = parseTriggerCallbackName(eventName.substr(1));
return this._engine.listen( return this._engine.listen(
element, name, phase, (event: any) => this._zone.run(() => callback(event))); element, namespaceify(this._namespaceId, name), phase, (event: any) => {
const e = event as any;
if (e.triggerName) {
e.triggerName = deNamespaceify(this._namespaceId, e.triggerName);
}
this._zone.run(() => callback(event));
});
} }
return this.delegate.listen(target, eventName, callback); return this.delegate.listen(target, eventName, callback);
} }
@ -151,3 +159,11 @@ function parseTriggerCallbackName(triggerName: string) {
const phase = triggerName.substr(dotIndex + 1); const phase = triggerName.substr(dotIndex + 1);
return [trigger, phase]; return [trigger, phase];
} }
function namespaceify(namespaceId: string, value: string): string {
return `${namespaceId}#${value}`;
}
function deNamespaceify(namespaceId: string, value: string): string {
return value.replace(namespaceId + '#', '');
}

View File

@ -55,8 +55,8 @@ export class DomAnimationEngine {
return players; return players;
} }
registerTrigger(trigger: AnimationTriggerMetadata): void { registerTrigger(trigger: AnimationTriggerMetadata, name: string = null): void {
const name = trigger.name; name = name || trigger.name;
if (this._triggers[name]) { if (this._triggers[name]) {
throw new Error(`The provided animation trigger "${name}" has already been registered!`); throw new Error(`The provided animation trigger "${name}" has already been registered!`);
} }

View File

@ -34,7 +34,7 @@ export class NoopAnimationEngine extends AnimationEngine {
private _onDoneFns: (() => any)[] = []; private _onDoneFns: (() => any)[] = [];
private _triggerStyles: {[triggerName: string]: {[stateName: string]: ɵStyleData}} = {}; private _triggerStyles: {[triggerName: string]: {[stateName: string]: ɵStyleData}} = {};
registerTrigger(trigger: AnimationTriggerMetadata): void { registerTrigger(trigger: AnimationTriggerMetadata, name: string = null): void {
const stateMap: {[stateName: string]: ɵStyleData} = {}; const stateMap: {[stateName: string]: ɵStyleData} = {};
trigger.definitions.forEach(def => { trigger.definitions.forEach(def => {
if (def.type === AnimationMetadataType.State) { if (def.type === AnimationMetadataType.State) {
@ -42,7 +42,8 @@ export class NoopAnimationEngine extends AnimationEngine {
stateMap[stateDef.name] = normalizeStyles(stateDef.styles.styles); stateMap[stateDef.name] = normalizeStyles(stateDef.styles.styles);
} }
}); });
this._triggerStyles[trigger.name] = stateMap; name = name || trigger.name;
this._triggerStyles[name] = stateMap;
} }
onInsert(element: any, domFn: () => any): void { domFn(); } onInsert(element: any, domFn: () => any): void { domFn(); }

View File

@ -385,6 +385,8 @@ export class DomRendererFactoryV2 implements RendererFactoryV2 {
} }
class DefaultDomRendererV2 implements RendererV2 { class DefaultDomRendererV2 implements RendererV2 {
data: {[key: string]: any} = Object.create(null);
constructor(private eventManager: EventManager) {} constructor(private eventManager: EventManager) {}
destroy(): void {} destroy(): void {}

View File

@ -82,7 +82,7 @@ export function main() {
expect(engine.captures['setProperty']).toBeFalsy(); expect(engine.captures['setProperty']).toBeFalsy();
renderer.setProperty(element, '@prop', 'value'); renderer.setProperty(element, '@prop', 'value');
expect(engine.captures['setProperty'].pop()).toEqual([element, 'prop', 'value']); expect(engine.captures['setProperty'].pop()).toEqual([element, 'id#prop', 'value']);
}); });
describe('listen', () => { describe('listen', () => {
@ -96,7 +96,7 @@ export function main() {
expect(engine.captures['listen']).toBeFalsy(); expect(engine.captures['listen']).toBeFalsy();
renderer.listen(element, '@event.phase', cb); renderer.listen(element, '@event.phase', cb);
expect(engine.captures['listen'].pop()).toEqual([element, 'event', 'phase']); expect(engine.captures['listen'].pop()).toEqual([element, 'id#event', 'phase']);
}); });
it('should resolve the body|document|window nodes given their values as strings as input', it('should resolve the body|document|window nodes given their values as strings as input',

View File

@ -306,6 +306,8 @@ export class ServerRendererFactoryV2 implements RendererFactoryV2 {
} }
class DefaultServerRendererV2 implements RendererV2 { class DefaultServerRendererV2 implements RendererV2 {
data: {[key: string]: any} = Object.create(null);
constructor( constructor(
private document: any, private ngZone: NgZone, private schema: DomElementSchemaRegistry) {} private document: any, private ngZone: NgZone, private schema: DomElementSchemaRegistry) {}

View File

@ -386,6 +386,8 @@ export class WebWorkerRendererFactoryV2 implements RendererFactoryV2 {
export class WebWorkerRendererV2 implements RendererV2 { export class WebWorkerRendererV2 implements RendererV2 {
data: {[key: string]: any} = Object.create(null);
constructor(private _rendererFactory: WebWorkerRendererFactoryV2) {} constructor(private _rendererFactory: WebWorkerRendererFactoryV2) {}
private asFnArg = new FnArg(this, SerializerTypes.RENDER_STORE_OBJECT); private asFnArg = new FnArg(this, SerializerTypes.RENDER_STORE_OBJECT);

View File

@ -834,7 +834,7 @@ export declare abstract class RendererFactoryV2 {
/** @experimental */ /** @experimental */
export interface RendererTypeV2 { export interface RendererTypeV2 {
data: { data: {
[kind: string]: any[]; [kind: string]: any;
}; };
encapsulation: ViewEncapsulation; encapsulation: ViewEncapsulation;
id: string; id: string;
@ -843,6 +843,9 @@ export interface RendererTypeV2 {
/** @experimental */ /** @experimental */
export declare abstract class RendererV2 { export declare abstract class RendererV2 {
readonly abstract data: {
[key: string]: any;
};
destroyNode: (node: any) => void | null; destroyNode: (node: any) => void | null;
abstract addClass(el: any, name: string): void; abstract addClass(el: any, name: string): void;
abstract appendChild(parent: any, newChild: any): void; abstract appendChild(parent: any, newChild: any): void;