Skip to content

Commit

Permalink
Use arrayChange event to update foreach binding. This improves perfor…
Browse files Browse the repository at this point in the history 
…mance when known operations make changes to large arrays.

In addition, add ko.isObservableArray and subscription.disposeWhenNodeIsRemoved
  • Loading branch information
@mbest
mbest committed Dec 3, 2017
1 parent 8237b7f commit dcfaf69
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 85 deletions.
10 changes: 7 additions & 3 deletions spec/defaultBindings/foreachBehaviors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('Binding: Foreach', function() {


it('Should add and remove nodes to match changes in the bound array', function() {
testNode.innerHTML = "<div data-bind='foreach: someItems'><span data-bind='text: childProp'></span></div>";
testNode.innerHTML = "<div data-bind='foreach: { data: someItems, includeDestroyed: false }'><span data-bind='text: childProp'></span></div>";
var someItems = ko.observableArray([
{ childProp: 'first child' },
{ childProp: 'second child' }
Expand Down Expand Up @@ -156,8 +156,8 @@ describe('Binding: Foreach', function() {
var afterAddCallbackData = [], beforeRemoveCallbackData = [];
ko.applyBindings({
someItems: someItems,
myAfterAdd: function(elem, index, value) { afterAddCallbackData.push({ elem: elem, value: value, currentParentClone: elem.parentNode.cloneNode(true) }) },
myBeforeRemove: function(elem, index, value) { beforeRemoveCallbackData.push({ elem: elem, value: value, currentParentClone: elem.parentNode.cloneNode(true) }) }
myAfterAdd: function(elem, index, value) { afterAddCallbackData.push({ elem: elem, index: index, value: value, currentParentClone: elem.parentNode.cloneNode(true) }) },
myBeforeRemove: function(elem, index, value) { beforeRemoveCallbackData.push({ elem: elem, index: index, value: value, currentParentClone: elem.parentNode.cloneNode(true) }) }
}, testNode);

expect(testNode.childNodes[0]).toContainHtml('<span data-bind="text: $data">first child</span>');
Expand All @@ -167,13 +167,15 @@ describe('Binding: Foreach', function() {
expect(testNode.childNodes[0]).toContainHtml('<span data-bind="text: $data">first child</span><span data-bind="text: $data">added child</span>');
expect(afterAddCallbackData.length).toEqual(1);
expect(afterAddCallbackData[0].elem).toEqual(testNode.childNodes[0].childNodes[1]);
expect(afterAddCallbackData[0].index).toEqual(0);
expect(afterAddCallbackData[0].value).toEqual("added child");
expect(afterAddCallbackData[0].currentParentClone).toContainHtml('<span data-bind="text: $data">first child</span><span data-bind="text: $data">added child</span>');

// Try removing
someItems.shift();
expect(beforeRemoveCallbackData.length).toEqual(1);
expect(beforeRemoveCallbackData[0].elem).toContainText("first child");
expect(beforeRemoveCallbackData[0].index).toEqual(0);
expect(beforeRemoveCallbackData[0].value).toEqual("first child");
// Note that when using "beforeRemove", we *don't* remove the node from the doc - it's up to the beforeRemove callback to do it. So, check it's still there.
expect(beforeRemoveCallbackData[0].currentParentClone).toContainHtml('<span data-bind="text: $data">first child</span><span data-bind="text: $data">added child</span>');
Expand All @@ -184,6 +186,7 @@ describe('Binding: Foreach', function() {
someItems.shift();
expect(beforeRemoveCallbackData.length).toEqual(1);
expect(beforeRemoveCallbackData[0].elem).toContainText("added child");
expect(beforeRemoveCallbackData[0].index).toEqual(0);
expect(beforeRemoveCallbackData[0].value).toEqual("added child");
// Neither item has yet been removed and both are still in their original locations
expect(beforeRemoveCallbackData[0].currentParentClone).toContainHtml('<span data-bind="text: $data">first child</span><span data-bind="text: $data">added child</span>');
Expand All @@ -196,6 +199,7 @@ describe('Binding: Foreach', function() {
expect(testNode.childNodes[0]).toContainHtml('<span data-bind="text: $data">added child</span>');
expect(afterAddCallbackData.length).toEqual(1);
expect(afterAddCallbackData[0].elem).toEqual(testNode.childNodes[0].childNodes[0]);
expect(afterAddCallbackData[0].index).toEqual(0);
expect(afterAddCallbackData[0].value).toEqual("added child");
expect(afterAddCallbackData[0].currentParentClone).toContainHtml('<span data-bind="text: $data">added child</span>');
});
Expand Down
17 changes: 17 additions & 0 deletions spec/observableArrayBehaviors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,23 @@ describe('Observable Array', function() {
expect(ko.isObservable(testObservableArray)).toEqual(true);
});

it('Should advertise as observable array', function () {
expect(ko.isObservableArray(ko.observableArray())).toEqual(true);
});

it('ko.isObservableArray should return false for non-observable array values', function () {
ko.utils.arrayForEach([
undefined,
null,
"x",
{},
function() {},
ko.observable([]),
], function (value) {
expect(ko.isObservableArray(value)).toEqual(false);
});
});

it('Should initialize to empty array if you pass no args to constructor', function() {
var instance = new ko.observableArray();
expect(instance().length).toEqual(0);
Expand Down
16 changes: 14 additions & 2 deletions spec/templatingBehaviors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,10 +815,10 @@ describe('Templating', function() {
expect(innerObservable.getSubscriptionsCount()).toEqual(0);
});

it('Should omit any items whose \'_destroy\' flag is set (unwrapping the flag if it is observable)', function() {
it('Should omit any items whose \'_destroy\' flag is set (unwrapping the flag if it is observable) if includeDestroyed is false', function() {
var myArray = new ko.observableArray([{ someProp: 1 }, { someProp: 2, _destroy: 'evals to true' }, { someProp : 3 }, { someProp: 4, _destroy: ko.observable(false) }]);
ko.setTemplateEngine(new dummyTemplateEngine({ itemTemplate: "<div>someProp=[js: someProp]</div>" }));
testNode.innerHTML = "<div data-bind='template: { name: \"itemTemplate\", foreach: myCollection }'></div>";
testNode.innerHTML = "<div data-bind='template: { name: \"itemTemplate\", foreach: myCollection, includeDestroyed: false }'></div>";

ko.applyBindings({ myCollection: myArray }, testNode);
expect(testNode.childNodes[0]).toContainHtml("<div>someprop=1</div><div>someprop=3</div><div>someprop=4</div>");
Expand All @@ -833,6 +833,18 @@ describe('Templating', function() {
expect(testNode.childNodes[0]).toContainHtml("<div>someprop=1</div><div>someprop=2</div><div>someprop=3</div>");
});

it('Should omit any items whose \'_destroy\' flag is set if foreachHidesDestroyed is set', function() {
this.restoreAfter(ko.options, 'foreachHidesDestroyed');
ko.options.foreachHidesDestroyed = true;

var myArray = new ko.observableArray([{ someProp: 1 }, { someProp: 2, _destroy: 'evals to true' }, { someProp : 3 }, { someProp: 4, _destroy: false }]);
ko.setTemplateEngine(new dummyTemplateEngine({ itemTemplate: "<div>someProp=[js: someProp]</div>" }));
testNode.innerHTML = "<div data-bind='template: { name: \"itemTemplate\", foreach: myCollection }'></div>";

ko.applyBindings({ myCollection: myArray }, testNode);
expect(testNode.childNodes[0]).toContainHtml("<div>someprop=1</div><div>someprop=3</div><div>someprop=4</div>");
});

it('Should be able to render a different template for each array entry by passing a function as template name, with the array entry\'s binding context available as a second parameter', function() {
var myArray = new ko.observableArray([
{ preferredTemplate: 1, someProperty: 'firstItemValue' },
Expand Down
144 changes: 86 additions & 58 deletions src/binding/editDetection/arrayToDomNodeChildren.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@
var lastMappingResultDomDataKey = ko.utils.domData.nextKey(),
deletedItemDummyValue = ko.utils.domData.nextKey();

ko.utils.setDomNodeChildrenFromArrayMapping = function (domNode, array, mapping, options, callbackAfterAddingNodes) {
// Compare the provided array against the previous one
ko.utils.setDomNodeChildrenFromArrayMapping = function (domNode, array, mapping, options, callbackAfterAddingNodes, editScript) {
array = array || [];
if (typeof array.length == "undefined") // Coerce single value into array
array = [array];

options = options || {};
var isFirstExecution = ko.utils.domData.get(domNode, lastMappingResultDomDataKey) === undefined;
var lastMappingResult = ko.utils.domData.get(domNode, lastMappingResultDomDataKey) || [];
var lastArray = ko.utils.arrayMap(lastMappingResult, function (x) { return x.arrayEntry; });
var editScript = ko.utils.compareArrays(lastArray, array, options['dontLimitMoves']);
var lastMappingResult = ko.utils.domData.get(domNode, lastMappingResultDomDataKey);
var isFirstExecution = !lastMappingResult;

// Build the new mapping result
var newMappingResult = [];
Expand All @@ -53,11 +53,21 @@
var itemsForMoveCallbacks = [];
var itemsForAfterAddCallbacks = [];
var mapData;
var countWaitingForRemove = 0;

function itemAdded(value) {
mapData = { arrayEntry: value, indexObservable: ko.observable(newMappingResultIndex++) };
newMappingResult.push(mapData);
itemsToProcess.push(mapData);
if (!isFirstExecution) {
itemsForAfterAddCallbacks.push(mapData);
}
}

function itemMovedOrRetained(editScriptIndex, oldPosition) {
function itemMovedOrRetained(oldPosition) {
mapData = lastMappingResult[oldPosition];
if (newMappingResultIndex !== oldPosition)
itemsForMoveCallbacks[editScriptIndex] = mapData;
itemsForMoveCallbacks.push(mapData);
// Since updating the index might change the nodes, do so before calling fixUpContinuousNodeArray
mapData.indexObservable(newMappingResultIndex++);
ko.utils.fixUpContinuousNodeArray(mapData.mappedNodes, domNode);
Expand All @@ -68,63 +78,83 @@
function callCallback(callback, items) {
if (callback) {
for (var i = 0, n = items.length; i < n; i++) {
if (items[i]) {
ko.utils.arrayForEach(items[i].mappedNodes, function(node) {
callback(node, i, items[i].arrayEntry);
});
}
ko.utils.arrayForEach(items[i].mappedNodes, function(node) {
callback(node, i, items[i].arrayEntry);
});
}
}
}

for (var i = 0, editScriptItem, movedIndex; editScriptItem = editScript[i]; i++) {
movedIndex = editScriptItem['moved'];
switch (editScriptItem['status']) {
case "deleted":
if (movedIndex === undefined) {
mapData = lastMappingResult[lastMappingResultIndex];

// Stop tracking changes to the mapping for these nodes
if (mapData.dependentObservable) {
mapData.dependentObservable.dispose();
mapData.dependentObservable = undefined;
if (isFirstExecution) {
ko.utils.arrayForEach(array, itemAdded);
} else {
if (!editScript || (lastMappingResult && lastMappingResult['_countWaitingForRemove'])) {
// Compare the provided array against the previous one
var lastArray = isFirstExecution ? [] : ko.utils.arrayMap(lastMappingResult, function (x) { return x.arrayEntry; }),
compareOptions = {
'dontLimitMoves': options['dontLimitMoves'],
'sparse': true
};
editScript = ko.utils.compareArrays(lastArray, array, compareOptions);
}

for (var i = 0, editScriptItem, movedIndex, itemIndex; editScriptItem = editScript[i]; i++) {
movedIndex = editScriptItem['moved'];
itemIndex = editScriptItem['index'];
switch (editScriptItem['status']) {
case "deleted":
while (lastMappingResultIndex < itemIndex) {
itemMovedOrRetained(lastMappingResultIndex++);
}
if (movedIndex === undefined) {
mapData = lastMappingResult[lastMappingResultIndex];

// Queue these nodes for later removal
if (ko.utils.fixUpContinuousNodeArray(mapData.mappedNodes, domNode).length) {
if (options['beforeRemove']) {
newMappingResult.push(mapData);
itemsToProcess.push(mapData);
if (mapData.arrayEntry === deletedItemDummyValue) {
mapData = null;
} else {
itemsForBeforeRemoveCallbacks[i] = mapData;
}
// Stop tracking changes to the mapping for these nodes
if (mapData.dependentObservable) {
mapData.dependentObservable.dispose();
mapData.dependentObservable = undefined;
}
if (mapData) {
nodesToDelete.push.apply(nodesToDelete, mapData.mappedNodes);

// Queue these nodes for later removal
if (ko.utils.fixUpContinuousNodeArray(mapData.mappedNodes, domNode).length) {
if (options['beforeRemove']) {
newMappingResult.push(mapData);
itemsToProcess.push(mapData);
countWaitingForRemove++;
if (mapData.arrayEntry === deletedItemDummyValue) {
mapData = null;
} else {
itemsForBeforeRemoveCallbacks.push(mapData);
}
}
if (mapData) {
nodesToDelete.push.apply(nodesToDelete, mapData.mappedNodes);
}
}
}
}
lastMappingResultIndex++;
break;

case "retained":
itemMovedOrRetained(i, lastMappingResultIndex++);
break;

case "added":
if (movedIndex !== undefined) {
itemMovedOrRetained(i, movedIndex);
} else {
mapData = { arrayEntry: editScriptItem['value'], indexObservable: ko.observable(newMappingResultIndex++) };
newMappingResult.push(mapData);
itemsToProcess.push(mapData);
if (!isFirstExecution)
itemsForAfterAddCallbacks[i] = mapData;
}
break;
lastMappingResultIndex++;
break;

case "added":
while (newMappingResultIndex < itemIndex) {
itemMovedOrRetained(lastMappingResultIndex++);
}
if (movedIndex !== undefined) {
itemMovedOrRetained(movedIndex);
} else {
itemAdded(editScriptItem['value']);
}
break;
}
}

while (newMappingResultIndex < array.length) {
itemMovedOrRetained(lastMappingResultIndex++);
}

// Record that the current view may still contain deleted items
// because it means we won't be able to use a provided editScript.
newMappingResult['_countWaitingForRemove'] = countWaitingForRemove;
}

// Store a copy of the array items we just considered so we can difference it next time
Expand Down Expand Up @@ -166,9 +196,7 @@
// as already "removed" so we won't call beforeRemove for it again, and it ensures that the item won't match up
// with an actual item in the array and appear as "retained" or "moved".
for (i = 0; i < itemsForBeforeRemoveCallbacks.length; ++i) {
if (itemsForBeforeRemoveCallbacks[i]) {
itemsForBeforeRemoveCallbacks[i].arrayEntry = deletedItemDummyValue;
}
itemsForBeforeRemoveCallbacks[i].arrayEntry = deletedItemDummyValue;
}

// Finally call afterMove and afterAdd callbacks
Expand Down
3 changes: 2 additions & 1 deletion src/options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
ko.options = {
'deferUpdates': false,
'useOnlyNativeEvents': false,
'noChildContextWithAs': false
'noChildContextWithAs': false,
'foreachHidesDestroyed': false
};

//ko.exportSymbol('options', ko.options); // 'options' isn't minified
7 changes: 7 additions & 0 deletions src/subscribables/observableArray.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,11 @@ ko.utils.arrayForEach(["slice"], function (methodName) {
};
});

ko.isObservableArray = function (instance) {
return ko.isObservable(instance)
&& typeof instance["remove"] == "function"
&& typeof instance["push"] == "function";
};

ko.exportSymbol('observableArray', ko.observableArray);
ko.exportSymbol('isObservableArray', ko.isObservableArray);
24 changes: 17 additions & 7 deletions src/subscribables/subscribable.js
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@

ko.subscription = function (target, callback, disposeCallback) {
this._target = target;
this.callback = callback;
this.disposeCallback = disposeCallback;
this.isDisposed = false;
this._callback = callback;
this._disposeCallback = disposeCallback;
this._isDisposed = false;
this._node = null;
this._domNodeDisposalCallback = null;
ko.exportProperty(this, 'dispose', this.dispose);
ko.exportProperty(this, 'disposeWhenNodeIsRemoved', this.disposeWhenNodeIsRemoved);
};
ko.subscription.prototype.dispose = function () {
this.isDisposed = true;
this.disposeCallback();
if (this._domNodeDisposalCallback) {
ko.utils.domNodeDisposal.removeDisposeCallback(this._node, this._domNodeDisposalCallback);
}
this._isDisposed = true;
this._disposeCallback();
};
ko.subscription.prototype.disposeWhenNodeIsRemoved = function (node) {
this._node = node;
ko.utils.domNodeDisposal.addDisposeCallback(node, this._domNodeDisposalCallback = this.dispose.bind(this));
};

ko.subscribable = function () {
Expand Down Expand Up @@ -69,8 +79,8 @@ var ko_subscribable_fn = {
for (var i = 0, subscription; subscription = subs[i]; ++i) {
// In case a subscription was disposed during the arrayForEach cycle, check
// for isDisposed on each subscription before invoking its callback
if (!subscription.isDisposed)
subscription.callback(valueToNotify);
if (!subscription._isDisposed)
subscription._callback(valueToNotify);
}
} finally {
ko.dependencyDetection.end(); // End suppressing dependency detection
Expand Down
Loading

0 comments on commit dcfaf69

Please sign in to comment.