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 #2476
Closes #2637
This commit is contained in:
Tobias Bosch
2015-06-18 15:44:44 -07:00
parent 2932377769
commit 0a51ccbd68
32 changed files with 643 additions and 568 deletions

View File

@ -28,7 +28,7 @@ function _getParser() {
function _createBindingRecords(expression: string): List<BindingRecord> {
var ast = _getParser().parseBinding(expression, 'location');
return [BindingRecord.createForElement(ast, 0, PROP_NAME)];
return [BindingRecord.createForElementProperty(ast, 0, PROP_NAME)];
}
function _convertLocalsToVariableBindings(locals: Locals): List<any> {
@ -247,8 +247,8 @@ class _DirectiveUpdating {
'interpolation':
new _DirectiveUpdating(
[
BindingRecord.createForElement(_getParser().parseInterpolation('B{{a}}A', 'location'),
0, PROP_NAME)
BindingRecord.createForElementProperty(
_getParser().parseInterpolation('B{{a}}A', 'location'), 0, PROP_NAME)
],
[])
};

View File

@ -446,7 +446,7 @@ export function main() {
expect(inj.hostActionAccessors.length).toEqual(1);
var accessor = inj.hostActionAccessors[0][0];
expect(accessor.actionExpression).toEqual('onAction');
expect(accessor.methodName).toEqual('onAction');
expect(accessor.getter(new HasHostAction())).toEqual('hostAction');
});
});

View File

