Optimize SVG support. Fix #155.

* Also remove redundant boolean/string checks when using setElementData/updateElementData, etc. * This temporarily breaks Hyperx until choojs/hyperx/issues/42 is resolved.
jorgebucaran · Mar 11, 2017 · be46a6b · be46a6b
1 parent e6a17f8
commit be46a6b
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 39 deletions.
diff --git a/src/app.js b/src/app.js
@@ -1,3 +1,5 @@
+var SVG_NS = "http://www.w3.org/2000/svg"
+
 export default function (app) {
   var view = app.view || function () {
     return ""
@@ -155,7 +157,7 @@ export default function (app) {
     return a.tag !== b.tag || typeof a !== typeof b || isPrimitive(a) && a !== b
   }
 
-  function createElementFrom(node) {
+  function createElementFrom(node, isSVG) {
     var element
 
     // There are only two types of nodes. A string node, which is
@@ -166,8 +168,8 @@ export default function (app) {
       element = document.createTextNode(node)
 
     } else {
-      element = node.data && node.data.ns
-        ? document.createElementNS(node.data.ns, node.tag)
+      element = (isSVG = isSVG || node.tag === "svg")
+        ? document.createElementNS(SVG_NS, node.tag)
         : document.createElement(node.tag)
 
       for (var name in node.data) {
@@ -179,21 +181,16 @@ export default function (app) {
       }
 
       for (var i = 0; i < node.children.length; i++) {
-        element.appendChild(createElementFrom(node.children[i]))
+        element.appendChild(createElementFrom(node.children[i], isSVG))
       }
     }
 
     return element
   }
 
   function removeElementData(element, name, value) {
-    // Hyperx adds a className attribute to nodes we must handle.
-
-    element.removeAttribute(name === "className" ? "class" : name)
-
-    if (typeof value === "boolean" || value === "true" || value === "false") {
-      element[name] = false
-    }
+    element[name] = value
+    element.removeAttribute(name)
   }
 
   function setElementData(element, name, value, oldValue) {
@@ -208,29 +205,25 @@ export default function (app) {
       element.removeEventListener(event, oldValue)
       element.addEventListener(event, value)
 
-    } else {
-      if (value === "false" || value === false) {
-        element.removeAttribute(name)
-        element[name] = false
+    } else if (value === false) {
+      removeElementData(element, name, value)
 
-      } else {
-        element.setAttribute(name, value)
-        if (element.namespaceURI !== "http://www.w3.org/2000/svg") {
-          // SVG element's props are read only in strict mode.
+    } else {
+      element.setAttribute(name, value)
 
-          var oldSelStart, oldSelEnd
-          var type = element.type
+      if (element.namespaceURI !== SVG_NS) {
+        var oldSelStart, oldSelEnd
+        var type = element.type
 
-          if (type && type.substr(0, 4) === "text") {
-            oldSelStart = element.selectionStart
-            oldSelEnd = element.selectionEnd
-          }
+        if (type && type.substr(0, 4) === "text") {
+          oldSelStart = element.selectionStart
+          oldSelEnd = element.selectionEnd
+        }
 
-          element[name] = value
+        element[name] = value
 
-          if (oldSelStart >= 0) {
-            element.setSelectionRange(oldSelStart, oldSelEnd)
-          }
+        if (oldSelStart >= 0) {
+          element.setSelectionRange(oldSelStart, oldSelEnd)
         }
       }
     }
@@ -243,7 +236,7 @@ export default function (app) {
       var realValue = element[name]
 
       if (value === undefined) {
-        removeElementData(element, name, oldValue)
+        removeElementData(element, name, value)
 
       } else if (name === "onUpdate") {
         defer(value, element)

diff --git a/test/app.test.js b/test/app.test.js
@@ -61,18 +61,57 @@ describe("app", () => {
 			`)
     })
 
-    it("renders an svg tag", () => {
+    it("creates an svg element", () => {
       app({
-        view: _ => h("svg", {}, "foo")
+        view: _ => h("svg", { id: "foo" }, "bar")
       })
 
-      expectHTMLToBe(`
-				<div>
-					<svg ns="http://www.w3.org/2000/svg">
-						foo
-					</svg>
-				</div>
-			`)
+      const elm = document.getElementById("foo")
+      expect(elm.namespaceURI).toBe("http://www.w3.org/2000/svg")
+    })
+
+    it("creates svg elements recursively", () => {
+      const SVG_NS = "http://www.w3.org/2000/svg"
+
+      app({
+        view: _ => h("div", {}, [
+          h("p", { id: "foo" }, "foo"),
+          h("svg", { id: "bar" }, [
+            h("quux", {}, [
+              h("beep", {}, [
+                h("ping", {}),
+                h("pong", {})
+              ]),
+              h("bop", {}),
+              h("boop", {}, [
+                h("ping", {}),
+                h("pong", {})
+              ])
+            ]),
+            h("xuuq", {}, [
+              h("beep", {}),
+              h("bop", {}, [
+                h("ping", {}),
+                h("pong", {})
+              ]),
+              h("boop", {})
+            ])
+          ]),
+          h("p", { id: "baz" }, "baz")
+        ])
+      })
+
+      expect(document.getElementById("foo").namespaceURI).not.toBe(SVG_NS)
+      expect(document.getElementById("baz").namespaceURI).not.toBe(SVG_NS)
+
+      const svg = document.getElementById("bar")
+      expect(svg.namespaceURI).toBe(SVG_NS)
+      expectChildren(svg)
+
+      function expectChildren(svgElement) {
+        Array.from(svgElement.childNodes).forEach(node =>
+          expectChildren(node, expect(node.namespaceURI).toBe(SVG_NS)))
+      }
     })
 
     it("can render conditionally / ignores bool/null children", () => {