Skip to content

Commit 5926d2e

Browse files
refactor: use attributes for directives matching
Closes angular#940
1 parent d35fdfc commit 5926d2e

File tree

5 files changed

+60
-21
lines changed

5 files changed

+60
-21
lines changed

modules/angular2/src/core/compiler/pipeline/directive_parser.js

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,16 @@ export class DirectiveParser extends CompileStep {
5252
for (var i=0; i < classList.length; i++) {
5353
cssSelector.addClassName(classList[i]);
5454
}
55+
5556
MapWrapper.forEach(attrs, (attrValue, attrName) => {
56-
if (isBlank(current.propertyBindings) ||
57-
isPresent(current.propertyBindings) && !MapWrapper.contains(current.propertyBindings, attrName)) {
58-
cssSelector.addAttribute(attrName, attrValue);
59-
}
57+
cssSelector.addAttribute(attrName, attrValue);
6058
});
61-
if (isPresent(current.propertyBindings)) {
62-
MapWrapper.forEach(current.propertyBindings, (expression, prop) => {
63-
cssSelector.addAttribute(prop, expression.source);
64-
});
65-
}
66-
if (isPresent(current.variableBindings)) {
67-
MapWrapper.forEach(current.variableBindings, (value, name) => {
68-
cssSelector.addAttribute(name, value);
69-
});
70-
}
59+
7160
// Note: We assume that the ViewSplitter already did its work, i.e. template directive should
7261
// only be present on <template> elements any more!
7362
var isTemplateElement = DOM.isTemplateElement(current.element);
7463
var matchedProperties; // StringMap - used in dev mode to store all properties that have been matched
75-
64+
7665
this._selectorMatcher.match(cssSelector, (selector, directive) => {
7766
matchedProperties = updateMatchedProperties(matchedProperties, selector, directive);
7867
checkDirectiveValidity(directive, current, isTemplateElement);
@@ -102,7 +91,7 @@ function updateMatchedProperties(matchedProperties, selector, directive) {
10291
if (isPresent(directive.annotation) && isPresent(directive.annotation.bind)) {
10392
var bindMap = directive.annotation.bind;
10493
StringMapWrapper.forEach(bindMap, (value, key) => {
105-
// value is the name of the property that is intepreted
94+
// value is the name of the property that is interpreted
10695
// e.g. 'myprop' or 'myprop | double' when a pipe is used to transform the property
10796

10897
// keep the property name and remove the pipe
@@ -142,7 +131,7 @@ function checkMissingDirectives(current, matchedProperties, isTemplateElement) {
142131
if (!DOM.hasProperty(current.element, prop) && !isSpecialProperty(prop)) {
143132
if (!isPresent(matchedProperties) || !isPresent(StringMapWrapper.get(matchedProperties, prop))) {
144133
throw new BaseException(`Missing directive to handle '${prop}' in ${current.elementDescription}`);
145-
}
134+
}
146135
}
147136
});
148137
}

modules/angular2/src/core/compiler/pipeline/property_binding_parser.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,27 +40,32 @@ export class PropertyBindingParser extends CompileStep {
4040
}
4141

4242
var attrs = current.attrs();
43+
var newAttrs = MapWrapper.create();
4344
var desc = current.elementDescription;
45+
4446
MapWrapper.forEach(attrs, (attrValue, attrName) => {
4547
var bindParts = RegExpWrapper.firstMatch(BIND_NAME_REGEXP, attrName);
4648
if (isPresent(bindParts)) {
4749
if (isPresent(bindParts[1])) {
4850
// match: bind-prop
4951
current.addPropertyBinding(bindParts[4], this._parseBinding(attrValue, desc));
52+
MapWrapper.set(newAttrs, bindParts[4], attrValue);
5053
} else if (isPresent(bindParts[2]) || isPresent(bindParts[7])) {
5154
// match: var-name / var-name="iden" / #name / #name="iden"
5255
var identifier = (isPresent(bindParts[4]) && bindParts[4] !== '') ?
5356
bindParts[4] : bindParts[8];
5457
var value = attrValue == '' ? '\$implicit' : attrValue;
5558
current.addVariableBinding(identifier, value);
59+
MapWrapper.set(newAttrs, identifier, value);
5660
} else if (isPresent(bindParts[3])) {
57-
// match: on-prop
61+
// match: on-event
5862
current.addEventBinding(bindParts[4], this._parseAction(attrValue, desc));
5963
} else if (isPresent(bindParts[5])) {
6064
// match: [prop]
6165
current.addPropertyBinding(bindParts[5], this._parseBinding(attrValue, desc));
66+
MapWrapper.set(newAttrs, bindParts[5], attrValue);
6267
} else if (isPresent(bindParts[6])) {
63-
// match: (prop)
68+
// match: (event)
6469
current.addEventBinding(bindParts[6], this._parseBinding(attrValue, desc));
6570
}
6671
} else {
@@ -70,6 +75,10 @@ export class PropertyBindingParser extends CompileStep {
7075
}
7176
}
7277
});
78+
79+
MapWrapper.forEach(newAttrs, (attrValue, attrName) => {
80+
MapWrapper.set(attrs, attrName, attrValue);
81+
});
7382
}
7483

7584
_parseInterpolation(input:string, location:string):AST {

modules/angular2/src/core/compiler/pipeline/view_splitter.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,10 @@ export class ViewSplitter extends CompileStep {
109109
var binding = bindings[i];
110110
if (binding.keyIsVar) {
111111
compileElement.addVariableBinding(binding.key, binding.name);
112+
MapWrapper.set(compileElement.attrs(), binding.key, binding.name);
112113
} else if (isPresent(binding.expression)) {
113114
compileElement.addPropertyBinding(binding.key, binding.expression);
115+
MapWrapper.set(compileElement.attrs(), binding.key, binding.expression.source);
114116
} else {
115117
DOM.setAttribute(compileElement.element, binding.key, '');
116118
}

modules/angular2/test/core/compiler/integration_spec.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,30 @@ export function main() {
243243
});
244244
});
245245

246+
it('should support directives where a selector matches property binding', function(done) {
247+
tplResolver.setTemplate(MyComp,
248+
new Template({
249+
inline: '<p [id]="ctxProp"></p>',
250+
directives: [IdComponent]
251+
}));
252+
253+
compiler.compile(MyComp).then((pv) => {
254+
createView(pv);
255+
256+
ctx.ctxProp = 'some_id';
257+
cd.detectChanges();
258+
expect(view.nodes[0].id).toEqual('some_id');
259+
expect(DOM.getInnerHTML(view.nodes[0].shadowRoot.childNodes[0])).toEqual('Matched on id with some_id');
260+
261+
ctx.ctxProp = 'other_id';
262+
cd.detectChanges();
263+
expect(view.nodes[0].id).toEqual('other_id');
264+
expect(DOM.getInnerHTML(view.nodes[0].shadowRoot.childNodes[0])).toEqual('Matched on id with other_id');
265+
266+
done();
267+
});
268+
});
269+
246270
it('should support template directives via `<template>` elements.', (done) => {
247271
tplResolver.setTemplate(MyComp,
248272
new Template({
@@ -714,3 +738,14 @@ class DecoratorListeningEvent {
714738
this.msg = msg;
715739
}
716740
}
741+
742+
@Component({
743+
selector: '[id]',
744+
bind: {'id': 'id'}
745+
})
746+
@Template({
747+
inline: '<div>Matched on id with {{id}}</div>'
748+
})
749+
class IdComponent {
750+
id: string;
751+
}

modules/angular2/test/core/compiler/pipeline/directive_parser_spec.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,14 @@ export function main() {
3939
if (isPresent(propertyBindings)) {
4040
StringMapWrapper.forEach(propertyBindings, (v, k) => {
4141
current.addPropertyBinding(k, parser.parseBinding(v, null));
42+
MapWrapper.set(current.attrs(), k, v);
4243
});
4344
}
4445
if (isPresent(variableBindings)) {
45-
current.variableBindings = MapWrapper.createFromStringMap(variableBindings);
46+
StringMapWrapper.forEach(variableBindings, (v, k) => {
47+
current.addVariableBinding(k, v);
48+
MapWrapper.set(current.attrs(), k, v);
49+
});
4650
}
4751
}), new DirectiveParser(annotatedDirectives)]);
4852
}
@@ -89,7 +93,7 @@ export function main() {
8993
createPipeline().process(
9094
el('<div some-comp some-comp2></div>')
9195
);
92-
}).toThrowError('Multiple component directives not allowed on the same element - check <div some-comp some-comp2>');
96+
}).toThrowError('Multiple component directives not allowed on the same element - check <div some-comp some-comp2>');
9397
});
9498

9599
it('should not allow component directives on <template> elements', () => {

0 commit comments

Comments
 (0)