@ -1141,10 +1141,10 @@ export function main() {
it('should specify a location of an error that happened during change detection (directive property)',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(MyComp, new viewAnn.View({
template: '<child-cmp [dir-prop]="a.b"></child-cmp>',
directives: [ChildComp]
}));
tb.overrideView(
MyComp,
new viewAnn.View(
{template: '<child-cmp [title]="a.b"></child-cmp>', directives: [ChildComp]}));
tb.createView(MyComp, {context: ctx})
.then((view) => {
@ -1474,17 +1474,14 @@ class DirectiveUpdatingHostProperties {
constructor() { this.id = "one"; }
}
@Directive({
selector: '[update-host-actions]',
host: {'@setAttr': 'setAttribute("key", $action["attrValue"])'}
})
@Directive({selector: '[update-host-actions]', host: {'@setAttr': 'setAttribute'}})
@Injectable()
class DirectiveUpdatingHostActions {
setAttr: EventEmitter;
constructor() { this.setAttr = new EventEmitter(); }
triggerSetAttr(attrValue) { ObservableWrapper.callNext(this.setAttr, {'attrValue': attrValue}); }
triggerSetAttr(attrValue) { ObservableWrapper.callNext(this.setAttr, ["key", attrValue]); }
}
@Directive({selector: '[listener]', host: {'(event)': 'onEvent($event)'}})

View File

@ -1,6 +1,12 @@
import {describe, it, expect, beforeEach, ddescribe, iit, xit} from 'angular2/test_lib';
import {List, ListWrapper, StringMap, StringMapWrapper} from 'angular2/src/facade/collection';
import {
List,
ListWrapper,
StringMap,
StringMapWrapper,
MapWrapper
} from 'angular2/src/facade/collection';
export function main() {
describe('ListWrapper', () => {
@ -109,5 +115,14 @@ export function main() {
expect(StringMapWrapper.equals(m2, m1)).toBe(false);
});
});
describe('MapWrapper', () => {
it('should return a list of keys values', () => {
var m = new Map();
m.set('a', 'b');
expect(MapWrapper.keys(m)).toEqual(['a']);
expect(MapWrapper.values(m)).toEqual(['b']);
});
});
});
}

View File

@ -27,8 +27,7 @@ export function main() {
someDirectiveWithInvalidHostProperties,
someDirectiveWithHostAttributes,
someDirectiveWithEvents,
someDirectiveWithGlobalEvents,
someDirectiveWithHostActions
someDirectiveWithGlobalEvents
];
parser = new Parser(new Lexer());
});
@ -161,12 +160,6 @@ export function main() {
expect(eventBinding.source.source).toEqual('doItGlobal()');
});
it('should bind directive host actions', () => {
var results = process(el('<div some-decor-host-actions></div>'));
var directiveBinding = results[0].directives[0];
expect(directiveBinding.hostActions[0].actionName).toEqual('focus');
});
// TODO: assertions should be enabled when running tests:
// https://github.com/angular/angular/issues/1340
describe('component directives', () => {
@ -255,11 +248,6 @@ var someDirectiveWithHostAttributes = DirectiveMetadata.create({
var someDirectiveWithEvents = DirectiveMetadata.create(
{selector: '[some-decor-events]', host: MapWrapper.createFromStringMap({'(click)': 'doIt()'})});
var someDirectiveWithHostActions = DirectiveMetadata.create({
selector: '[some-decor-host-actions]',
host: MapWrapper.createFromStringMap({'@focus': 'focus()'})
});
var someDirectiveWithGlobalEvents = DirectiveMetadata.create({
selector: '[some-decor-globalevents]',
host: MapWrapper.createFromStringMap({'(window:resize)': 'doItGlobal()'})

View File

@ -91,12 +91,13 @@ export function main() {
});
}));
it('should update element properties', inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
it('should update any element property/attributes/class/style independent of the compilation',
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
tb.compileAll([
someComponent,
new ViewDefinition({
componentId: 'someComponent',
template: '<input [value]="someProp">asdf',
template: '<input [title]="y" style="position:absolute">',
directives: []
})
])
@ -104,33 +105,50 @@ export function main() {
var rootView = tb.createRootView(protoViewDtos[0]);
var cmpView = tb.createComponentView(rootView.viewRef, 0, protoViewDtos[1]);
var el = DOM.childNodes(tb.rootEl)[0];
tb.renderer.setElementProperty(cmpView.viewRef, 0, 'value', 'hello');
expect(el.value).toEqual('hello');
tb.renderer.setElementClass(cmpView.viewRef, 0, 'a', true);
expect(DOM.childNodes(tb.rootEl)[0].value).toEqual('hello');
tb.renderer.setElementClass(cmpView.viewRef, 0, 'a', false);
expect(DOM.hasClass(el, 'a')).toBe(false);
tb.renderer.setElementStyle(cmpView.viewRef, 0, 'width', '10px');
expect(DOM.getStyle(el, 'width')).toEqual('10px');
tb.renderer.setElementStyle(cmpView.viewRef, 0, 'width', null);
expect(DOM.getStyle(el, 'width')).toEqual('');
tb.renderer.setElementAttribute(cmpView.viewRef, 0, 'someAttr', 'someValue');
expect(DOM.getAttribute(el, 'some-attr')).toEqual('someValue');
async.done();
});
}));
it('should call actions on the element',
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
tb.compileAll([
someComponent,
new ViewDefinition({
componentId: 'someComponent',
template: '<input with-host-actions></input>',
directives: [directiveWithHostActions]
})
])
.then((protoViewDtos) => {
var views = tb.createRootViews(protoViewDtos);
var componentView = views[1];
if (DOM.supportsDOMEvents()) {
it('should call actions on the element independent of the compilation',
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
tb.compileAll([
someComponent,
new ViewDefinition({
componentId: 'someComponent',
template: '<input [title]="y"></input>',
directives: []
})
])
.then((protoViewDtos) => {
var views = tb.createRootViews(protoViewDtos);
var componentView = views[1];
tb.renderer.callAction(componentView.viewRef, 0, 'value = "val"', null);
expect(DOM.getValue(DOM.childNodes(tb.rootEl)[0])).toEqual('val');
async.done();
});
}));
tb.renderer.invokeElementMethod(componentView.viewRef, 0, 'setAttribute',
['a', 'b']);
expect(DOM.getAttribute(DOM.childNodes(tb.rootEl)[0], 'a')).toEqual('b');
async.done();
});
}));
}
it('should add and remove views to and from containers',
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
@ -188,10 +206,3 @@ export function main() {
var someComponent = DirectiveMetadata.create(
{id: 'someComponent', type: DirectiveMetadata.COMPONENT_TYPE, selector: 'some-comp'});
var directiveWithHostActions = DirectiveMetadata.create({
id: 'withHostActions',
type: DirectiveMetadata.DIRECTIVE_TYPE,
selector: '[with-host-actions]',
host: MapWrapper.createFromStringMap({'@setValue': 'value = "val"'})
});

View File

@ -1,190 +0,0 @@
import {
describe,
ddescribe,
it,
iit,
xit,
xdescribe,
expect,
beforeEach,
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';
export function main() {
var div, input, setterFactory;
beforeEach(() => {
div = el('<div></div>');
input = el('<input>');
setterFactory = new PropertySetterFactory();
});
describe('property setter factory', () => {
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');
}
});
}
});
describe('non-standard property setters', () => {
it('should map readonly name to readOnly property', () => {
var setterFn = setterFactory.createSetter(input, false, 'readonly');
expect(input.readOnly).toBeFalsy();
setterFn(input, true);
expect(input.readOnly).toBeTruthy();
var otherSetterFn = setterFactory.createSetter(input, false, 'readonly');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for innerHtml', () => {
var setterFn = setterFactory.createSetter(div, false, 'innerHtml');
setterFn(div, '<span></span>');
expect(DOM.getInnerHTML(div)).toEqual('<span></span>');
var otherSetterFn = setterFactory.createSetter(div, false, 'innerHtml');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for tabIndex', () => {
var setterFn = setterFactory.createSetter(div, false, 'tabindex');
setterFn(div, 1);
expect(div.tabIndex).toEqual(1);
var otherSetterFn = setterFactory.createSetter(div, false, 'tabindex');
expect(setterFn).toBe(otherSetterFn);
});
});
describe('attribute setters', () => {
it('should return a setter for an attribute', () => {
var setterFn = setterFactory.createSetter(div, false, 'attr.role');
setterFn(div, 'button');
expect(DOM.getAttribute(div, 'role')).toEqual('button');
setterFn(div, null);
expect(DOM.getAttribute(div, 'role')).toEqual(null);
expect(() => { setterFn(div, 4); })
.toThrowError("Invalid role attribute, only string values are allowed, got '4'");
var otherSetterFn = setterFactory.createSetter(div, false, 'attr.role');
expect(setterFn).toBe(otherSetterFn);
});
it('should de-normalize attribute names', () => {
var setterFn = setterFactory.createSetter(div, false, 'attr.ariaLabel');
setterFn(div, 'fancy button');
expect(DOM.getAttribute(div, 'aria-label')).toEqual('fancy button');
var otherSetterFn = setterFactory.createSetter(div, false, 'attr.ariaLabel');
expect(setterFn).toBe(otherSetterFn);
});
});
describe('classList setters', () => {
it('should return a setter for a class', () => {
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(div, false, 'class.active');
expect(setterFn).toBe(otherSetterFn);
});
it('should de-normalize class names', () => {
var setterFn = setterFactory.createSetter(div, false, 'class.veryActive');
setterFn(div, true);
expect(DOM.hasClass(div, 'very-active')).toEqual(true);
setterFn(div, false);
expect(DOM.hasClass(div, 'very-active')).toEqual(false);
var otherSetterFn = setterFactory.createSetter(div, false, 'class.veryActive');
expect(setterFn).toBe(otherSetterFn);
});
});
describe('style setters', () => {
it('should return a setter for a style', () => {
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(div, false, 'style.width');
expect(setterFn).toBe(otherSetterFn);
});
it('should de-normalize style names', () => {
var setterFn = setterFactory.createSetter(div, false, 'style.textAlign');
setterFn(div, 'right');
expect(DOM.getStyle(div, 'text-align')).toEqual('right');
setterFn(div, null);
expect(DOM.getStyle(div, 'text-align')).toEqual('');
var otherSetterFn = setterFactory.createSetter(div, false, 'style.textAlign');
expect(setterFn).toBe(otherSetterFn);
});
it('should return a setter for a style with a unit', () => {
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(div, false, 'style.height.px');
expect(setterFn).toBe(otherSetterFn);
});
});
});
}

