Optimize ifIfnotWith slightly; make sure saved nodes are cleaned. Add…

… spec for new 'with' functionality.
vibster · Sep 30, 2012 · 112b3dd · 112b3dd
1 parent 72a8489
commit 112b3dd
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 14 deletions.
diff --git a/spec/defaultBindings/withBehaviors.js b/spec/defaultBindings/withBehaviors.js
@@ -16,6 +16,17 @@ describe('Binding: With', {
         value_of(testNode.childNodes[0].childNodes[0]).should_contain_text("Child prop value");
     },
 
+    'Should leave descendant nodes unchanged if the value is truthy': function() {
+        var someItem = ko.observable({ childProp: 'child prop value' });
+        testNode.innerHTML = "<div data-bind='with: someItem'><span data-bind='text: childProp'></span></div>";
+        var originalNode = testNode.childNodes[0].childNodes[0];
+
+        // Value is initially true, so nodes are retained
+        ko.applyBindings({ someItem: someItem }, testNode);
+        value_of(testNode.childNodes[0].childNodes[0]).should_contain_text("child prop value");
+        value_of(testNode.childNodes[0].childNodes[0]).should_be(originalNode);
+    },
+
     'Should toggle the presence and bindedness of descendant nodes according to the truthiness of the value, performing binding in the context of the value': function() {
         var someItem = ko.observable(undefined);
         testNode.innerHTML = "<div data-bind='with: someItem'><span data-bind='text: occasionallyExistentChildProp'></span></div>";

diff --git a/src/binding/defaultBindings/ifIfnotWith.js b/src/binding/defaultBindings/ifIfnotWith.js
@@ -3,31 +3,31 @@ var withIfDomDataKey = '__ko_withIfBindingData';
 function makeWithIfBinding(bindingKey, isWith, isNot, makeContextCallback) {
     ko.bindingHandlers[bindingKey] = {
         'init': function(element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) {
-            ko.utils.domData.set(element, withIfDomDataKey, { isFirstRender: true });
+            ko.utils.domData.set(element, withIfDomDataKey, {});
             return { 'controlsDescendantBindings': true };
         },
         'update': function(element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) {
-            var withData = ko.utils.domData.get(element, withIfDomDataKey),
+            var withIfData = ko.utils.domData.get(element, withIfDomDataKey),
                 dataValue = ko.utils.unwrapObservable(valueAccessor()),
-                shouldDisplay = isNot ? !dataValue : !!dataValue, // Ensure it's really a bool, to avoid storing extra refs to model objects
-                needsRefresh = withData.isFirstRender || isWith || (shouldDisplay !== withData.didDisplayOnLastUpdate);
+                shouldDisplay = !isNot !== !dataValue, // equivalent to isNot ? !dataValue : !!dataValue
+                isFirstRender = !withIfData.savedNodes,
+                needsRefresh = isFirstRender || isWith || (shouldDisplay !== withIfData.didDisplayOnLastUpdate);
 
             if (needsRefresh) {
-                if (withData.isFirstRender) {
-                    withData.savedNodes = ko.utils.cloneNodes(ko.virtualElements.childNodes(element));
+                if (isFirstRender) {
+                    withIfData.savedNodes = ko.utils.cloneNodes(ko.virtualElements.childNodes(element), true /* shouldCleanNodes */);
                 }
 
                 if (shouldDisplay) {
-                    if (!withData.isFirstRender) {
-                        ko.virtualElements.setDomNodeChildren(element, ko.utils.cloneNodes(withData.savedNodes));
+                    if (!isFirstRender) {
+                        ko.virtualElements.setDomNodeChildren(element, ko.utils.cloneNodes(withIfData.savedNodes));
                     }
                     ko.applyBindingsToDescendants(makeContextCallback ? makeContextCallback(bindingContext, dataValue) : bindingContext, element);
                 } else {
                     ko.virtualElements.emptyNode(element);
                 }
 
-                withData.isFirstRender = false;
-                withData.didDisplayOnLastUpdate = shouldDisplay;
+                withIfData.didDisplayOnLastUpdate = shouldDisplay;
             }
         }
     };

diff --git a/src/utils.domNodeDisposal.js b/src/utils.domNodeDisposal.js
@@ -79,6 +79,7 @@ ko.utils.domNodeDisposal = new (function () {
                         cleanSingleNode(descendants[i]);
                 }
             }
+            return node;
         },
 
         removeNode : function(node) {

diff --git a/src/utils.js b/src/utils.js
@@ -126,15 +126,15 @@ ko.utils = new (function () {
 
             var container = document.createElement('div');
             for (var i = 0, j = nodesArray.length; i < j; i++) {
-                ko.cleanNode(nodesArray[i]);
-                container.appendChild(nodesArray[i]);
+                container.appendChild(ko.cleanNode(nodesArray[i]));
             }
             return container;
         },
 
-        cloneNodes: function (nodesArray) {
+        cloneNodes: function (nodesArray, shouldCleanNodes) {
             for (var i = 0, j = nodesArray.length, newNodesArray = []; i < j; i++) {
-                newNodesArray.push(nodesArray[i].cloneNode(true));
+                var clonedNode = nodesArray[i].cloneNode(true);
+                newNodesArray.push(shouldCleanNodes ? ko.cleanNode(clonedNode) : clonedNode);
             }
             return newNodesArray;
         },
-Original file line number
+Diff line change
@@ Expand Up / @@ -79,6 +79,7 @@ ko.utils.domNodeDisposal = new (function () { @@
                             cleanSingleNode(descendants[i]);
                     }
                 }
+                return node;
             },
             removeNode : function(node) {
@@ Expand Down @@