Skip to content

Commit 0a51ccb

Browse files
tboschtbosch
committed
feat(render): don’t use the reflector for setting properties
BREAKING CHANGES: - host actions don't take an expression as value any more but only a method name, and assumes to get an array via the EventEmitter with the method arguments. - Renderer.setElementProperty does not take `style.`/... prefixes any more. Use the new methods `Renderer.setElementAttribute`, ... instead Part of angular#2476 Closes angular#2637
1 parent 2932377 commit 0a51ccb

File tree

32 files changed

+642
-567
lines changed

32 files changed

+642
-567
lines changed

‎modules/angular2/src/change_detection/binding_record.ts‎

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,17 @@ import {DirectiveIndex, DirectiveRecord} from './directive_record';
55

66
const DIRECTIVE = "directive";
77
const DIRECTIVE_LIFECYCLE = "directiveLifecycle";
8-
const ELEMENT = "element";
8+
const ELEMENT_PROPERTY = "elementProperty";
9+
const ELEMENT_ATTRIBUTE = "elementAttribute";
10+
const ELEMENT_CLASS = "elementClass";
11+
const ELEMENT_STYLE = "elementStyle";
912
const TEXT_NODE = "textNode";
1013

1114
export class BindingRecord {
1215
constructor(public mode: string, public implicitReceiver: any, public ast: AST,
13-
public elementIndex: number, public propertyName: string, public setter: SetterFn,
14-
public lifecycleEvent: string, public directiveRecord: DirectiveRecord) {}
16+
public elementIndex: number, public propertyName: string, public propertyUnit: string,
17+
public setter: SetterFn, public lifecycleEvent: string,
18+
public directiveRecord: DirectiveRecord) {}
1519

1620
callOnChange(): boolean {
1721
return isPresent(this.directiveRecord) && this.directiveRecord.callOnChange;
@@ -25,41 +29,85 @@ export class BindingRecord {
2529

2630
isDirectiveLifecycle(): boolean { return this.mode === DIRECTIVE_LIFECYCLE; }
2731

28-
isElement(): boolean { return this.mode === ELEMENT; }
32+
isElementProperty(): boolean { return this.mode === ELEMENT_PROPERTY; }
33+
34+
isElementAttribute(): boolean { return this.mode === ELEMENT_ATTRIBUTE; }
35+
36+
isElementClass(): boolean { return this.mode === ELEMENT_CLASS; }
37+
38+
isElementStyle(): boolean { return this.mode === ELEMENT_STYLE; }
2939

3040
isTextNode(): boolean { return this.mode === TEXT_NODE; }
3141

3242
static createForDirective(ast: AST, propertyName: string, setter: SetterFn,
3343
directiveRecord: DirectiveRecord): BindingRecord {
34-
return new BindingRecord(DIRECTIVE, 0, ast, 0, propertyName, setter, null, directiveRecord);
44+
return new BindingRecord(DIRECTIVE, 0, ast, 0, propertyName, null, setter, null,
45+
directiveRecord);
3546
}
3647

3748
static createDirectiveOnCheck(directiveRecord: DirectiveRecord): BindingRecord {
38-
return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, "onCheck",
49+
return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, null, "onCheck",
3950
directiveRecord);
4051
}
4152

4253
static createDirectiveOnInit(directiveRecord: DirectiveRecord): BindingRecord {
43-
return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, "onInit",
54+
return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, null, "onInit",
4455
directiveRecord);
4556
}
4657

4758
static createDirectiveOnChange(directiveRecord: DirectiveRecord): BindingRecord {
48-
return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, "onChange",
59+
return new BindingRecord(DIRECTIVE_LIFECYCLE, 0, null, 0, null, null, null, "onChange",
4960
directiveRecord);
5061
}
5162

