Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix obscure nested-rewritten-templates-plus-binding-provider bug (and…

… add spec to demonstrate it)
  • Loading branch information...
commit 9a42b9fac3c955fc08486c2e5d1ba82f1905b27a 1 parent 0aa485c
@SteveSanderson SteveSanderson authored
Showing with 45 additions and 19 deletions.
  1. +26 −6 spec/templatingBehaviors.js
  2. +19 −13 src/templating/templating.js
View
32 spec/templatingBehaviors.js
@@ -67,7 +67,7 @@ var dummyTemplateEngine = function (templates) {
}
if (options.bypassDomNodeWrap)
- return result;
+ return ko.utils.parseHtmlFragment(result);
else {
var node = document.createElement("div");
@@ -297,13 +297,33 @@ describe('Templating', {
value_of(viewModel.didCallMyFunction).should_be(true);
},
- 'Data binding syntax should permit nested templates': function() {
+ 'Data binding syntax should permit nested templates, and only bind inner templates once': function() {
+ // Will verify that bindings are applied only once for both inline (rewritten) bindings,
+ // and external (non-rewritten) ones
+ var originalBindingProvider = ko.bindingProvider.instance;
+ ko.bindingProvider.instance = {
+ nodeHasBindings: function(node, bindingContext) {
+ return (node.className == 'applyExternalBinding') || originalBindingProvider.nodeHasBindings(node, bindingContext);
+ },
+ getBindings: function(node, bindingContext) {
+ if (node.className == 'applyExternalBinding')
+ return { text: ++model.numBindings };
+ return originalBindingProvider.getBindings(node, bindingContext);
+ }
+ };
+
ko.setTemplateEngine(new dummyTemplateEngine({
- outerTemplate: "Outer <div data-bind='template: \"innerTemplate\"'></div>",
- innerTemplate: "Inner"
+ outerTemplate: "Outer <div data-bind='template: { name: \"innerTemplate\", bypassDomNodeWrap: true }'></div>",
+ innerTemplate: "Inner via inline binding: <span data-bind='text: ++numBindings'></span>"
+ + "Inner via external binding: <span class='applyExternalBinding'></span>"
}));
- ko.renderTemplate("outerTemplate", null, null, testNode);
- value_of(testNode.childNodes[0]).should_contain_html("outer <div><div>inner</div></div>");
+ var model = { numBindings: 0 };
+ testNode.innerHTML = "<div data-bind='template: { name: \"outerTemplate\", bypassDomNodeWrap: true }'></div>";
+ ko.applyBindings(model, testNode);
+ value_of(model.numBindings).should_be(2);
+ value_of(testNode.childNodes[0]).should_contain_html("outer <div>inner via inline binding: <span>2</span>inner via external binding: <span class=\"applyexternalbinding\">1</span></div>");
+
+ ko.bindingProvider.instance = originalBindingProvider;
},
'Data binding syntax should support \'foreach\' option, whereby it renders for each item in an array but doesn\'t rerender everything if you push or splice': function () {
View
32 src/templating/templating.js
@@ -7,6 +7,15 @@
_templateEngine = templateEngine;
}
+ function invokeForEachNodeOrCommentInParent(nodeArray, parent, action) {
+ for (var i = 0; node = nodeArray[i]; i++) {
+ if (node.parentNode !== parent) // Skip anything that has been removed during binding
+ continue;
+ if ((node.nodeType === 1) || (node.nodeType === 8))
+ action(node);
+ }
+ }
+
ko.activateBindingsOnTemplateRenderedNodes = function(nodeArray, bindingContext) {
// To be used on any nodes that have been rendered by a template and have been inserted into some parent element.
// Safely iterates through nodeArray (being tolerant of any changes made to it during binding, e.g.,
@@ -15,20 +24,17 @@
// (2) Unmemoizes any memos in the DOM subtree (e.g., to activate bindings that had been memoized during template rewriting)
var nodeArrayClone = ko.utils.arrayPushAll([], nodeArray); // So we can tolerate insertions/deletions during binding
- var node;
var commonParentElement = (nodeArray.length > 0) ? nodeArray[0].parentNode : null; // All items must be in the same parent, so this is OK
- for (var i = 0; node = nodeArrayClone[i]; i++) {
- if (node.parentNode !== commonParentElement) // Skip anything that has been removed during binding
- continue;
-
- switch (node.nodeType) {
- case 1: // Elements (can have bindings)
- case 8: // Comment nodes (may be containerless templates)
- ko.applyBindings(bindingContext, node);
- ko.memoization.unmemoizeDomNodeAndDescendants(node, [bindingContext]);
- break;
- }
- }
+
+ // Need to applyBindings *before* unmemoziation, because unmemoization might introduce extra nodes (that we don't want to re-bind)
+ // whereas a regular applyBindings won't introduce new memoized nodes
+
+ invokeForEachNodeOrCommentInParent(nodeArrayClone, commonParentElement, function(node) {
+ ko.applyBindings(bindingContext, node);
+ });
+ invokeForEachNodeOrCommentInParent(nodeArrayClone, commonParentElement, function(node) {
+ ko.memoization.unmemoizeDomNodeAndDescendants(node, [bindingContext]);
+ });
}
function getFirstNodeFromPossibleArray(nodeOrNodeArray) {
Please sign in to comment.
Something went wrong with that request. Please try again.