From 89b3995756587cca4ebea66daefbe69c9a58a573 Mon Sep 17 00:00:00 2001
From: Marc Laval <mlaval@amadeus.com>
Date: Thu, 19 Feb 2015 11:18:23 +0100
Subject: [PATCH] refactor(core): remove direct accesses to DOM

Closes #713
---
 .../compiler/pipeline/compile_pipeline.js     |  6 ++--
 .../compiler/pipeline/directive_parser.js     |  6 ++--
 .../pipeline/text_interpolation_parser.js     |  8 ++---
 .../core/compiler/pipeline/view_splitter.js   | 14 ++++----
 modules/angular2/src/core/compiler/view.js    |  8 ++---
 modules/angular2/src/facade/dom.dart          | 19 +++++++++++
 modules/angular2/src/facade/dom.es6           | 33 +++++++++++++++++++
 modules/angular2/src/forms/directives.js      |  8 ++---
 .../angular2/src/test_lib/benchmark_util.js   |  5 +--
 modules/angular2/src/test_lib/test_lib.es6    |  4 +--
 modules/angular2/src/test_lib/utils.js        |  2 +-
 11 files changed, 84 insertions(+), 29 deletions(-)

diff --git a/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js b/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js
index 21f5a28628..e995be1ac7 100644
--- a/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js
+++ b/modules/angular2/src/core/compiler/pipeline/compile_pipeline.js
@@ -1,6 +1,6 @@
 import {isPresent} from 'angular2/src/facade/lang';
 import {List, ListWrapper} from 'angular2/src/facade/collection';
-import {Element, Node, DOM} from 'angular2/src/facade/dom';
+import {Element, DOM} from 'angular2/src/facade/dom';
 import {CompileElement} from './compile_element';
 import {CompileControl} from './compile_control';
 import {CompileStep} from './compile_step';