52-
static createForElement(ast: AST, elementIndex: number, propertyName: string): BindingRecord {
53-
return new BindingRecord(ELEMENT, 0, ast, elementIndex, propertyName, null, null, null);
63+
static createForElementProperty(ast: AST, elementIndex: number,
64+
propertyName: string): BindingRecord {
65+
return new BindingRecord(ELEMENT_PROPERTY, 0, ast, elementIndex, propertyName, null, null, null,
66+
null);
67+
}
68+
69+
static createForElementAttribute(ast: AST, elementIndex: number,
70+
attributeName: string): BindingRecord {
71+
return new BindingRecord(ELEMENT_ATTRIBUTE, 0, ast, elementIndex, attributeName, null, null,
72+
null, null);
73+
}
74+
75+
static createForElementClass(ast: AST, elementIndex: number, className: string): BindingRecord {
76+
return new BindingRecord(ELEMENT_CLASS, 0, ast, elementIndex, className, null, null, null,
77+
null);
78+
}
79+
80+
static createForElementStyle(ast: AST, elementIndex: number, styleName: string,
81+
unit: string): BindingRecord {
82+
return new BindingRecord(ELEMENT_STYLE, 0, ast, elementIndex, styleName, unit, null, null,
83+
null);
5484
}
5585

5686
static createForHostProperty(directiveIndex: DirectiveIndex, ast: AST,
5787
propertyName: string): BindingRecord {
58-
return new BindingRecord(ELEMENT, directiveIndex, ast, directiveIndex.elementIndex,
59-
propertyName, null, null, null);
88+
return new BindingRecord(ELEMENT_PROPERTY, directiveIndex, ast, directiveIndex.elementIndex,
89+
propertyName, null, null, null, null);
90+
}
91+
92+
static createForHostAttribute(directiveIndex: DirectiveIndex, ast: AST,
93+
attributeName: string): BindingRecord {
94+
return new BindingRecord(ELEMENT_ATTRIBUTE, directiveIndex, ast, directiveIndex.elementIndex,
95+
attributeName, null, null, null, null);
96+
}
97+
98+
static createForHostClass(directiveIndex: DirectiveIndex, ast: AST,
99+
className: string): BindingRecord {
100+
return new BindingRecord(ELEMENT_CLASS, directiveIndex, ast, directiveIndex.elementIndex,
101+
className, null, null, null, null);
102+
}
103+
104+
static createForHostStyle(directiveIndex: DirectiveIndex, ast: AST, styleName: string,
105+
unit: string): BindingRecord {
106+
return new BindingRecord(ELEMENT_STYLE, directiveIndex, ast, directiveIndex.elementIndex,
107+
styleName, unit, null, null, null);
60108
}
61109

62110
static createForTextNode(ast: AST, elementIndex: number): BindingRecord {
63-
return new BindingRecord(TEXT_NODE, 0, ast, elementIndex, null, null, null, null);
111+
return new BindingRecord(TEXT_NODE, 0, ast, elementIndex, null, null, null, null, null);
64112
}
65113
}

‎modules/angular2/src/core/compiler/element_binder.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import {List, StringMap} from 'angular2/src/facade/collection';
66
import * as viewModule from './view';
77

