cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

perf(render): don't create property setters if not needed

Browse Source
This commit is contained in:
Tobias Bosch 2015-06-10 10:31:38 -07:00
parent 24e647e0f7
commit 4f27611ae6
5 changed files with 87 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
modules/angular2/src/dom/html_adapter.dart
View File

@ -142,7 +142,7 @@ class Html5LibDomAdapter implements DomAdapter {
throw 'not implemented';
}
getText(el) {
throw 'not implemented';
return el.text;
}
setText(el, String value) => el.text = value;
@ -183,7 +183,8 @@ class Html5LibDomAdapter implements DomAdapter {
clone(node) => node.clone(true);
hasProperty(element, String name) {
throw 'not implemented';
// This is needed for serverside compile to generate the right getters/setters...
return true;
}
getElementsByClassName(element, String name) {
throw 'not implemented';

2
modules/angular2/src/dom/parse5_adapter.ts
View File

@ -205,7 +205,7 @@ export class Parse5DomAdapter extends DomAdapter {
getText(el) {
if (this.isTextNode(el)) {
return el.data;
} else if (el.childNodes.length == 0) {
} else if (isBlank(el.childNodes) || el.childNodes.length == 0) {
return "";
} else {
var textContent = "";

37
modules/angular2/src/render/dom/view/property_setter_factory.ts
View File

@ -18,13 +18,18 @@ const CLASS_PREFIX = 'class.';
const STYLE_PREFIX = 'style.';
export class PropertySetterFactory {
private _propertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
private static _noopSetter(el, value) {}
private _lazyPropertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
private _eagerPropertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
private _innerHTMLSetterCache: Function;
private _attributeSettersCache: StringMap<string, Function> = StringMapWrapper.create();
private _classSettersCache: StringMap<string, Function> = StringMapWrapper.create();
private _styleSettersCache: StringMap<string, Function> = StringMapWrapper.create();
createSetter(property: string): Function {
constructor() { this._innerHTMLSetterCache = (el, value) => DOM.setInnerHTML(el, value); }
createSetter(protoElement: /*element*/ any, isNgComponent: boolean, property: string): Function {
var setterFn, styleParts, styleSuffix;
if (StringWrapper.startsWith(property, ATTRIBUTE_PREFIX)) {
setterFn =
@ -36,13 +41,21 @@ export class PropertySetterFactory {
styleSuffix = styleParts.length > 2 ? ListWrapper.get(styleParts, 2) : '';
setterFn = this._styleSetterFactory(ListWrapper.get(styleParts, 1), styleSuffix);
} else if (StringWrapper.equals(property, 'innerHtml')) {
if (isBlank(this._innerHTMLSetterCache)) {
this._innerHTMLSetterCache = (el, value) => DOM.setInnerHTML(el, value);
}
setterFn = this._innerHTMLSetterCache;
} else {
property = this._resolvePropertyName(property);
setterFn = StringMapWrapper.get(this._propertySettersCache, property);
setterFn = this._propertySetterFactory(protoElement, isNgComponent, property);
}
return setterFn;
}
private _propertySetterFactory(protoElement, isNgComponent: boolean, property: string): Function {
var setterFn;
var tagName = DOM.tagName(protoElement);
var possibleCustomElement = tagName.indexOf('-') !== -1;
if (possibleCustomElement && !isNgComponent) {
// need to use late check to be able to set properties on custom elements
setterFn = StringMapWrapper.get(this._lazyPropertySettersCache, property);
if (isBlank(setterFn)) {
var propertySetterFn = reflector.setter(property);
setterFn = (receiver, value) => {
@ -50,7 +63,17 @@ export class PropertySetterFactory {
return propertySetterFn(receiver, value);
}
};
StringMapWrapper.set(this._propertySettersCache, property, setterFn);
StringMapWrapper.set(this._lazyPropertySettersCache, property, setterFn);
}
} else {
setterFn = StringMapWrapper.get(this._eagerPropertySettersCache, property);
if (isBlank(setterFn)) {
if (DOM.hasProperty(protoElement, property)) {
setterFn = reflector.setter(property);
} else {
setterFn = PropertySetterFactory._noopSetter;
}
StringMapWrapper.set(this._eagerPropertySettersCache, property, setterFn);
}
}
return setterFn;

4
modules/angular2/src/render/dom/view/proto_view_builder.ts
View File

@ -57,7 +57,7 @@ export class ProtoViewBuilder {
MapWrapper.forEach(dbb.hostPropertyBindings, (_, hostPropertyName) => {
MapWrapper.set(propertySetters, hostPropertyName,
setterFactory.createSetter(hostPropertyName));
setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), hostPropertyName));
});
ListWrapper.forEach(dbb.hostActions, (hostAction) => {
@ -73,7 +73,7 @@ export class ProtoViewBuilder {
});
MapWrapper.forEach(ebb.propertyBindings, (_, propertyName) => {
MapWrapper.set(propertySetters, propertyName, setterFactory.createSetter(propertyName));
MapWrapper.set(propertySetters, propertyName, setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName));
});
var nestedProtoView =

68
modules/angular2/test/render/dom/view/property_setter_factory_spec.ts
View File

@ -7,7 +7,8 @@ import {
xdescribe,
expect,
beforeEach,
el
el,
IS_DARTIUM
} from 'angular2/test_lib';
import {PropertySetterFactory} from 'angular2/src/render/dom/view/property_setter_factory';
import {DOM} from 'angular2/src/dom/dom_adapter';
@ -20,17 +21,50 @@ export function main() {
});
describe('property setter factory', () => {
it('should return a setter for a property', () => {
var setterFn = setterFactory.createSetter('title');
setterFn(div, 'Hello');
expect(div.title).toEqual('Hello');
describe('property setters', () => {
it('should set an existing property', () => {
var setterFn = setterFactory.createSetter(div, false, 'title');
setterFn(div, 'Hello');
expect(div.title).toEqual('Hello');
var otherSetterFn = setterFactory.createSetter(div, false, 'title');
expect(setterFn).toBe(otherSetterFn);
});
if (!IS_DARTIUM) {
it('should use a noop setter if the property did not exist when the setter was created', () => {
var setterFn = setterFactory.createSetter(div, false, 'someProp');
div.someProp = '';
setterFn(div, 'Hello');
expect(div.someProp).toEqual('');
});
it('should use a noop setter if the property did not exist when the setter was created for ng components', () => {
var ce = el('<some-ce></some-ce>');
var setterFn = setterFactory.createSetter(ce, true, 'someProp');
ce.someProp = '';
setterFn(ce, 'Hello');
expect(ce.someProp).toEqual('');
});
it('should set the property for custom elements even if it was not present when the setter was created', () => {
var ce = el('<some-ce></some-ce>');
var setterFn = setterFactory.createSetter(ce, false, 'someProp');
ce.someProp = '';
// Our CJS DOM adapter does not support custom properties,
// need to exclude here.
if (DOM.hasProperty(ce, 'someProp')) {
setterFn(ce, 'Hello');
expect(ce.someProp).toEqual('Hello');
}
});
}
var otherSetterFn = setterFactory.createSetter('title');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for an attribute', () => {
var setterFn = setterFactory.createSetter('attr.role');
var setterFn = setterFactory.createSetter(div, false, 'attr.role');
setterFn(div, 'button');
expect(DOM.getAttribute(div, 'role')).toEqual('button');
setterFn(div, null);
@ -38,49 +72,49 @@ export function main() {
expect(() => { setterFn(div, 4); })
.toThrowError("Invalid role attribute, only string values are allowed, got '4'");
var otherSetterFn = setterFactory.createSetter('attr.role');
var otherSetterFn = setterFactory.createSetter(div, false, 'attr.role');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for a class', () => {
var setterFn = setterFactory.createSetter('class.active');
var setterFn = setterFactory.createSetter(div, false, 'class.active');
setterFn(div, true);
expect(DOM.hasClass(div, 'active')).toEqual(true);
setterFn(div, false);
expect(DOM.hasClass(div, 'active')).toEqual(false);
var otherSetterFn = setterFactory.createSetter('class.active');
var otherSetterFn = setterFactory.createSetter(div, false, 'class.active');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for a style', () => {
var setterFn = setterFactory.createSetter('style.width');
var setterFn = setterFactory.createSetter(div, false, 'style.width');
setterFn(div, '40px');
expect(DOM.getStyle(div, 'width')).toEqual('40px');
setterFn(div, null);
expect(DOM.getStyle(div, 'width')).toEqual('');
var otherSetterFn = setterFactory.createSetter('style.width');
var otherSetterFn = setterFactory.createSetter(div, false, 'style.width');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for a style with a unit', () => {
var setterFn = setterFactory.createSetter('style.height.px');
var setterFn = setterFactory.createSetter(div, false, 'style.height.px');
setterFn(div, 40);
expect(DOM.getStyle(div, 'height')).toEqual('40px');
setterFn(div, null);
expect(DOM.getStyle(div, 'height')).toEqual('');
var otherSetterFn = setterFactory.createSetter('style.height.px');
var otherSetterFn = setterFactory.createSetter(div, false, 'style.height.px');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for innerHtml', () => {
var setterFn = setterFactory.createSetter('innerHtml');
var setterFn = setterFactory.createSetter(div, false, 'innerHtml');
setterFn(div, '<span></span>');
expect(DOM.getInnerHTML(div)).toEqual('<span></span>');
var otherSetterFn = setterFactory.createSetter('innerHtml');
var otherSetterFn = setterFactory.createSetter(div, false, 'innerHtml');
expect(setterFn).toBe(otherSetterFn);
});