@@ -25,12 +25,12 @@ export class CompilePipeline {
     var additionalChildren = this._control.internalProcess(results, 0, parent, current);
 
     if (current.compileChildren) {
-      var node = DOM.templateAwareRoot(current.element).firstChild;
+      var node = DOM.firstChild(DOM.templateAwareRoot(current.element));
       while (isPresent(node)) {
         // compiliation can potentially move the node, so we need to store the
         // next sibling before recursing.
         var nextNode = DOM.nextSibling(node);
-        if (node.nodeType === Node.ELEMENT_NODE) {
+        if (DOM.isElementNode(node)) {
           this._process(results, current, new CompileElement(node));
         }
         node = nextNode;
diff --git a/modules/angular2/src/core/compiler/pipeline/directive_parser.js b/modules/angular2/src/core/compiler/pipeline/directive_parser.js
index 5b8c464e57..0aedf261d1 100644
--- a/modules/angular2/src/core/compiler/pipeline/directive_parser.js
+++ b/modules/angular2/src/core/compiler/pipeline/directive_parser.js
@@ -1,6 +1,6 @@
 import {isPresent, isBlank, BaseException} from 'angular2/src/facade/lang';
 import {List, MapWrapper} from 'angular2/src/facade/collection';
-import {TemplateElement} from 'angular2/src/facade/dom';
+import {DOM} from 'angular2/src/facade/dom';
 import {SelectorMatcher} from '../selector';
 import {CssSelector} from '../selector';
 
@@ -44,7 +44,7 @@ export class DirectiveParser extends CompileStep {
     var classList = current.classList();
 
     var cssSelector = new CssSelector();
-    cssSelector.setElement(current.element.nodeName);
+    cssSelector.setElement(DOM.nodeName(current.element));
     for (var i=0; i < classList.length; i++) {
       cssSelector.addClassName(classList[i]);
     }
@@ -66,7 +66,7 @@ export class DirectiveParser extends CompileStep {
     }
     // Note: We assume that the ViewSplitter already did its work, i.e. template directive should
     // only be present on <template> elements any more!
-    var isTemplateElement = current.element instanceof TemplateElement;
+    var isTemplateElement = DOM.isTemplateElement(current.element);
     this._selectorMatcher.match(cssSelector, (directive) => {
       if (directive.annotation instanceof Viewport) {
         if (!isTemplateElement) {
diff --git a/modules/angular2/src/core/compiler/pipeline/text_interpolation_parser.js b/modules/angular2/src/core/compiler/pipeline/text_interpolation_parser.js
index ec3474a9ae..b4c5bf59cb 100644
--- a/modules/angular2/src/core/compiler/pipeline/text_interpolation_parser.js
+++ b/modules/angular2/src/core/compiler/pipeline/text_interpolation_parser.js
@@ -1,5 +1,5 @@
 import {RegExpWrapper, StringWrapper, isPresent} from 'angular2/src/facade/lang';
-import {Node, DOM} from 'angular2/src/facade/dom';
+import {DOM} from 'angular2/src/facade/dom';
 
 import {Parser} from 'angular2/change_detection';
 
@@ -27,17 +27,17 @@ export class TextInterpolationParser extends CompileStep {
       return;
     }
     var element = current.element;
-    var childNodes = DOM.templateAwareRoot(element).childNodes;
+    var childNodes = DOM.childNodes(DOM.templateAwareRoot(element));
     for (var i=0; i<childNodes.length; i++) {
       var node = childNodes[i];
-      if (node.nodeType === Node.TEXT_NODE) {
+      if (DOM.isTextNode(node)) {
         this._parseTextNode(current, node, i);
       }
     }
   }
 
   _parseTextNode(pipelineElement, node, nodeIndex) {
-    var ast = this._parser.parseInterpolation(node.nodeValue, this._compilationUnit);
+    var ast = this._parser.parseInterpolation(DOM.nodeValue(node), this._compilationUnit);
     if (isPresent(ast)) {
       DOM.setText(node, ' ');
       pipelineElement.addTextNodeBinding(nodeIndex, ast);
diff --git a/modules/angular2/src/core/compiler/pipeline/view_splitter.js b/modules/angular2/src/core/compiler/pipeline/view_splitter.js
index 03729935e5..e53b1d060a 100644
--- a/modules/angular2/src/core/compiler/pipeline/view_splitter.js
+++ b/modules/angular2/src/core/compiler/pipeline/view_splitter.js
@@ -45,12 +45,12 @@ export class ViewSplitter extends CompileStep {
     if (isBlank(parent)) {
       current.isViewRoot = true;
     } else {
-      if (current.element instanceof TemplateElement) {
+      if (DOM.isTemplateElement(current.element)) {
         if (!current.isViewRoot) {
           var viewRoot = new CompileElement(DOM.createTemplate(''));
           var currentElement:TemplateElement = current.element;
           var viewRootElement:TemplateElement = viewRoot.element;
-          this._moveChildNodes(currentElement.content, viewRootElement.content);
+          this._moveChildNodes(DOM.content(currentElement), DOM.content(viewRootElement));
           viewRoot.isViewRoot = true;
           control.addChild(viewRoot);
         }
@@ -81,15 +81,17 @@ export class ViewSplitter extends CompileStep {
           this._addParentElement(current.element, newParent.element);
 
           control.addParent(newParent);
-          current.element.remove();
+          DOM.remove(current.element);
         }
       }
     }
   }
 
   _moveChildNodes(source, target) {
-    while (isPresent(source.firstChild)) {
-      DOM.appendChild(target, source.firstChild);
+    var next = DOM.firstChild(source);
+    while (isPresent(next)) {
+      DOM.appendChild(target, next);
+      next = DOM.firstChild(source);
     }
   }
 
@@ -107,7 +109,7 @@ export class ViewSplitter extends CompileStep {
       } else if (isPresent(binding.expression)) {
         compileElement.addPropertyBinding(binding.key, binding.expression);
       } else {
-        compileElement.element.setAttribute(binding.key, '');
+        DOM.setAttribute(compileElement.element, binding.key, '');
       }
     }
   }
diff --git a/modules/angular2/src/core/compiler/view.js b/modules/angular2/src/core/compiler/view.js
index d299472d80..b39ce3743d 100644
--- a/modules/angular2/src/core/compiler/view.js
+++ b/modules/angular2/src/core/compiler/view.js
@@ -1,4 +1,4 @@
-import {DOM, Element, Node, Text, DocumentFragment, TemplateElement} from 'angular2/src/facade/dom';
+import {DOM, Element, Node, Text, DocumentFragment} from 'angular2/src/facade/dom';
 import {ListWrapper, MapWrapper, StringMapWrapper, List} from 'angular2/src/facade/collection';
 import {AST, ContextWithVariableBindings, ChangeDispatcher, ProtoChangeDetector, ChangeDetector, ChangeRecord}
   from 'angular2/change_detection';
@@ -289,7 +289,7 @@ export class ProtoView {
     this.instantiateInPlace = false;
     this.rootBindingOffset = (isPresent(this.element) && DOM.hasClass(this.element, NG_BINDING_CLASS))
       ? 1 : 0;
-    this.isTemplateElement = this.element instanceof TemplateElement;
+    this.isTemplateElement = DOM.isTemplateElement(this.element);
     this.shadowDomStrategy = shadowDomStrategy;
     this._viewPool = new ViewPool(VIEW_POOL_CAPACITY);
   }
@@ -311,7 +311,7 @@ export class ProtoView {
     var rootElementClone = this.instantiateInPlace ? this.element : DOM.clone(this.element);
     var elementsWithBindingsDynamic;
     if (this.isTemplateElement) {
-      elementsWithBindingsDynamic = DOM.querySelectorAll(rootElementClone.content, NG_BINDING_CLASS_SELECTOR);
+      elementsWithBindingsDynamic = DOM.querySelectorAll(DOM.content(rootElementClone), NG_BINDING_CLASS_SELECTOR);
     } else {
       elementsWithBindingsDynamic= DOM.getElementsByClassName(rootElementClone, NG_BINDING_CLASS);
     }
@@ -323,7 +323,7 @@ export class ProtoView {
 
     var viewNodes;
     if (this.isTemplateElement) {
-      var childNode = DOM.firstChild(rootElementClone.content);
+      var childNode = DOM.firstChild(DOM.content(rootElementClone));
       viewNodes = []; // TODO(perf): Should be fixed size, since we could pre-compute in in ProtoView
       // Note: An explicit loop is the fastest way to convert a DOM array into a JS array!
       while(childNode != null) {
diff --git a/modules/angular2/src/facade/dom.dart b/modules/angular2/src/facade/dom.dart
index 7822d9518f..d9c3762067 100644
--- a/modules/angular2/src/facade/dom.dart
+++ b/modules/angular2/src/facade/dom.dart
@@ -11,6 +11,7 @@ export 'dart:html' show
   Node,
   StyleElement,
   TemplateElement,
+  InputElement,
   Text,
   window;
 
@@ -57,6 +58,10 @@ class DOM {
   static void setInnerHTML(Element el, String value) {
     el.innerHtml = value;
   }
+  static String nodeName(Node el) => el.nodeName;
+  static String nodeValue(Node el) => el.nodeValue;
+  static String type(InputElement el) => el.type;
+  static Node content(TemplateElement el) => el.content;
   static Node firstChild(el) => el.firstChild;
   static Node nextSibling(Node el) => el.nextNode;
   static Element parentElement(Node el) => el.parent;
@@ -87,6 +92,14 @@ class DOM {
   static void setText(Node el, String value) {
     el.text = value;
   }
+  static String getValue(InputElement el) => el.value;
+  static void setValue(InputElement el, String value) {
+    el.value = value;
+  }
+  static bool getChecked(InputElement el) => el.checked;
+  static void setChecked(InputElement el, bool isChecked) {
+    el.checked = isChecked;
+  }
   static TemplateElement createTemplate(String html) {
     var t = new TemplateElement();
     t.setInnerHtml(html, treeSanitizer: identitySanitizer);
@@ -163,4 +176,10 @@ class DOM {
   static HtmlDocument defaultDoc() => document;
   static bool elementMatches(n, String selector) =>
       n is Element && n.matches(selector);
+  static bool isTemplateElement(Element el) =>
+      el is TemplateElement;
+  static bool isTextNode(Node node) =>
+      node.nodeType == Node.TEXT_NODE;
+  static bool isElementNode(Node node) =>
+      node.nodeType == Node.ELEMENT_NODE;
 }
diff --git a/modules/angular2/src/facade/dom.es6 b/modules/angular2/src/facade/dom.es6
index 0d38f2e8e2..47566e6f94 100644
--- a/modules/angular2/src/facade/dom.es6
+++ b/modules/angular2/src/facade/dom.es6
@@ -42,6 +42,18 @@ export class DOM {
   static getOuterHTML(el) {
     return el.outerHTML;
   }
+  static nodeName(node:Node):string {
+    return node.nodeName;
+  }
+  static nodeValue(node:Node):string {
+    return node.nodeValue;
+  }
+  static type(node:Element):string {
+    return node.type;
+  }
+  static content(node:TemplateElement):Node {
+    return node.content;
+  }
   static firstChild(el):Node {
     return el.firstChild;
   }
@@ -97,6 +109,18 @@ export class DOM {
   static setText(el, value:string) {
     el.textContent = value;
   }
+  static getValue(el: Element) {
+    return el.value;
+  }
+  static setValue(el: Element, value:string) {
+    el.value = value;
+  }
+  static getChecked(el: Element) {
+    return el.checked;
+  }
+  static setChecked(el: Element, value:boolean) {
+    el.checked = value;
+  }
   static createTemplate(html) {
     var t = document.createElement('template');
     t.innerHTML = html;
@@ -181,4 +205,13 @@ export class DOM {
   static elementMatches(n, selector:string):boolean {
     return n instanceof Element && n.matches(selector);
   }
+  static isTemplateElement(el:any):boolean {
+    return el instanceof TemplateElement;
+  }
+  static isTextNode(node:Node):boolean {
+    return node.nodeType === Node.TEXT_NODE;
+  }
+  static isElementNode(node:Node):boolean {
+    return node.nodeType === Node.ELEMENT_NODE;
+  }
 }
diff --git a/modules/angular2/src/forms/directives.js b/modules/angular2/src/forms/directives.js
index 8b676ed37b..f5de056df4 100644
--- a/modules/angular2/src/forms/directives.js
+++ b/modules/angular2/src/forms/directives.js
@@ -22,11 +22,11 @@ class DefaultControlValueAccessor extends ControlValueAccessor {
   }
 
   readValue(el) {
-    return el.value;
+    return DOM.getValue(el);
   }
 
   writeValue(el, value):void {
-    el.value = value;
+    DOM.setValue(el,value);
   }
 }
 
@@ -37,11 +37,11 @@ class CheckboxControlValueAccessor extends ControlValueAccessor {
   }
 
   readValue(el):boolean {
-    return el.checked;
+    return DOM.getChecked(el);
   }
 
   writeValue(el, value:boolean):void {
-    el.checked = value;
+    DOM.setChecked(el, value);
   }
 }
 
diff --git a/modules/angular2/src/test_lib/benchmark_util.js b/modules/angular2/src/test_lib/benchmark_util.js
index 058b586c74..6aba8fee4d 100644
--- a/modules/angular2/src/test_lib/benchmark_util.js
+++ b/modules/angular2/src/test_lib/benchmark_util.js
@@ -12,8 +12,9 @@ export function getStringParameter(name:string) {
 
   for (var i=0; i<els.length; i++) {
     el = els[i];
-    if ((el.type !== 'radio' && el.type !== 'checkbox') || el.checked) {
-      value = el.value;
+    var type = DOM.type(el);
+    if ((type !== 'radio' && type !== 'checkbox') || DOM.getChecked(el)) {
+      value = DOM.getValue(el);
       break;
     }
   }
diff --git a/modules/angular2/src/test_lib/test_lib.es6 b/modules/angular2/src/test_lib/test_lib.es6
index 25c6a835e9..3877f56b51 100644
--- a/modules/angular2/src/test_lib/test_lib.es6
+++ b/modules/angular2/src/test_lib/test_lib.es6
@@ -151,12 +151,12 @@ export class SpyObject {
 
 function elementText(n) {
   var hasShadowRoot = (n) => n instanceof Element && n.shadowRoot;
-  var hasNodes = (n) => n.childNodes && n.childNodes.length > 0;
+  var hasNodes = (n) => {var children = DOM.childNodes(n); return children && children.length > 0;}
 
   if (n instanceof Comment)         return '';
 
   if (n instanceof Array)           return n.map((nn) => elementText(nn)).join("");
-  if (n instanceof Element && n.tagName == 'CONTENT')
+  if (n instanceof Element && DOM.tagName(n) == 'CONTENT')
     return elementText(Array.prototype.slice.apply(n.getDistributedNodes()));
   if (hasShadowRoot(n))             return elementText(DOM.childNodesAsList(n.shadowRoot));
   if (hasNodes(n))                  return elementText(DOM.childNodesAsList(n));
diff --git a/modules/angular2/src/test_lib/utils.js b/modules/angular2/src/test_lib/utils.js
index cf17a1982c..d4deff8744 100644
--- a/modules/angular2/src/test_lib/utils.js
+++ b/modules/angular2/src/test_lib/utils.js
@@ -39,5 +39,5 @@ export function dispatchEvent(element, eventType) {
 }
 
 export function el(html) {
-  return DOM.firstChild(DOM.createTemplate(html).content);
+  return DOM.firstChild(DOM.content(DOM.createTemplate(html)));
 }