Skip to content

Commit

Permalink
prevent applying bindings more than once to the same node
Browse files Browse the repository at this point in the history
  • Loading branch information
mbest committed Jan 4, 2013
1 parent 0170022 commit 17508a1
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 4 deletions.
51 changes: 51 additions & 0 deletions spec/bindingAttributeBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,4 +437,55 @@ describe('Binding attribute syntax', function() {
ko.applyBindings({ myObservable: observable }, testNode);
expect(hasUpdatedSecondBinding).toEqual(true);
});

it('Should not allow multiple applyBindings calls for the same element', function() {
testNode.innerHTML = "<div data-bind='text: \"Some Text\"'></div>";

// First call is fine
ko.applyBindings({}, testNode);

// Second call throws an error
var didThrow = false;
try { ko.applyBindings({}, testNode); }
catch (ex) {
didThrow = true;
expect(ex.message).toEqual("Knockout doesn't support applying bindings multiple times.");
}
if (!didThrow)
throw new Error("Did not prevent multiple applyBindings calls");
});

it('Should allow multiple applyBindings calls for the same element if cleanNode is used', function() {
testNode.innerHTML = "<div data-bind='text: \"Some Text\"'></div>";

// First call
ko.applyBindings({}, testNode);

// cleanNode called before second call
ko.cleanNode(testNode);
ko.applyBindings({}, testNode);
// Should not throw any errors
});

it('Should allow multiple applyBindings calls for the same element if subsequent call provides a binding', function() {
testNode.innerHTML = "<div data-bind='text: \"Some Text\"'></div>";

// First call uses data-bind
ko.applyBindings({}, testNode);

// Second call provides a binding
ko.applyBindingsToNode(testNode, { visible: false }, {});
// Should not throw any errors
});

it('Should allow multiple applyBindings calls for the same element if initial call provides a binding', function() {
testNode.innerHTML = "<div data-bind='text: \"Some Text\"'></div>";

// First call provides a binding
ko.applyBindingsToNode(testNode, { visible: false }, {});

// Second call uses data-bind
ko.applyBindings({}, testNode);
// Should not throw any errors
});
});
1 change: 1 addition & 0 deletions spec/defaultBindings/valueBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ describe('Binding: Value', function() {
expect(observable()).toEqual("A");

// ... and with value specified before options
ko.utils.domData.clear(testNode);
testNode.innerHTML = "<select data-bind='value:myObservable, options:[\"A\", \"B\"]'></select>";
observable(undefined);
expect(observable()).toEqual(undefined);
Expand Down
7 changes: 4 additions & 3 deletions spec/templatingBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ describe('Templating', function() {

// Bind against initial array containing one entry. UI just shows "original"
var myArray = ko.observableArray(["original"]);
ko.applyBindings({ items: myArray });
ko.applyBindings({ items: myArray }, testNode);
expect(testNode.childNodes[0]).toContainHtml("<div>original</div>");

// Now replace the entire array contents with one different entry.
Expand All @@ -437,7 +437,7 @@ describe('Templating', function() {

// Bind against initial array containing one entry.
var myArray = ko.observableArray(["original"]);
ko.applyBindings({ items: myArray });
ko.applyBindings({ items: myArray }, testNode);
expect(testNode.childNodes[0]).toContainHtml("<div>original</div>inner <span>123</span>x");

// Now replace the entire array contents with one different entry.
Expand All @@ -454,7 +454,7 @@ describe('Templating', function() {

// Bind against array, referencing an observable property
var myItem = { name: ko.observable("a") };
ko.applyBindings({ items: [myItem] });
ko.applyBindings({ items: [myItem] }, testNode);
expect(testNode.childNodes[0]).toContainHtml("<div>a</div>");

// Modify the observable property and check that UI is updated
Expand Down Expand Up @@ -803,6 +803,7 @@ describe('Templating', function() {
testNode.innerHTML = "<div data-bind='template: { name: \"myTemplate\" }'></div>";

var didThrow = false;
ko.utils.domData.clear(testNode);
try { ko.applyBindings({ someData: { childProp: 'abc' } }, testNode) }
catch (ex) {
didThrow = true;
Expand Down
13 changes: 12 additions & 1 deletion src/binding/bindingAttributeSyntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
}
}

var boundElementDomDataKey = '__ko_boundElement';
function applyBindingsToNodeInternal (node, bindings, viewModelOrBindingContext, bindingContextMayDifferFromDomParentElement) {
// Need to be sure that inits are only run once, and updates never run until all the inits have been run
var initPhase = 0; // 0 = before all inits, 1 = during inits, 2 = after all inits
Expand All @@ -89,6 +90,16 @@
}

var bindingHandlerThatControlsDescendantBindings;

// Prevent multiple applyBindings calls for the same node, except when a binding value is specified
var alreadyBound = ko.utils.domData.get(node, boundElementDomDataKey);
if (!bindings) {
if (alreadyBound) {
throw Error("Knockout doesn't support applying bindings multiple times.");
}
ko.utils.domData.set(node, boundElementDomDataKey, true);
}

ko.dependentObservable(
function () {
// Ensure we have a nonnull binding context to work with
Expand All @@ -100,7 +111,7 @@
// Optimization: Don't store the binding context on this node if it's definitely the same as on node.parentNode, because
// we can easily recover it just by scanning up the node's ancestors in the DOM
// (note: here, parent node means "real DOM parent" not "virtual parent", as there's no O(1) way to find the virtual parent)
if (bindingContextMayDifferFromDomParentElement)
if (!alreadyBound && bindingContextMayDifferFromDomParentElement)
ko.storedBindingContextForNode(node, bindingContextInstance);

// Use evaluatedBindings if given, otherwise fall back on asking the bindings provider to give us some bindings
Expand Down

0 comments on commit 17508a1

Please sign in to comment.