Skip to content

Commit

Permalink
More proactive subscription disposal for templates
Browse files Browse the repository at this point in the history
  • Loading branch information
@SteveSanderson
SteveSanderson committed Mar 21, 2011
1 parent 9313450 commit f878a09
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 18 deletions.
63 changes: 57 additions & 6 deletions spec/templatingBehaviors.js
View file
Expand Up @@ -122,12 +122,10 @@ describe('Templating', {
value_of(testNode.childNodes[0].innerHTML).should_be("Value = B");
},

'Should stop updating DOM nodes if they are removed from the document': function () {
'Should stop updating DOM nodes when the dependency next changes if the DOM node has been removed from the document': function () {
var dependency = new ko.observable("A");
ko.setTemplateEngine(new dummyTemplateEngine({ someTemplate: function () {
return "Value = " + dependency();
}
}));
var template = { someTemplate: function () { return "Value = " + dependency() } };
ko.setTemplateEngine(new dummyTemplateEngine(template));

ko.renderTemplate("someTemplate", null, null, testNode);
value_of(testNode.childNodes.length).should_be(1);
Expand All @@ -138,7 +136,7 @@ describe('Templating', {
value_of(testNode.childNodes.length).should_be(1);
value_of(testNode.childNodes[0].innerHTML).should_be("Value = A");
},

'Should be able to render a template using data-bind syntax': function () {
ko.setTemplateEngine(new dummyTemplateEngine({ someTemplate: "template output" }));
testNode.innerHTML = "<div data-bind='template:\"someTemplate\"'></div>";
Expand All @@ -153,6 +151,17 @@ describe('Templating', {
value_of(testNode.childNodes[0].innerHTML.toLowerCase()).should_be("<div>result = 123</div>");
},

'Should stop tracking inner observables immediately when the container node is removed from the document': function() {
var innerObservable = ko.observable("some value");
ko.setTemplateEngine(new dummyTemplateEngine({ someTemplate: "result = [js: childProp()]" }));
testNode.innerHTML = "<div data-bind='template: { name: \"someTemplate\", data: someProp }'></div>";
ko.applyBindings({ someProp: { childProp: innerObservable} }, testNode);

value_of(innerObservable.getSubscriptionsCount()).should_be(1);
ko.removeNode(testNode.childNodes[0]);
value_of(innerObservable.getSubscriptionsCount()).should_be(0);
},

'Should be able to pick template as a function of the data item using data-bind syntax': function () {
var templatePicker = function(dataItem) {
return dataItem.myTemplate;
Expand Down Expand Up @@ -191,6 +200,20 @@ describe('Templating', {
value_of(timesRenderedOuter).should_be(1);
value_of(timesRenderedInner).should_be(2);
},

'Should stop tracking inner observables referenced by a chained template as soon as the chained template output node is removed from the document': function() {
var innerObservable = ko.observable("some value");
ko.setTemplateEngine(new dummyTemplateEngine({
outerTemplate: "outer template output, <span id='innerTemplateOutput'>[renderTemplate:innerTemplate]</span>",
innerTemplate: "result = [js: childProp()]"
}));
testNode.innerHTML = "<div data-bind='template: { name: \"outerTemplate\", data: someProp }'></div>";
ko.applyBindings({ someProp: { childProp: innerObservable} }, testNode);

value_of(innerObservable.getSubscriptionsCount()).should_be(1);
ko.removeNode(document.getElementById('innerTemplateOutput'));
value_of(innerObservable.getSubscriptionsCount()).should_be(0);
},

'Should handle data-bind attributes from inside templates, regardless of element and attribute casing': function () {
ko.setTemplateEngine(new dummyTemplateEngine({ someTemplate: "<INPUT Data-Bind='value:\"Hi\"' />" }));
Expand Down Expand Up @@ -285,6 +308,34 @@ describe('Templating', {
value_of(testNode.childNodes[0].childNodes.length).should_be(0);
},

'Data binding \'foreach\' option should stop tracking inner observables when the container node is removed': function() {
var innerObservable = ko.observable("some value");
var myArray = new ko.observableArray([{obsVal:innerObservable}, {obsVal:innerObservable}]);
ko.setTemplateEngine(new dummyTemplateEngine({ itemTemplate: "The item is [js: ko.utils.unwrapObservable(obsVal)]" }));
testNode.innerHTML = "<div data-bind='template: { name: \"itemTemplate\", foreach: myCollection }'></div>";

ko.applyBindings({ myCollection: myArray }, testNode);
value_of(innerObservable.getSubscriptionsCount()).should_be(2);

ko.removeNode(testNode.childNodes[0]);
value_of(innerObservable.getSubscriptionsCount()).should_be(0);
},

'Data binding \'foreach\' option should stop tracking inner observables related to each array item when that array item is removed': function() {
var innerObservable = ko.observable("some value");
var myArray = new ko.observableArray([{obsVal:innerObservable}, {obsVal:innerObservable}]);
ko.setTemplateEngine(new dummyTemplateEngine({ itemTemplate: "The item is [js: ko.utils.unwrapObservable(obsVal)]" }));
testNode.innerHTML = "<div data-bind='template: { name: \"itemTemplate\", foreach: myCollection }'></div>";

ko.applyBindings({ myCollection: myArray }, testNode);
value_of(innerObservable.getSubscriptionsCount()).should_be(2);

myArray.splice(1);
value_of(innerObservable.getSubscriptionsCount()).should_be(1);
myArray([]);
value_of(innerObservable.getSubscriptionsCount()).should_be(0);
},

'Data binding syntax should omit any items whose \'_destroy\' flag is set' : function() {
var myArray = new ko.observableArray([{ someProp: 1 }, { someProp: 2, _destroy: 'evals to true' }, { someProp : 3 }]);
ko.setTemplateEngine(new dummyTemplateEngine({ itemTemplate: "someProp=[js: someProp]" }));
Expand Down
17 changes: 11 additions & 6 deletions src/binding/editDetection/arrayToDomNodeChildren.js
View file
Expand Up @@ -7,10 +7,10 @@
// so that its children is again the concatenation of the mappings of the array elements, but don't re-map any array elements that we
// previously mapped - retain those nodes, and just insert/delete other ones

function mapNodeAndRefreshWhenChanged(mapping, valueToMap) {
function mapNodeAndRefreshWhenChanged(containerNode, mapping, valueToMap) {
// Map this array value inside a dependentObservable so we re-map when any dependency changes
var mappedNodes = [];
ko.dependentObservable(function() {
var dependentObservable = ko.dependentObservable(function() {
var newMappedNodes = mapping(valueToMap) || [];

// On subsequent evaluations, just replace the previously-inserted DOM nodes
Expand All @@ -21,8 +21,8 @@
// of which nodes would be deleted if valueToMap was itself later removed
mappedNodes.splice(0, mappedNodes.length);
ko.utils.arrayPushAll(mappedNodes, newMappedNodes);
}, null, { 'disposeWhen': function() { return (mappedNodes.length == 0) || !ko.utils.domNodeIsAttachedToDocument(mappedNodes[0]) } });
return mappedNodes;
}, null, { 'disposeWhenNodeIsRemoved': containerNode, 'disposeWhen': function() { return (mappedNodes.length == 0) || !ko.utils.domNodeIsAttachedToDocument(mappedNodes[0]) } });
return { mappedNodes : mappedNodes, dependentObservable : dependentObservable };
}

ko.utils.setDomNodeChildrenFromArrayMapping = function (domNode, array, mapping, options) {
Expand Down Expand Up @@ -52,6 +52,9 @@
break;

case "deleted":
// Stop tracking changes to the mapping for these nodes
lastMappingResult[lastMappingResultIndex].dependentObservable.dispose();

// Queue these nodes for later removal
ko.utils.arrayForEach(lastMappingResult[lastMappingResultIndex].domNodes, function (node) {
nodesToDelete.push({
Expand All @@ -65,9 +68,11 @@
break;

case "added":
var mappedNodes = mapNodeAndRefreshWhenChanged(mapping, editScript[i].value);
var mapData = mapNodeAndRefreshWhenChanged(domNode, mapping, editScript[i].value);
var mappedNodes = mapData.mappedNodes;

// On the first evaluation, insert the nodes at the current insertion point
newMappingResult.push({ arrayEntry: editScript[i].value, domNodes: mappedNodes });
newMappingResult.push({ arrayEntry: editScript[i].value, domNodes: mappedNodes, dependentObservable: mapData.dependentObservable });
for (var nodeIndex = 0, nodeIndexMax = mappedNodes.length; nodeIndex < nodeIndexMax; nodeIndex++) {
var node = mappedNodes[nodeIndex];
nodesAdded.push({
Expand Down
2 changes: 1 addition & 1 deletion src/subscribables/dependentObservable.js
View file
Expand Up @@ -18,7 +18,7 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction

// "disposeWhenNodeIsRemoved" option both proactively disposes as soon as the node is removed using ko.removeNode(),
// plus adds a "disposeWhen" callback that, on each evaluation, disposes if the node was removed by some other means.
if (typeof options["disposeWhenNodeIsRemoved"] == "object") {
if ((typeof options["disposeWhenNodeIsRemoved"] == "object") && options["disposeWhenNodeIsRemoved"]) {
var nodeToWatch = options["disposeWhenNodeIsRemoved"];
ko.utils.domNodeDisposal.addDisposeCallback(nodeToWatch, function() {
dependentObservable.dispose();
Expand Down
11 changes: 6 additions & 5 deletions src/templating/templating.js
View file
Expand Up @@ -51,7 +51,10 @@

if (targetNodeOrNodeArray) {
var firstTargetNode = getFirstNodeFromPossibleArray(targetNodeOrNodeArray);
var whenToDispose = function () { return (!firstTargetNode) || !ko.utils.domNodeIsAttachedToDocument(firstTargetNode); };

var whenToDispose = function () { return (!firstTargetNode) || !ko.utils.domNodeIsAttachedToDocument(firstTargetNode); }; // Passive disposal (on next evaluation)
var activelyDisposeWhenNodeIsRemoved = (firstTargetNode && renderMode == "replaceNode") ? firstTargetNode.parentNode : firstTargetNode;

return new ko.dependentObservable( // So the DOM is automatically updated when any dependency changes
function () {
// Support selecting template as a function of the data being rendered
Expand All @@ -64,7 +67,7 @@
}
},
null,
{ 'disposeWhen': whenToDispose }
{ 'disposeWhen': whenToDispose, 'disposeWhenNodeIsRemoved': activelyDisposeWhenNodeIsRemoved }
);
} else {
// We don't yet have a DOM node to evaluate, so use a memo and render the template later when there is a DOM node
Expand All @@ -75,8 +78,6 @@
};

ko.renderTemplateForEach = function (template, arrayOrObservableArray, options, targetNode) {
var whenToDispose = function () { return !ko.utils.domNodeIsAttachedToDocument(targetNode); };

return new ko.dependentObservable(function () {
var unwrappedArray = ko.utils.unwrapObservable(arrayOrObservableArray) || [];
if (typeof unwrappedArray.length == "undefined") // Coerce single value into array
Expand All @@ -93,7 +94,7 @@

return executeTemplate(null, "ignoreTargetNode", templateName, arrayValue, options);
}, options);
}, null, { 'disposeWhen': whenToDispose });
}, null, { 'disposeWhenNodeIsRemoved': targetNode });
};

var templateSubscriptionDomDataKey = '__ko__templateSubscriptionDomDataKey__';
Expand Down

0 comments on commit f878a09

Please sign in to comment.