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

Commit aaae1d6

Browse files
vicbmhevery
vicb
authored and
mhevery
committed
fix(NodeCursor): Removes nodeList() in favor of current
Closes #644
1 parent 88d2af6 commit aaae1d6

File tree

4 files changed

+77
-105
lines changed

4 files changed

+77
-105
lines changed

‎lib/core_dom/common.dart‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@ class DirectiveRef {
1919
DirectiveRef(this.element, this.type, this.annotation, [ this.value ]);
2020

2121
String toString() {
22-
var html = element is dom.Element ? (element as dom.Element).outerHtml : element.nodeValue;
23-
return '{ element: $html, selector: ${annotation.selector}, value: $value, type: $type }';
22+
var html = element is dom.Element
23+
? (element as dom.Element).outerHtml
24+
: element.nodeValue;
25+
return '{ element: $html, selector: ${annotation.selector}, value: $value, '
26+
'type: $type }';
2427
}
2528
}
2629

@@ -29,10 +32,12 @@ class DirectiveRef {
2932
* services from the provided modules.
3033
*/
3134
Injector forceNewDirectivesAndFilters(Injector injector, List<Module> modules) {
32-
modules.add(new Module()..factory(Scope, (i) {
33-
var scope = i.parent.get(Scope);
34-
return scope.createChild(new PrototypeMap(scope.context));
35-
}));
35+
modules.add(new Module()
36+
..factory(Scope, (i) {
37+
var scope = i.parent.get(Scope);
38+
return scope.createChild(new PrototypeMap(scope.context));
39+
}));
40+
3641
return injector.createChild(modules,
3742
forceNewInstances: [DirectiveMap, FilterMap]);
3843
}

‎lib/core_dom/compiler.dart‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@ class Compiler {
99
Compiler(this._perf, this._parser, this._expando);
1010

1111
_compileBlock(NodeCursor domCursor, NodeCursor templateCursor,
12-
List<DirectiveRef> useExistingDirectiveRefs,
12+
List<DirectiveRef> existingDirectiveRefs,
1313
DirectiveMap directives) {
14-
if (domCursor.nodeList().length == 0) return null;
14+
if (domCursor.current == null) return null;
1515

1616
var directivePositions = null; // don't pre-create to create sparse tree and prevent GC pressure.
1717
var cursorAlreadyAdvanced;
1818

1919
do {
20-
var declaredDirectiveRefs = useExistingDirectiveRefs == null
21-
? directives.selector(domCursor.nodeList()[0])
22-
: useExistingDirectiveRefs;
20+
var declaredDirectiveRefs = existingDirectiveRefs == null
21+
? directives.selector(domCursor.current)
22+
: existingDirectiveRefs;
2323
var children = NgAnnotation.COMPILE_CHILDREN;
2424
var childDirectivePositions = null;
2525
List<DirectiveRef> usableDirectiveRefs = null;
@@ -73,7 +73,7 @@ class Compiler {
7373
..add(usableDirectiveRefs)
7474
..add(childDirectivePositions);
7575
}
76-
} while (templateCursor.microNext() && domCursor.microNext());
76+
} while (templateCursor.moveNext() && domCursor.moveNext());
7777

7878
return directivePositions;
7979
}
@@ -83,27 +83,28 @@ class Compiler {
8383
DirectiveRef directiveRef,
8484
List<DirectiveRef> transcludedDirectiveRefs,
8585
DirectiveMap directives) {
86-
var anchorName = directiveRef.annotation.selector + (directiveRef.value != null ? '=' + directiveRef.value : '');
86+
var anchorName = directiveRef.annotation.selector +
87+
(directiveRef.value != null ? '=' + directiveRef.value : '');
8788
var blockFactory;
8889
var blocks;
8990

9091
var transcludeCursor = templateCursor.replaceWithAnchor(anchorName);
9192
var domCursorIndex = domCursor.index;
9293
var directivePositions =
93-
_compileBlock(domCursor, transcludeCursor, transcludedDirectiveRefs, directives);
94+
_compileBlock(domCursor, transcludeCursor, transcludedDirectiveRefs,
95+
directives);
9496
if (directivePositions == null) directivePositions = [];
9597

96-
blockFactory = new BlockFactory(transcludeCursor.elements, directivePositions, _perf, _expando);
98+
blockFactory = new BlockFactory(transcludeCursor.elements,
99+
directivePositions, _perf, _expando);
97100
domCursor.index = domCursorIndex;
98101

99-
if (domCursor.isInstance()) {
102+
if (domCursor.isInstance) {
100103
domCursor.insertAnchorBefore(anchorName);
101-
blocks = [blockFactory(domCursor.nodeList())];
102-
domCursor.macroNext();
103-
templateCursor.macroNext();
104-
while (domCursor.isValid() && domCursor.isInstance()) {
105-
blocks.add(blockFactory(domCursor.nodeList()));
106-
domCursor.macroNext();
104+
blocks = [blockFactory([domCursor.current])];
105+
templateCursor.moveNext();
106+
while (domCursor.moveNext() && domCursor.isInstance) {
107+
blocks.add(blockFactory([domCursor.current]));
107108
templateCursor.remove();
108109
}
109110
} else {
@@ -116,8 +117,8 @@ class Compiler {
116117
BlockFactory call(List<dom.Node> elements, DirectiveMap directives) {
117118
var timerId;
118119
assert((timerId = _perf.startTimer('ng.compile', _html(elements))) != false);
119-
List<dom.Node> domElements = elements;
120-
List<dom.Node> templateElements = cloneElements(domElements);
120+
final domElements = elements;
121+
final templateElements = cloneElements(domElements);
121122
var directivePositions = _compileBlock(
122123
new NodeCursor(domElements), new NodeCursor(templateElements),
123124
null, directives);
@@ -144,20 +145,23 @@ class Compiler {
144145
String dstExpression = dstPath.isEmpty ? attrName : dstPath;
145146
Expression dstPathFn = _parser(dstExpression);
146147
if (!dstPathFn.isAssignable) {
147-
throw "Expression '$dstPath' is not assignable in mapping '$mapping' for attribute '$attrName'.";
148+
throw "Expression '$dstPath' is not assignable in mapping '$mapping' "
149+
"for attribute '$attrName'.";
148150
}
149151
ApplyMapping mappingFn;
150152
switch (mode) {
151153
case '@':
152-
mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) {
154+
mappingFn = (NodeAttrs attrs, Scope scope, Object controller,
155+
FilterMap filters, notify()) {
153156
attrs.observe(attrName, (value) {
154157
dstPathFn.assign(controller, value);
155158
notify();
156159
});
157160
};
158161
break;
159162
case '<=>':
160-
mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) {
163+
mappingFn = (NodeAttrs attrs, Scope scope, Object controller,
164+
FilterMap filters, notify()) {
161165
if (attrs[attrName] == null) return notify();
162166
String expression = attrs[attrName];
163167
Expression expressionFn = _parser(expression);
@@ -194,7 +198,8 @@ class Compiler {
194198
};
195199
break;
196200
case '=>':
197-
mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) {
201+
mappingFn = (NodeAttrs attrs, Scope scope, Object controller,
202+
FilterMap filters, notify()) {
198203
if (attrs[attrName] == null) return notify();
199204
Expression attrExprFn = _parser(attrs[attrName]);
200205
var shadowValue = null;
@@ -207,7 +212,8 @@ class Compiler {
207212
};
208213
break;
209214
case '=>!':
210-
mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) {
215+
mappingFn = (NodeAttrs attrs, Scope scope, Object controller,
216+
FilterMap filters, notify()) {
211217
if (attrs[attrName] == null) return notify();
212218
Expression attrExprFn = _parser(attrs[attrName]);
213219
var watch;
@@ -223,8 +229,10 @@ class Compiler {
223229
};
224230
break;
225231
case '&':
226-
mappingFn = (NodeAttrs attrs, Scope scope, Object dst, FilterMap filters, notify()) {
227-
dstPathFn.assign(dst, _parser(attrs[attrName]).bind(scope.context, ScopeLocals.wrapper));
232+
mappingFn = (NodeAttrs attrs, Scope scope, Object dst,
233+
FilterMap filters, notify()) {
234+
dstPathFn.assign(dst, _parser(attrs[attrName])
235+
.bind(scope.context, ScopeLocals.wrapper));
228236
notify();
229237
};
230238
break;

‎lib/core_dom/node_cursor.dart‎

Lines changed: 14 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,90 +1,49 @@
11
part of angular.core.dom;
22

33
class NodeCursor {
4-
List<dynamic> stack = [];
4+
final stack = [];
55
List<dom.Node> elements;
6-
int index;
6+
int index = 0;
77

8-
NodeCursor(this.elements) : index = 0;
8+
NodeCursor(this.elements);
99

10-
isValid() => index < elements.length;
10+
bool moveNext() => ++index < elements.length;
1111

12-
cursorSize() => 1;
12+
dom.Node get current => index < elements.length ? elements[index] : null;
1313

14-
macroNext() {
15-
for (var i = 0, ii = cursorSize(); i < ii; i++, index++){}
16-
17-
return this.isValid();
18-
}
19-
20-
microNext() {
21-
var length = elements.length;
22-
23-
if (index < length) {
24-
index++;
25-
}
26-
27-
return index < length;
28-
}
29-
30-
nodeList() {
31-
if (!isValid()) return []; // or should we return null?
32-
33-
return elements.sublist(index, index + cursorSize());
34-
}
35-
36-
descend() {
14+
bool descend() {
3715
var childNodes = elements[index].nodes;
38-
var hasChildren = childNodes != null && childNodes.length > 0;
16+
var hasChildren = childNodes != null && childNodes.isNotEmpty;
3917

4018
if (hasChildren) {
41-
stack.add(index);
42-
stack.add(elements);
19+
stack..add(index)..add(elements);
4320
elements = new List.from(childNodes);
4421
index = 0;
4522
}
4623

4724
return hasChildren;
4825
}
4926

50-
ascend() {
27+
void ascend() {
5128
elements = stack.removeLast();
5229
index = stack.removeLast();
5330
}
5431

55-
insertAnchorBefore(String name) {
56-
var current = elements[index];
32+
void insertAnchorBefore(String name) {
5733
var parent = current.parentNode;
58-
5934
var anchor = new dom.Comment('ANCHOR: $name');
60-
6135
elements.insert(index++, anchor);
62-
63-
if (parent != null) {
64-
parent.insertBefore(anchor, current);
65-
}
36+
if (parent != null) parent.insertBefore(anchor, current);
6637
}
6738

68-
replaceWithAnchor(String name) {
39+
NodeCursor replaceWithAnchor(String name) {
6940
insertAnchorBefore(name);
7041
var childCursor = remove();
7142
this.index--;
7243
return childCursor;
7344
}
7445

75-
remove() {
76-
var nodes = nodeList();
77-
78-
for (var i = 0; i < nodes.length; i++) {
79-
// NOTE(deboer): If elements is a list of child nodes on a node, then
80-
// calling Node.remove() may also remove it from the list. Thus, we
81-
// call elements.removeAt first so only one node is removed.
82-
elements.removeAt(index);
83-
nodes[i].remove();
84-
}
85-
86-
return new NodeCursor(nodes);
87-
}
46+
NodeCursor remove() => new NodeCursor([elements.removeAt(index)..remove()]);
8847

89-
isInstance() => false;
48+
bool get isInstance => false;
9049
}

‎test/core_dom/node_cursor_spec.dart‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,25 @@ main() {
1919
it('should allow single level traversal', () {
2020
var cursor = new NodeCursor([a, b]);
2121

22-
expect(cursor.nodeList(), equals([a]));
23-
expect(cursor.microNext(), equals(true));
24-
expect(cursor.nodeList(), equals([b]));
25-
expect(cursor.microNext(), equals(false));
22+
expect(cursor.current, equals(a));
23+
expect(cursor.moveNext(), equals(true));
24+
expect(cursor.current, equals(b));
25+
expect(cursor.moveNext(), equals(false));
2626
});
2727

2828

2929
it('should descend and ascend', () {
3030
var cursor = new NodeCursor([d, c]);
3131

3232
expect(cursor.descend(), equals(true));
33-
expect(cursor.nodeList(), equals([a]));
34-
expect(cursor.microNext(), equals(true));
35-
expect(cursor.nodeList(), equals([b]));
36-
expect(cursor.microNext(), equals(false));
33+
expect(cursor.current, equals(a));
34+
expect(cursor.moveNext(), equals(true));
35+
expect(cursor.current, equals(b));
36+
expect(cursor.moveNext(), equals(false));
3737
cursor.ascend();
38-
expect(cursor.microNext(), equals(true));
39-
expect(cursor.nodeList(), equals([c]));
40-
expect(cursor.microNext(), equals(false));
38+
expect(cursor.moveNext(), equals(true));
39+
expect(cursor.current, equals(c));
40+
expect(cursor.moveNext(), equals(false));
4141
});
4242

4343
it('should descend and ascend two levels', () {
@@ -51,17 +51,17 @@ main() {
5151
var cursor = new NodeCursor([l1, c]);
5252

5353
expect(cursor.descend(), equals(true));
54-
expect(cursor.nodeList(), equals([l2]));
54+
expect(cursor.current, equals(l2));
5555
expect(cursor.descend(), equals(true));
56-
expect(cursor.nodeList(), equals([e]));
56+
expect(cursor.current, equals(e));
5757
cursor.ascend();
58-
expect(cursor.microNext(), equals(true));
59-
expect(cursor.nodeList(), equals([f]));
60-
expect(cursor.microNext(), equals(false));
58+
expect(cursor.moveNext(), equals(true));
59+
expect(cursor.current, equals(f));
60+
expect(cursor.moveNext(), equals(false));
6161
cursor.ascend();
62-
expect(cursor.microNext(), equals(true));
63-
expect(cursor.nodeList(), equals([c]));
64-
expect(cursor.microNext(), equals(false));
62+
expect(cursor.moveNext(), equals(true));
63+
expect(cursor.current, equals(c));
64+
expect(cursor.moveNext(), equals(false));
6565
});
6666

6767

0 commit comments

Comments
 (0)