Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Commit 3ea2b10

Browse files
juliemrrkirov
authored andcommitted
fix(introspection): fix ngProbe with introspected bindings and findElements returning only elements
This change 1) organizes the introspection unit tests to more clearly separate them into elementProbe tests and testability tests. 2) Modifies `interpolate` to return an `Interpolation` object which contains the interpolated string and a list of binding expressions. 3) Adds an optional third parameter to findElements and findBindings which makes the returned list include only elements or both elements and nodes (in practice, these are text nodes). This defaults to only returning elements.
1 parent 0dbcd6c commit 3ea2b10

File tree

8 files changed

+230
-144
lines changed

8 files changed

+230
-144
lines changed

lib/core/interpolate.dart

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
part of angular.core_internal;
22

3+
class Interpolation {
4+
final String expression;
5+
final List<String> bindingExpressions;
6+
7+
Interpolation(this.expression, this.bindingExpressions);
8+
}
9+
10+
final EMPTY_INTERPOLATION = new Interpolation("", <String>[]);
11+
312
/**
413
* Compiles a string with markup into an expression. This service is used by the
514
* HTML [Compiler] service for data binding.
@@ -15,6 +24,7 @@ class Interpolate implements Function {
1524
Interpolate(CacheRegister cacheRegister) {
1625
cacheRegister.registerCache("Interpolate", _cache);
1726
}
27+
1828
/**
1929
* Compiles markup text into expression.
2030
*
@@ -25,19 +35,18 @@ class Interpolate implements Function {
2535
* - [startSymbol]: The symbol to start interpolation. '{{' by default.
2636
* - [endSymbol]: The symbol to end interpolation. '}}' by default.
2737
*/
28-
29-
String call(String template, [bool mustHaveExpression = false,
30-
String startSymbol = '{{', String endSymbol = '}}']) {
38+
Interpolation call(String template, [bool mustHaveExpression = false,
39+
String startSymbol = '{{', String endSymbol = '}}']) {
3140
if (mustHaveExpression == false && startSymbol == '{{' && endSymbol == '}}') {
3241
// cachable
3342
return _cache.putIfAbsent(template, () => _call(template, mustHaveExpression, startSymbol, endSymbol));
3443
}
3544
return _call(template, mustHaveExpression, startSymbol, endSymbol);
3645
}
3746

38-
String _call(String template, [bool mustHaveExpression = false,
39-
String startSymbol, String endSymbol]) {
40-
if (template == null || template.isEmpty) return "";
47+
Interpolation _call(String template, [bool mustHaveExpression = false,
48+
String startSymbol, String endSymbol]) {
49+
if (template == null || template.isEmpty) return EMPTY_INTERPOLATION;
4150

4251
final startLen = startSymbol.length;
4352
final endLen = endSymbol.length;
@@ -51,6 +60,7 @@ class Interpolate implements Function {
5160

5261
String exp;
5362
final expParts = <String>[];
63+
final bindings = <String>[];
5464

5565
while (index < length) {
5666
startIdx = template.indexOf(startSymbol, index);
@@ -61,7 +71,9 @@ class Interpolate implements Function {
6171
// formatter
6272
expParts.add(_wrapInQuotes(template.substring(index, startIdx)));
6373
}
64-
expParts.add('(' + template.substring(startIdx + startLen, endIdx) + '|stringify)');
74+
var binding = template.substring(startIdx + startLen, endIdx);
75+
bindings.add(binding);
76+
expParts.add('(' + binding + '|stringify)');
6577

6678
index = endIdx + endLen;
6779
hasInterpolation = true;
@@ -72,7 +84,8 @@ class Interpolate implements Function {
7284
}
7385
}
7486

75-
return !mustHaveExpression || hasInterpolation ? expParts.join('+') : null;
87+
return !mustHaveExpression || hasInterpolation ?
88+
new Interpolation(expParts.join('+'), bindings) : null;
7689
}
7790

7891
String _wrapInQuotes(String s){

lib/core_dom/common.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ class DirectiveRef {
2929
final String value;
3030
final AST valueAST;
3131
final mappings = <MappingParts>[];
32+
final List<String> bindingExpressions;
3233

33-
DirectiveRef(this.element, type, this.annotation, this.typeKey, [ this.value, this.valueAST ])
34+
DirectiveRef(this.element, type, this.annotation, this.typeKey, [ this.value, this.valueAST, this.bindingExpressions ])
3435
: type = type,
3536
factory = Module.DEFAULT_REFLECTOR.factoryFor(type),
3637
paramKeys = Module.DEFAULT_REFLECTOR.parameterKeysFor(type);

lib/core_dom/element_binder.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,8 @@ class ElementBinder {
296296
DirectiveBinderFn config = ref.annotation.module;
297297
if (config != null) config(nodeInjector);
298298
}
299-
if (_config.elementProbeEnabled && ref.valueAST != null) {
300-
nodeInjector.elementProbe.bindingExpressions.add(ref.valueAST.expression);
299+
if (_config.elementProbeEnabled && ref.bindingExpressions != null) {
300+
nodeInjector.elementProbe.bindingExpressions.addAll(ref.bindingExpressions);
301301
}
302302
}
303303

lib/core_dom/selector.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,10 @@ class DirectiveSelector {
105105
// the value. Yes it is a bit of a hack.
106106
_directives[selectorRegExp.selector].forEach((DirectiveTypeTuple tuple) {
107107
// Pre-compute the AST to watch this value.
108-
String expression = _interpolate(value);
109-
AST valueAST = _astParser(expression, formatters: _formatters);
108+
Interpolation interpolation = _interpolate(value);
109+
AST valueAST = _astParser(interpolation.expression, formatters: _formatters);
110110
builder.addDirective(new DirectiveRef(
111-
node, tuple.type, tuple.directive, new Key(tuple.type), attrName, valueAST));
111+
node, tuple.type, tuple.directive, new Key(tuple.type), attrName, valueAST, interpolation.bindingExpressions));
112112
});
113113
}
114114
}
@@ -143,11 +143,11 @@ class DirectiveSelector {
143143
if (selectorRegExp.regexp.hasMatch(value)) {
144144
_directives[selectorRegExp.selector].forEach((tuple) {
145145
// Pre-compute the AST to watch this value.
146-
String expression = _interpolate(value);
147-
var valueAST = _astParser(expression, formatters: _formatters);
146+
Interpolation interpolation = _interpolate(value);
147+
var valueAST = _astParser(interpolation.expression, formatters: _formatters);
148148

149149
builder.addDirective(new DirectiveRef(node, tuple.type,
150-
tuple.directive, new Key(tuple.type), value, valueAST));
150+
tuple.directive, new Key(tuple.type), value, valueAST, interpolation.bindingExpressions));
151151
});
152152
}
153153
}