88
export class ElementBinder {
9-
// updated later when events are bound
10-
nestedProtoView: viewModule.AppProtoView = null;
119
// updated later, so we are able to resolve cycles
10+
nestedProtoView: viewModule.AppProtoView = null;
11+
// updated later when events are bound
1212
hostListeners: StringMap<string, Map<number, AST>> = null;
1313

1414
constructor(public index: int, public parent: ElementBinder, public distanceToParent: int,

‎modules/angular2/src/core/compiler/element_injector.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,13 +315,13 @@ export class EventEmitterAccessor {
315315
}
316316

317317
export class HostActionAccessor {
318-
constructor(public actionExpression: string, public getter: Function) {}
318+
constructor(public methodName: string, public getter: Function) {}
319319

320320
subscribe(view: viewModule.AppView, boundElementIndex: number, directive: Object) {
321321
var eventEmitter = this.getter(directive);
322322
return ObservableWrapper.subscribe(
323323
eventEmitter,
324-
actionObj => view.callAction(boundElementIndex, this.actionExpression, actionObj));
324+
actionArgs => view.invokeElementMethod(boundElementIndex, this.methodName, actionArgs));
325325
}
326326
}
327327

‎modules/angular2/src/core/compiler/proto_view_factory.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,20 @@ class BindingRecordsCreator {
6666

6767
_createElementPropertyRecords(bindings: List<BindingRecord>, boundElementIndex: number,
6868
renderElementBinder: renderApi.ElementBinder) {
69-
MapWrapper.forEach(renderElementBinder.propertyBindings, (astWithSource, propertyName) => {
70-
71-
bindings.push(BindingRecord.createForElement(astWithSource, boundElementIndex, propertyName));
69+
ListWrapper.forEach(renderElementBinder.propertyBindings, (binding) => {
70+
if (binding.type === renderApi.PropertyBindingType.PROPERTY) {
71+
bindings.push(BindingRecord.createForElementProperty(binding.astWithSource,
72+
boundElementIndex, binding.property));
73+
} else if (binding.type === renderApi.PropertyBindingType.ATTRIBUTE) {
74+
bindings.push(BindingRecord.createForElementAttribute(binding.astWithSource,
75+
boundElementIndex, binding.property));
76+
} else if (binding.type === renderApi.PropertyBindingType.CLASS) {
77+
bindings.push(BindingRecord.createForElementClass(binding.astWithSource, boundElementIndex,
78+
binding.property));
79+
} else if (binding.type === renderApi.PropertyBindingType.STYLE) {
80+
bindings.push(BindingRecord.createForElementStyle(binding.astWithSource, boundElementIndex,
81+
binding.property, binding.unit));
82+
}
7283
});
7384
}
7485

@@ -103,10 +114,21 @@ class BindingRecordsCreator {
103114
for (var i = 0; i < directiveBinders.length; i++) {
104115
var directiveBinder = directiveBinders[i];
105116
// host properties
106-
MapWrapper.forEach(directiveBinder.hostPropertyBindings, (astWithSource, propertyName) => {
117+
ListWrapper.forEach(directiveBinder.hostPropertyBindings, (binding) => {
107118
var dirIndex = new DirectiveIndex(boundElementIndex, i);
108-
109-
bindings.push(BindingRecord.createForHostProperty(dirIndex, astWithSource, propertyName));
119+
if (binding.type === renderApi.PropertyBindingType.PROPERTY) {
120+
bindings.push(BindingRecord.createForHostProperty(dirIndex, binding.astWithSource,
121+
binding.property));
122+
} else if (binding.type === renderApi.PropertyBindingType.ATTRIBUTE) {
123+
bindings.push(BindingRecord.createForHostAttribute(dirIndex, binding.astWithSource,
124+
binding.property));
125+
} else if (binding.type === renderApi.PropertyBindingType.CLASS) {
126+
bindings.push(
127+
BindingRecord.createForHostClass(dirIndex, binding.astWithSource, binding.property));
128+
} else if (binding.type === renderApi.PropertyBindingType.STYLE) {
129+
bindings.push(BindingRecord.createForHostStyle(dirIndex, binding.astWithSource,
130+
binding.property, binding.unit));
131+
}
110132
});
111133
}
112134
}

‎modules/angular2/src/core/compiler/view.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,20 @@ export class AppView implements ChangeDispatcher, EventDispatcher {
9999

100100
// dispatch to element injector or text nodes based on context
101101
notifyOnBinding(b: BindingRecord, currentValue: any): void {
102-
if (b.isElement()) {
102+
if (b.isElementProperty()) {
103103
this.renderer.setElementProperty(this.render, b.elementIndex, b.propertyName, currentValue);
104-
} else {
105-
// we know it refers to _textNodes.
104+
} else if (b.isElementAttribute()) {
105+
this.renderer.setElementAttribute(this.render, b.elementIndex, b.propertyName, currentValue);
106+
} else if (b.isElementClass()) {
107+
this.renderer.setElementClass(this.render, b.elementIndex, b.propertyName, currentValue);
108+
} else if (b.isElementStyle()) {
109+
var unit = isPresent(b.propertyUnit) ? b.propertyUnit : '';
110+
this.renderer.setElementStyle(this.render, b.elementIndex, b.propertyName,
111+
`${currentValue}${unit}`);
112+
} else if (b.isTextNode()) {
106113
this.renderer.setText(this.render, b.elementIndex, currentValue);
114+
} else {
115+
throw new BaseException('Unsupported directive record');
107116
}
108117
}
109118

@@ -124,8 +133,8 @@ export class AppView implements ChangeDispatcher, EventDispatcher {
124133
return isPresent(childView) ? childView.changeDetector : null;
125134
}
126135

127-
callAction(elementIndex: number, actionExpression: string, action: Object) {
128-
this.renderer.callAction(this.render, elementIndex, actionExpression, action);
136+
invokeElementMethod(elementIndex: number, methodName: string, args: List<any>) {
137+
this.renderer.invokeElementMethod(this.render, elementIndex, methodName, args);
129138
}
130139

131140
// implementation of EventDispatcher#dispatchEvent

‎modules/angular2/src/dom/browser_adapter.dart‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
library angular.core.facade.dom;
22

33
import 'dart:html';
4-
import 'dart:js' show JsObject;
54
import 'dom_adapter.dart' show setRootDomAdapter;
65
import 'generic_browser_adapter.dart' show GenericBrowserDomAdapter;
76
import '../facade/browser.dart';
@@ -97,9 +96,28 @@ final _keyCodeToKeyMap = const {
9796
};
9897

9998
class BrowserDomAdapter extends GenericBrowserDomAdapter {
99+
js.JsFunction _setProperty;
100+
js.JsFunction _getProperty;
101+
js.JsFunction _hasProperty;
102+
BrowserDomAdapter() {
103+
_setProperty = js.context.callMethod('eval', ['(function(el, prop, value) { el[prop] = value; })']);
104+
_getProperty = js.context.callMethod('eval', ['(function(el, prop) { return el[prop]; })']);
105+
_hasProperty = js.context.callMethod('eval', ['(function(el, prop) { return prop in el; })']);
106+
}
100107
static void makeCurrent() {
101108
setRootDomAdapter(new BrowserDomAdapter());
102109
}
110+
bool hasProperty(Element element, String name) =>
111+
_hasProperty.apply([element, name]);
112+
113+
void setProperty(Element element, String name, Object value) =>
114+
_setProperty.apply([element, name, value]);
115+
116+
getProperty(Element element, String name) =>
117+
_getProperty.apply([element, name]);
118+
119+
invoke(Element element, String methodName, List args) =>
120+
this.getProperty(element, methodName).apply(args, thisArg: element);
103121

104122
// TODO(tbosch): move this into a separate environment class once we have it
105123
logError(error) {
@@ -108,7 +126,7 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter {
108126

109127
@override
110128
Map<String, String> get attrToPropMap => const <String, String>{
111-
'innerHtml': 'innerHtml',
129+
'innerHtml': 'innerHTML',
112130
'readonly': 'readOnly',
113131
'tabindex': 'tabIndex',
114132
};
@@ -221,8 +239,6 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter {
221239
ShadowRoot getShadowRoot(Element el) => el.shadowRoot;
222240
Element getHost(Element el) => (el as ShadowRoot).host;
223241
clone(Node node) => node.clone(true);
224-
bool hasProperty(Element element, String name) =>
225-
new JsObject.fromBrowserObject(element).hasProperty(name);
226242
List<Node> getElementsByClassName(Element element, String name) =>
227243
element.getElementsByClassName(name);
228244
List<Node> getElementsByTagName(Element element, String name) =>

‎modules/angular2/src/dom/browser_adapter.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ var _chromeNumKeyPadMap = {
5050

5151
export class BrowserDomAdapter extends GenericBrowserDomAdapter {
5252
static makeCurrent() { setRootDomAdapter(new BrowserDomAdapter()); }
53+
hasProperty(element, name: string) { return name in element; }
54+
setProperty(el: /*element*/ any, name: string, value: any) { el[name] = value; }
55+
getProperty(el: /*element*/ any, name: string): any { return el[name]; }
56+
invoke(el: /*element*/ any, methodName: string, args: List<any>): any {
57+
el[methodName].apply(el, args);
58+
}
5359

5460
// TODO(tbosch): move this into a separate environment class once we have it
5561
logError(error) { window.console.error(error); }
@@ -152,7 +158,6 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
152158
getShadowRoot(el: HTMLElement): DocumentFragment { return (<any>el).shadowRoot; }
153159
getHost(el: HTMLElement): HTMLElement { return (<any>el).host; }
154160
clone(node: Node) { return node.cloneNode(true); }
155-
hasProperty(element, name: string) { return name in element; }
156161
getElementsByClassName(element, name: string) { return element.getElementsByClassName(name); }
157162
getElementsByTagName(element, name: string) { return element.getElementsByTagName(name); }
158163
classList(element): List<any> {

‎modules/angular2/src/dom/dom_adapter.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ function _abstract() {
1616
* Provides DOM operations in an environment-agnostic way.
1717
*/
1818
export class DomAdapter {
19+
hasProperty(element, name: string): boolean { throw _abstract(); }
20+
setProperty(el: /*element*/ any, name: string, value: any) { throw _abstract(); }
21+
getProperty(el: /*element*/ any, name: string): any { throw _abstract(); }
22+
invoke(el: /*element*/ any, methodName: string, args: List<any>): any { throw _abstract(); }
23+
1924
logError(error) { throw _abstract(); }
2025

2126
/**
@@ -70,7 +75,6 @@ export class DomAdapter {
7075
getHost(el): any { throw _abstract(); }
7176
getDistributedNodes(el): List<any> { throw _abstract(); }
7277
clone(node): any { throw _abstract(); }
73-
hasProperty(element, name: string): boolean { throw _abstract(); }
7478
getElementsByClassName(element, name: string): List<any> { throw _abstract(); }
7579
getElementsByTagName(element, name: string): List<any> { throw _abstract(); }
7680
classList(element): List<any> { throw _abstract(); }

‎modules/angular2/src/dom/html_adapter.dart‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,24 @@ class Html5LibDomAdapter implements DomAdapter {
1010
setRootDomAdapter(new Html5LibDomAdapter());
1111
}
1212

13+
hasProperty(element, String name) {
14+
// This is needed for serverside compile to generate the right getters/setters...
15+
return true;
16+
}
17+
18+
void setProperty(Element element, String name, Object value) => throw 'not implemented';
19+
20+
getProperty(Element element, String name) => throw 'not implemented';
21+
22+
invoke(Element element, String methodName, List args) => throw 'not implemented';
23+
1324
logError(error) {
1425
stderr.writeln('${error}');
1526
}
1627

1728
@override
1829
final attrToPropMap = const {
19-
'innerHtml': 'innerHtml',
30+
'innerHtml': 'innerHTML',
2031
'readonly': 'readOnly',
2132
'tabindex': 'tabIndex',
2233
};
@@ -184,11 +195,6 @@ class Html5LibDomAdapter implements DomAdapter {
184195
throw 'not implemented';
185196
}
186197
clone(node) => node.clone(true);
187-
188-
hasProperty(element, String name) {
189-
// This is needed for serverside compile to generate the right getters/setters...
190-
return true;
191-
}
192198
getElementsByClassName(element, String name) {
193199
throw 'not implemented';
194200
}

‎modules/angular2/src/dom/parse5_adapter.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,20 @@ function _notImplemented(methodName) {
2626
export class Parse5DomAdapter extends DomAdapter {
2727
static makeCurrent() { setRootDomAdapter(new Parse5DomAdapter()); }
2828

29+
hasProperty(element, name: string) { return _HTMLElementPropertyList.indexOf(name) > -1; }
30+
// TODO(tbosch): don't even call this method when we run the tests on server side
31+
// by not using the DomRenderer in tests. Keeping this for now to make tests happy...
32+
setProperty(el: /*element*/ any, name: string, value: any) {
33+
if (name === 'innerHTML') {
34+
this.setInnerHTML(el, value);
35+
} else {
36+
el[name] = value;
37+
}
38+
}
39+
// TODO(tbosch): don't even call this method when we run the tests on server side
40+
// by not using the DomRenderer in tests. Keeping this for now to make tests happy...
41+
getProperty(el: /*element*/ any, name: string): any { return el[name]; }
42+
2943
logError(error) { console.error(error); }
3044

3145
get attrToPropMap() { return _attrToPropMap; }
@@ -268,7 +282,6 @@ export class Parse5DomAdapter extends DomAdapter {
268282
return newParser.parseFragment(serialized).childNodes[0];
269283
}
270284
}
271-
hasProperty(element, name: string) { return _HTMLElementPropertyList.indexOf(name) > -1; }
272285
getElementsByClassName(element, name: string) {
273286
return this.querySelectorAll(element, "." + name);
274287
}

0 commit comments

Comments
 (0)