From aaae1d60832b331be62b0fd94c65935ce68b2856 Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 3 Mar 2014 16:57:25 +0100 Subject: [PATCH] fix(NodeCursor): Removes nodeList() in favor of current Closes #644 --- lib/core_dom/common.dart | 17 ++++--- lib/core_dom/compiler.dart | 58 +++++++++++++----------- lib/core_dom/node_cursor.dart | 69 ++++++----------------------- test/core_dom/node_cursor_spec.dart | 38 ++++++++-------- 4 files changed, 77 insertions(+), 105 deletions(-) diff --git a/lib/core_dom/common.dart b/lib/core_dom/common.dart index 47e004d0f..b76ce1afe 100644 --- a/lib/core_dom/common.dart +++ b/lib/core_dom/common.dart @@ -19,8 +19,11 @@ class DirectiveRef { DirectiveRef(this.element, this.type, this.annotation, [ this.value ]); String toString() { - var html = element is dom.Element ? (element as dom.Element).outerHtml : element.nodeValue; - return '{ element: $html, selector: ${annotation.selector}, value: $value, type: $type }'; + var html = element is dom.Element + ? (element as dom.Element).outerHtml + : element.nodeValue; + return '{ element: $html, selector: ${annotation.selector}, value: $value, ' + 'type: $type }'; } } @@ -29,10 +32,12 @@ class DirectiveRef { * services from the provided modules. */ Injector forceNewDirectivesAndFilters(Injector injector, List modules) { - modules.add(new Module()..factory(Scope, (i) { - var scope = i.parent.get(Scope); - return scope.createChild(new PrototypeMap(scope.context)); - })); + modules.add(new Module() + ..factory(Scope, (i) { + var scope = i.parent.get(Scope); + return scope.createChild(new PrototypeMap(scope.context)); + })); + return injector.createChild(modules, forceNewInstances: [DirectiveMap, FilterMap]); } diff --git a/lib/core_dom/compiler.dart b/lib/core_dom/compiler.dart index ad7f3c786..45b11bdca 100644 --- a/lib/core_dom/compiler.dart +++ b/lib/core_dom/compiler.dart @@ -9,17 +9,17 @@ class Compiler { Compiler(this._perf, this._parser, this._expando); _compileBlock(NodeCursor domCursor, NodeCursor templateCursor, - List useExistingDirectiveRefs, + List existingDirectiveRefs, DirectiveMap directives) { - if (domCursor.nodeList().length == 0) return null; + if (domCursor.current == null) return null; var directivePositions = null; // don't pre-create to create sparse tree and prevent GC pressure. var cursorAlreadyAdvanced; do { - var declaredDirectiveRefs = useExistingDirectiveRefs == null - ? directives.selector(domCursor.nodeList()[0]) - : useExistingDirectiveRefs; + var declaredDirectiveRefs = existingDirectiveRefs == null + ? directives.selector(domCursor.current) + : existingDirectiveRefs; var children = NgAnnotation.COMPILE_CHILDREN; var childDirectivePositions = null; List usableDirectiveRefs = null; @@ -73,7 +73,7 @@ class Compiler { ..add(usableDirectiveRefs) ..add(childDirectivePositions); } - } while (templateCursor.microNext() && domCursor.microNext()); + } while (templateCursor.moveNext() && domCursor.moveNext()); return directivePositions; } @@ -83,27 +83,28 @@ class Compiler { DirectiveRef directiveRef, List transcludedDirectiveRefs, DirectiveMap directives) { - var anchorName = directiveRef.annotation.selector + (directiveRef.value != null ? '=' + directiveRef.value : ''); + var anchorName = directiveRef.annotation.selector + + (directiveRef.value != null ? '=' + directiveRef.value : ''); var blockFactory; var blocks; var transcludeCursor = templateCursor.replaceWithAnchor(anchorName); var domCursorIndex = domCursor.index; var directivePositions = - _compileBlock(domCursor, transcludeCursor, transcludedDirectiveRefs, directives); + _compileBlock(domCursor, transcludeCursor, transcludedDirectiveRefs, + directives); if (directivePositions == null) directivePositions = []; - blockFactory = new BlockFactory(transcludeCursor.elements, directivePositions, _perf, _expando); + blockFactory = new BlockFactory(transcludeCursor.elements, + directivePositions, _perf, _expando); domCursor.index = domCursorIndex; - if (domCursor.isInstance()) { + if (domCursor.isInstance) { domCursor.insertAnchorBefore(anchorName); - blocks = [blockFactory(domCursor.nodeList())]; - domCursor.macroNext(); - templateCursor.macroNext(); - while (domCursor.isValid() && domCursor.isInstance()) { - blocks.add(blockFactory(domCursor.nodeList())); - domCursor.macroNext(); + blocks = [blockFactory([domCursor.current])]; + templateCursor.moveNext(); + while (domCursor.moveNext() && domCursor.isInstance) { + blocks.add(blockFactory([domCursor.current])); templateCursor.remove(); } } else { @@ -116,8 +117,8 @@ class Compiler { BlockFactory call(List elements, DirectiveMap directives) { var timerId; assert((timerId = _perf.startTimer('ng.compile', _html(elements))) != false); - List domElements = elements; - List templateElements = cloneElements(domElements); + final domElements = elements; + final templateElements = cloneElements(domElements); var directivePositions = _compileBlock( new NodeCursor(domElements), new NodeCursor(templateElements), null, directives); @@ -144,12 +145,14 @@ class Compiler { String dstExpression = dstPath.isEmpty ? attrName : dstPath; Expression dstPathFn = _parser(dstExpression); if (!dstPathFn.isAssignable) { - throw "Expression '$dstPath' is not assignable in mapping '$mapping' for attribute '$attrName'."; + throw "Expression '$dstPath' is not assignable in mapping '$mapping' " + "for attribute '$attrName'."; } ApplyMapping mappingFn; switch (mode) { case '@': - mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) { + mappingFn = (NodeAttrs attrs, Scope scope, Object controller, + FilterMap filters, notify()) { attrs.observe(attrName, (value) { dstPathFn.assign(controller, value); notify(); @@ -157,7 +160,8 @@ class Compiler { }; break; case '<=>': - mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) { + mappingFn = (NodeAttrs attrs, Scope scope, Object controller, + FilterMap filters, notify()) { if (attrs[attrName] == null) return notify(); String expression = attrs[attrName]; Expression expressionFn = _parser(expression); @@ -194,7 +198,8 @@ class Compiler { }; break; case '=>': - mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) { + mappingFn = (NodeAttrs attrs, Scope scope, Object controller, + FilterMap filters, notify()) { if (attrs[attrName] == null) return notify(); Expression attrExprFn = _parser(attrs[attrName]); var shadowValue = null; @@ -207,7 +212,8 @@ class Compiler { }; break; case '=>!': - mappingFn = (NodeAttrs attrs, Scope scope, Object controller, FilterMap filters, notify()) { + mappingFn = (NodeAttrs attrs, Scope scope, Object controller, + FilterMap filters, notify()) { if (attrs[attrName] == null) return notify(); Expression attrExprFn = _parser(attrs[attrName]); var watch; @@ -223,8 +229,10 @@ class Compiler { }; break; case '&': - mappingFn = (NodeAttrs attrs, Scope scope, Object dst, FilterMap filters, notify()) { - dstPathFn.assign(dst, _parser(attrs[attrName]).bind(scope.context, ScopeLocals.wrapper)); + mappingFn = (NodeAttrs attrs, Scope scope, Object dst, + FilterMap filters, notify()) { + dstPathFn.assign(dst, _parser(attrs[attrName]) + .bind(scope.context, ScopeLocals.wrapper)); notify(); }; break; diff --git a/lib/core_dom/node_cursor.dart b/lib/core_dom/node_cursor.dart index 2405fe0a4..c57f1da79 100644 --- a/lib/core_dom/node_cursor.dart +++ b/lib/core_dom/node_cursor.dart @@ -1,45 +1,22 @@ part of angular.core.dom; class NodeCursor { - List stack = []; + final stack = []; List elements; - int index; + int index = 0; - NodeCursor(this.elements) : index = 0; + NodeCursor(this.elements); - isValid() => index < elements.length; + bool moveNext() => ++index < elements.length; - cursorSize() => 1; + dom.Node get current => index < elements.length ? elements[index] : null; - macroNext() { - for (var i = 0, ii = cursorSize(); i < ii; i++, index++){} - - return this.isValid(); - } - - microNext() { - var length = elements.length; - - if (index < length) { - index++; - } - - return index < length; - } - - nodeList() { - if (!isValid()) return []; // or should we return null? - - return elements.sublist(index, index + cursorSize()); - } - - descend() { + bool descend() { var childNodes = elements[index].nodes; - var hasChildren = childNodes != null && childNodes.length > 0; + var hasChildren = childNodes != null && childNodes.isNotEmpty; if (hasChildren) { - stack.add(index); - stack.add(elements); + stack..add(index)..add(elements); elements = new List.from(childNodes); index = 0; } @@ -47,44 +24,26 @@ class NodeCursor { return hasChildren; } - ascend() { + void ascend() { elements = stack.removeLast(); index = stack.removeLast(); } - insertAnchorBefore(String name) { - var current = elements[index]; + void insertAnchorBefore(String name) { var parent = current.parentNode; - var anchor = new dom.Comment('ANCHOR: $name'); - elements.insert(index++, anchor); - - if (parent != null) { - parent.insertBefore(anchor, current); - } + if (parent != null) parent.insertBefore(anchor, current); } - replaceWithAnchor(String name) { + NodeCursor replaceWithAnchor(String name) { insertAnchorBefore(name); var childCursor = remove(); this.index--; return childCursor; } - remove() { - var nodes = nodeList(); - - for (var i = 0; i < nodes.length; i++) { - // NOTE(deboer): If elements is a list of child nodes on a node, then - // calling Node.remove() may also remove it from the list. Thus, we - // call elements.removeAt first so only one node is removed. - elements.removeAt(index); - nodes[i].remove(); - } - - return new NodeCursor(nodes); - } + NodeCursor remove() => new NodeCursor([elements.removeAt(index)..remove()]); - isInstance() => false; + bool get isInstance => false; } diff --git a/test/core_dom/node_cursor_spec.dart b/test/core_dom/node_cursor_spec.dart index e15080429..5a1c46fab 100644 --- a/test/core_dom/node_cursor_spec.dart +++ b/test/core_dom/node_cursor_spec.dart @@ -19,10 +19,10 @@ main() { it('should allow single level traversal', () { var cursor = new NodeCursor([a, b]); - expect(cursor.nodeList(), equals([a])); - expect(cursor.microNext(), equals(true)); - expect(cursor.nodeList(), equals([b])); - expect(cursor.microNext(), equals(false)); + expect(cursor.current, equals(a)); + expect(cursor.moveNext(), equals(true)); + expect(cursor.current, equals(b)); + expect(cursor.moveNext(), equals(false)); }); @@ -30,14 +30,14 @@ main() { var cursor = new NodeCursor([d, c]); expect(cursor.descend(), equals(true)); - expect(cursor.nodeList(), equals([a])); - expect(cursor.microNext(), equals(true)); - expect(cursor.nodeList(), equals([b])); - expect(cursor.microNext(), equals(false)); + expect(cursor.current, equals(a)); + expect(cursor.moveNext(), equals(true)); + expect(cursor.current, equals(b)); + expect(cursor.moveNext(), equals(false)); cursor.ascend(); - expect(cursor.microNext(), equals(true)); - expect(cursor.nodeList(), equals([c])); - expect(cursor.microNext(), equals(false)); + expect(cursor.moveNext(), equals(true)); + expect(cursor.current, equals(c)); + expect(cursor.moveNext(), equals(false)); }); it('should descend and ascend two levels', () { @@ -51,17 +51,17 @@ main() { var cursor = new NodeCursor([l1, c]); expect(cursor.descend(), equals(true)); - expect(cursor.nodeList(), equals([l2])); + expect(cursor.current, equals(l2)); expect(cursor.descend(), equals(true)); - expect(cursor.nodeList(), equals([e])); + expect(cursor.current, equals(e)); cursor.ascend(); - expect(cursor.microNext(), equals(true)); - expect(cursor.nodeList(), equals([f])); - expect(cursor.microNext(), equals(false)); + expect(cursor.moveNext(), equals(true)); + expect(cursor.current, equals(f)); + expect(cursor.moveNext(), equals(false)); cursor.ascend(); - expect(cursor.microNext(), equals(true)); - expect(cursor.nodeList(), equals([c])); - expect(cursor.microNext(), equals(false)); + expect(cursor.moveNext(), equals(true)); + expect(cursor.current, equals(c)); + expect(cursor.moveNext(), equals(false)); });