lib/directive/ng_pluralize.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ class NgPluralize {
168168
void _setAndWatch(template) {
169169
if (_watch != null) _watch.remove();
170170
var expression = _expressionCache.putIfAbsent(template, () =>
171-
_interpolate(template, false, r'${', '}'));
171+
_interpolate(template, false, r'${', '}').expression);
172172
_watch = _scope.watch(expression, _updateMarkup, formatters: _formatters);
173173
}
174174

lib/introspection.dart

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ List<ElementProbe> _findAllProbesInTree(dom.Node node) {
7373
}
7474

7575

76+
dom.Element _nearestElementAncestory(dom.Node node) {
77+
if (node.nodeType == dom.Node.ELEMENT_NODE) {
78+
return node;
79+
} else {
80+
return _nearestElementAncestory(node.parentNode);
81+
}
82+
}
83+
7684
/**
7785
* Return the [ElementProbe] object for the closest [Element] in the hierarchy.
7886
*
@@ -279,30 +287,42 @@ class _Testability implements _JsObjectProxyable {
279287
* Returns a list of all nodes in the selected tree that have an `ng-model`
280288
* binding specified by the [modelString]. If the optional [exactMatch]
281289
* parameter is provided and true, it restricts the searches to bindings that
282-
* are exact matches for [modelString].
290+
* are exact matches for [modelString]. If the optional [allowNonElementNodes]
291+
* parameter is true, returned values will be the nearest parent node which is
292+
* an element.
283293
*/
284-
List<dom.Node> findModels(String modelString, [bool exactMatch]) => _findByExpression(
285-
modelString, exactMatch, (ElementProbe probe) => probe.modelExpressions);
294+
List<dom.Node> findModels(String modelString, [bool exactMatch, bool allowNonElementNodes]) => _findByExpression(
295+
modelString, exactMatch, allowNonElementNodes, (ElementProbe probe) => probe.modelExpressions);
286296

