Reordered the sync flow, needs more tests

tbranyen · Jan 9, 2016 · f6f7533 · f6f7533
1 parent df043df
commit f6f7533
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 54 deletions.
diff --git a/dist/diffhtml.js b/dist/diffhtml.js
@@ -1066,8 +1066,8 @@ function sync(oldTree, newTree, patches) {
   }
 
   // If the top level nodeValue has changed we should reflect it.
-  if (oldTree.nodeValue !== nodeValue && !(!oldTree.nodeValue && !nodeValue)) {
-    if (oldTree.nodeValue !== null) {
+  if (oldTree.nodeValue && nodeValue) {
+    if (oldTree.nodeValue !== null && newTree.nodeValue) {
       patches[patches.length] = {
         __do__: CHANGE_TEXT,
         element: oldTree,
@@ -1108,24 +1108,13 @@ function sync(oldTree, newTree, patches) {
 
   // Remove these elements.
   if (oldChildNodesLength > childNodesLength) {
-    var cloneOldChildNodes = slice.call(oldChildNodes, 0);
-
-    // Find the correct elements to remove, is smart about keeping existing
-    // elements.
-    var toRemove = filter.call(cloneOldChildNodes, function (el, index) {
-      var newChild = childNodes[oldChildNodes.indexOf(el)];
-      var notSame = newChild ? el.nodeName !== newChild.nodeName : true;
-
-      if (notSame && oldChildNodes.indexOf(el) > -1) {
-        oldChildNodes.splice(oldChildNodes.indexOf(el), 1);
-      }
-
-      return notSame;
-    });
+    // For now just splice out the end items.
+    var diff = oldChildNodesLength - childNodesLength;
+    var toRemove = oldChildNodes.splice(oldChildNodesLength - diff, diff);
 
     oldChildNodesLength = oldChildNodes.length;
 
-    if (oldChildNodesLength === 0) {
+    if (oldChildNodesLength === 0 && childNodesLength === 0) {
       patches[patches.length] = {
         __do__: REMOVE_ELEMENT_CHILDREN,
         element: oldTree,
@@ -1148,12 +1137,8 @@ function sync(oldTree, newTree, patches) {
   }
 
   // Replace elements if they are different.
-  if (oldChildNodesLength) {
+  if (oldChildNodesLength >= childNodesLength) {
     for (var i = 0; i < childNodesLength; i++) {
-      if (!oldChildNodes[i]) {
-        continue;
-      }
-
       if (oldChildNodes[i].nodeName !== childNodes[i].nodeName) {
         // Add to the patches.
         patches[patches.length] = {
@@ -1329,7 +1314,11 @@ function process(element, patches) {
     if (descriptor.nodeName === '#text' || descriptor.nodeName === 'text') {
       var textPromises = transition.makePromises('textChanged', [element], null, descriptor.nodeValue);
 
-      element.innerHTML = descriptor.nodeValue;
+      if (descriptor.nodeName === 'text') {
+        element.innerText = descriptor.nodeValue;
+      } else {
+        element.innerHTML = descriptor.nodeValue;
+      }
 
       triggerTransition('textChanged', textPromises, function (promises) {});
     }
@@ -2132,9 +2121,7 @@ function makeParser() {
             // if has content
             text = data.slice(lastTextPos, kMarkupPattern.lastIndex - match[0].length);
 
-            if (text.trim()) {
-              currentParent.childNodes.push(TextNode(text));
-            }
+            currentParent.childNodes.push(TextNode(text));
           }
         }
 

diff --git a/lib/node/sync.js b/lib/node/sync.js
@@ -73,8 +73,8 @@ export default function sync(oldTree, newTree, patches) {
   }
 
   // If the top level nodeValue has changed we should reflect it.
-  if ((oldTree.nodeValue !== nodeValue) && !(!oldTree.nodeValue && !nodeValue)) {
-    if (oldTree.nodeValue !== null) {
+  if (oldTree.nodeValue && nodeValue) {
+    if (oldTree.nodeValue !== null && newTree.nodeValue) {
       patches[patches.length] = {
         __do__: CHANGE_TEXT,
         element: oldTree,
@@ -115,24 +115,13 @@ export default function sync(oldTree, newTree, patches) {
 
   // Remove these elements.
   if (oldChildNodesLength > childNodesLength) {
-    let cloneOldChildNodes = slice.call(oldChildNodes, 0);
-
-    // Find the correct elements to remove, is smart about keeping existing
-    // elements.
-    let toRemove = filter.call(cloneOldChildNodes, function(el, index) {
-      var newChild = childNodes[oldChildNodes.indexOf(el)];
-      var notSame = newChild ? el.nodeName !== newChild.nodeName : true;
-
-      if (notSame && oldChildNodes.indexOf(el) > -1) {
-        oldChildNodes.splice(oldChildNodes.indexOf(el), 1);
-      }
-
-      return notSame;
-    });
+    // For now just splice out the end items.
+    let diff = oldChildNodesLength - childNodesLength;
+    let toRemove = oldChildNodes.splice(oldChildNodesLength - diff, diff);
 
     oldChildNodesLength = oldChildNodes.length;
 
-    if (oldChildNodesLength === 0) {
+    if (oldChildNodesLength === 0 && childNodesLength === 0) {
       patches[patches.length] = {
         __do__: REMOVE_ELEMENT_CHILDREN,
         element: oldTree,
@@ -157,10 +146,8 @@ export default function sync(oldTree, newTree, patches) {
   }
 
   // Replace elements if they are different.
-  if (oldChildNodesLength) {
+  if (oldChildNodesLength >= childNodesLength) {
     for (let i = 0; i < childNodesLength; i++) {
-      if (!oldChildNodes[i]) { continue; }
-
       if (oldChildNodes[i].nodeName !== childNodes[i].nodeName) {
         // Add to the patches.
         patches[patches.length] = {

diff --git a/lib/patches/process.js b/lib/patches/process.js
@@ -29,7 +29,11 @@ export default function process(element, patches) {
       let textPromises = transition.makePromises('textChanged',
         [element], null, descriptor.nodeValue);
 
-      element.innerHTML = descriptor.nodeValue;
+      if (descriptor.nodeName === 'text') {
+        element.innerText = descriptor.nodeValue;
+      } else {
+        element.innerHTML = descriptor.nodeValue;
+      }
 
       triggerTransition('textChanged', textPromises, promises => {});
     }
@@ -42,7 +46,6 @@ export default function process(element, patches) {
       fragment.appendChild(element);
     }
 
-
     return element;
   };
 

diff --git a/lib/util/parser.js b/lib/util/parser.js
@@ -189,9 +189,7 @@ export function makeParser() {
             // if has content
             text = data.slice(lastTextPos, kMarkupPattern.lastIndex - match[0].length);
 
-            if (text.trim()) {
-              currentParent.childNodes.push(TextNode(text));
-            }
+            currentParent.childNodes.push(TextNode(text));
           }
         }
 

diff --git a/test/integration/basics.js b/test/integration/basics.js
@@ -188,6 +188,12 @@ describe('Integration: Basics', function() {
       });
 
       it('can track very-complex state', function(done) {
+        function filterOutText(elements) {
+          return [].slice.call(elements).filter(element => {
+            return element.nodeType === 1;
+          });
+        }
+
         var count = 0;
         var tagNames = ['p', 'strong', 'div', 'span'];
 
@@ -207,11 +213,11 @@ describe('Integration: Basics', function() {
           count++;
 
           if (count === 3) {
-            assert.equal(this.fixture.childNodes.length, 100);
+            assert.equal(this.fixture.querySelectorAll('p, strong, div, span').length, 100);
             done();
           }
           else if (count === 1) {
-            assert.equal(this.fixture.childNodes.length, 1000);
+            assert.equal(this.fixture.querySelectorAll('p, strong, div, span').length, 1000);
 
             diff.innerHTML(this.fixture, makePs(10), {
               enableWorker: false

diff --git a/test/integration/outer.js b/test/integration/outer.js
@@ -102,7 +102,7 @@ describe('Integration: outerHTML', function() {
       `);
 
       assert.equal(this.fixture.querySelector('p'), null);
-      assert.equal(this.fixture.firstChild.firstChild.textContent.trim(), `
+      assert.equal(this.fixture.childNodes[1].firstChild.textContent.trim(), `
          var test = \"<p></p>\";
       `.trim());
     });