View File

@ -0,0 +1,115 @@
import {
describe,
ddescribe,
it,
iit,
xit,
xdescribe,
expect,
beforeEach,
el,
IS_DARTIUM
} from 'angular2/test_lib';
import {ProtoViewBuilder} from 'angular2/src/render/dom/view/proto_view_builder';
import {ASTWithSource, AST} from 'angular2/change_detection';
import {PropertyBindingType, ViewType} from 'angular2/src/render/api';
export function main() {
function emptyExpr() { return new ASTWithSource(new AST(), 'empty', 'empty'); }
describe('ProtoViewBuilder', () => {
var builder;
beforeEach(() => { builder = new ProtoViewBuilder(el('<div/>'), ViewType.EMBEDDED); });
describe('verification of properties', () => {
it('should throw for unknown properties', () => {
builder.bindElement(el('<div/>')).bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build())
.toThrowError(
`Can't bind to 'unknownProperty' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`);
});
it('should should allow unknown properties if a directive uses it', () => {
builder.bindElement(el('<div/>')).bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build())
.toThrowError(
`Can't bind to 'unknownProperty' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`);
});
it('should allow unknown properties on custom elements', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
binder.bindDirective(0).bindProperty('someDirProperty', emptyExpr(), 'unknownProperty');
expect(() => builder.build()).not.toThrow();
});
it('should throw for unkown properties on custom elements if there is an ng component', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
binder.setComponentId('someComponent');
expect(() => builder.build())
.toThrowError(
`Can't bind to 'unknownProperty' since it isn't a know property of the 'some-custom' element and there are no matching directives with a corresponding property`);
});
});
describe('property normalization', () => {
it('should normalize "innerHtml" to "innerHTML"', () => {
builder.bindElement(el('<div/>')).bindProperty('innerHtml', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].property).toEqual('innerHTML');
});
it('should normalize "tabindex" to "tabIndex"', () => {
builder.bindElement(el('<div/>')).bindProperty('tabindex', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].property).toEqual('tabIndex');
});
it('should normalize "readonly" to "readOnly"', () => {
builder.bindElement(el('<input/>')).bindProperty('readonly', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].property).toEqual('readOnly');
});
});
describe('property binding types', () => {
it('should detect property names', () => {
builder.bindElement(el('<div/>')).bindProperty('tabindex', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].type).toEqual(PropertyBindingType.PROPERTY);
});
it('should detect attribute names', () => {
builder.bindElement(el('<div/>')).bindProperty('attr.someName', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].type)
.toEqual(PropertyBindingType.ATTRIBUTE);
});
it('should detect class names', () => {
builder.bindElement(el('<div/>')).bindProperty('class.someName', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].type).toEqual(PropertyBindingType.CLASS);
});
it('should detect style names', () => {
builder.bindElement(el('<div/>')).bindProperty('style.someName', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].type).toEqual(PropertyBindingType.STYLE);
});
it('should detect style units', () => {
builder.bindElement(el('<div/>')).bindProperty('style.someName.someUnit', emptyExpr());
var pv = builder.build();
expect(pv.elementBinders[0].propertyBindings[0].unit).toEqual('someUnit');
});
});
});
}