287297
/**
288298
* Returns a list of all nodes in the selected tree that have `ng-bind` or
289299
* mustache bindings specified by the [bindingString]. If the optional
290300
* [exactMatch] parameter is provided and true, it restricts the searches to
291-
* bindings that are exact matches for [bindingString].
301+
* bindings that are exact matches for [bindingString]. If the optional
302+
* [allowNonElementNodes] parameter is true, returned values will be the nearest parent
303+
* node which is an element.
292304
*/
293-
List<dom.Node> findBindings(String bindingString, [bool exactMatch]) => _findByExpression(
294-
bindingString, exactMatch, (ElementProbe probe) => probe.bindingExpressions);
305+
List<dom.Node> findBindings(String bindingString, [bool exactMatch, bool allowNonElementNodes]) => _findByExpression(
306+
bindingString, exactMatch, allowNonElementNodes, (ElementProbe probe) => probe.bindingExpressions);
307+
308+
List<dom.Node> _findByExpression(String query, bool exactMatch, bool allowNonElementNodes, _GetExpressionsFromProbe getExpressions) {
295309

296-
List<dom.Node> _findByExpression(String query, bool exactMatch, _GetExpressionsFromProbe getExpressions) {
297310
List<ElementProbe> probes = _findAllProbesInTree(node);
298311
if (probes.length == 0) {
299312
probes.add(_findProbeWalkingUp(node));
300313
}
301314
List<dom.Node> results = [];
302315
for (ElementProbe probe in probes) {
303316
for (String expression in getExpressions(probe)) {
304-
if(exactMatch == true ? expression == query : expression.indexOf(query) >= 0) {
305-
results.add(probe.element);
317+
if (exactMatch == true ? expression == query : expression.indexOf(query) >= 0) {
318+
if (allowNonElementNodes == true) {
319+
results.add(probe.element);
320+
} else {
321+
var nearestElement = _nearestElementAncestory(probe.element);
322+
if (!results.contains(nearestElement)) {
323+
results.add(nearestElement);
324+
}
325+
}
306326
}
307327
}
308328
}
@@ -319,10 +339,10 @@ class _Testability implements _JsObjectProxyable {
319339
js.JsObject _toJsObject() {
320340
return _jsify({
321341
'allowAnimations': allowAnimations,
322-
'findBindings': (bindingString, [exactMatch]) =>
323-
findBindings(bindingString, exactMatch),
324-
'findModels': (modelExpressions, [exactMatch]) =>
325-
findModels(modelExpressions, exactMatch),
342+
'findBindings': (bindingString, [exactMatch, allowNonElementNodes]) =>
343+
findBindings(bindingString, exactMatch, allowNonElementNodes),
344+
'findModels': (modelExpressions, [exactMatch, allowNonElementNodes]) =>
345+
findModels(modelExpressions, exactMatch, allowNonElementNodes),
326346
'whenStable': (callback) =>
327347
whenStable(() => callback.apply([])),
328348
'notifyWhenNoOutstandingRequests': (callback) {

test/core/interpolate_spec.dart

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,34 @@ main() {
1515
});
1616

1717
it('should return an expression', (Interpolate interpolate) {
18-
expect(interpolate('Hello {{name}}!'))
18+
expect(interpolate('Hello {{name}}!').expression)
1919
.toEqual('"Hello "+(name|stringify)+"!"');
20-
expect(interpolate('a{{b}}C'))
20+
expect(interpolate('a{{b}}C').expression)
2121
.toEqual('"a"+(b|stringify)+"C"');
22-
expect(interpolate('a{{b}}')).toEqual('"a"+(b|stringify)');
23-
expect(interpolate('{{a}}b')).toEqual('(a|stringify)+"b"');
24-
expect(interpolate('{{b}}')).toEqual('(b|stringify)');
25-
expect(interpolate('{{b}}+{{c}}'))
22+
expect(interpolate('a{{b}}').expression).toEqual('"a"+(b|stringify)');
23+
expect(interpolate('{{a}}b').expression).toEqual('(a|stringify)+"b"');
24+
expect(interpolate('{{b}}').expression).toEqual('(b|stringify)');
25+
expect(interpolate('{{b}}+{{c}}').expression)
2626
.toEqual('(b|stringify)+"+"+(c|stringify)');
27-
expect(interpolate('{{b}}x{{c}}'))
27+
expect(interpolate('{{b}}x{{c}}').expression)
2828
.toEqual('(b|stringify)+"x"+(c|stringify)');
2929
});
3030

31+
it('should return expression bindings', (Interpolate interpolate) {
32+
expect(interpolate('Hello {{name}}').bindingExpressions).toEqual(['name']);
33+
expect(interpolate('a{{b}}C').bindingExpressions)
34+
.toEqual(['b']);
35+
expect(interpolate('{{b}}x{{c}}').bindingExpressions).toEqual(['b', 'c']);
36+
});
37+
3138
it('should Parse Multiline', (Interpolate interpolate) {
32-
expect(interpolate("X\nY{{A\n+B}}C\nD"))
39+
expect(interpolate("X\nY{{A\n+B}}C\nD").expression)
3340
.toEqual('"X\nY"+(A\n+B|stringify)+"C\nD"');
3441
});
3542

3643
it('should escape double quotes', (Interpolate interpolate) {
37-
expect(interpolate(r'"{{a}}')).toEqual(r'"\""+(a|stringify)');
38-
expect(interpolate(r'\"{{a}}')).toEqual(r'"\\\""+(a|stringify)');
44+
expect(interpolate(r'"{{a}}').expression).toEqual(r'"\""+(a|stringify)');
45+
expect(interpolate(r'\"{{a}}').expression).toEqual(r'"\\\""+(a|stringify)');
3946
});
4047
});
41-
}
48+
}

0 commit comments

Comments
 (0)