View File

@ -16,6 +16,7 @@ import {
proxy
} from 'angular2/test_lib';
import {isBlank} from 'angular2/src/facade/lang';
import {ListWrapper} from 'angular2/src/facade/collection';
import {DomProtoView} from 'angular2/src/render/dom/view/proto_view';
import {ElementBinder} from 'angular2/src/render/dom/view/element_binder';
@ -40,7 +41,7 @@ export function main() {
function createView(pv = null, boundElementCount = 0) {
if (isBlank(pv)) {
pv = createProtoView();
pv = createProtoView(ListWrapper.createFixedSize(boundElementCount));
}
var root = el('<div><div></div></div>');
var boundElements = [];
@ -72,5 +73,87 @@ export function main() {
});
describe('setElementProperty', () => {
var el, view;
beforeEach(() => {
view = createView(null, 1);
el = view.boundElements[0].element;
});
it('should update the property value', () => {
view.setElementProperty(0, 'title', 'Hello');
expect(el.title).toEqual('Hello');
});
});
describe('setElementAttribute', () => {
var el, view;
beforeEach(() => {
view = createView(null, 1);
el = view.boundElements[0].element;
});
it('should update and remove an attribute', () => {
view.setElementAttribute(0, 'role', 'button');
expect(DOM.getAttribute(el, 'role')).toEqual('button');
view.setElementAttribute(0, 'role', null);
expect(DOM.getAttribute(el, 'role')).toEqual(null);
});
it('should de-normalize attribute names', () => {
view.setElementAttribute(0, 'ariaLabel', 'fancy button');
expect(DOM.getAttribute(el, 'aria-label')).toEqual('fancy button');
});
});
describe('setElementClass', () => {
var el, view;
beforeEach(() => {
view = createView(null, 1);
el = view.boundElements[0].element;
});
it('should set and remove a class', () => {
view.setElementClass(0, 'active', true);
expect(DOM.hasClass(el, 'active')).toEqual(true);
view.setElementClass(0, 'active', false);
expect(DOM.hasClass(el, 'active')).toEqual(false);
});
it('should de-normalize class names', () => {
view.setElementClass(0, 'veryActive', true);
expect(DOM.hasClass(el, 'very-active')).toEqual(true);
view.setElementClass(0, 'veryActive', false);
expect(DOM.hasClass(el, 'very-active')).toEqual(false);
});
});
describe('setElementStyle', () => {
var el, view;
beforeEach(() => {
view = createView(null, 1);
el = view.boundElements[0].element;
});
it('should set and remove styles', () => {
view.setElementStyle(0, 'width', '40px');
expect(DOM.getStyle(el, 'width')).toEqual('40px');
view.setElementStyle(0, 'width', null);
expect(DOM.getStyle(el, 'width')).toEqual('');
});
it('should de-normalize style names', () => {
view.setElementStyle(0, 'textAlign', 'right');
expect(DOM.getStyle(el, 'text-align')).toEqual('right');
view.setElementStyle(0, 'textAlign', null);
expect(DOM.getStyle(el, 'text-align')).toEqual('');
});
});
});
}

View File

@ -18,9 +18,6 @@ void initReflector(reflector) {
]
})
..registerGetters({'b': (o) => o.b, 'greeting': (o) => o.greeting})
..registerSetters({
'b': (o, v) => o.b = v,
'greeting': (o, v) => o.greeting = v,
'a': (o, v) => o.a = v
});
..registerSetters(
{'b': (o, v) => o.b = v, 'greeting': (o, v) => o.greeting = v});
}