Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Rename fixUpVirtualElements to fixUpNodesToBeRemoved and amend commen…

…ts, since the function now has a different role
  • Loading branch information...
commit c558d081be103fc9ab84f7ed8a8fa95d9bc4727f 1 parent 3ec0059
@SteveSanderson SteveSanderson authored
Showing with 18 additions and 11 deletions.
  1. +18 −11 src/binding/editDetection/arrayToDomNodeChildren.js
View
29 src/binding/editDetection/arrayToDomNodeChildren.js
@@ -10,18 +10,25 @@
// "callbackAfterAddingNodes" will be invoked after any "mapping"-generated nodes are inserted into the container node
// You can use this, for example, to activate bindings on those nodes.
- function fixUpVirtualElements(contiguousNodeArray) {
- // Re-written templates insert comments into the node list during memoization and then remove them during un-memoization.
- // If the first node in the list is one of those comments, we need to remove it from the list and get the actual first node.
- // See https://github.com/SteveSanderson/knockout/pull/440
+ function fixUpNodesToBeRemoved(contiguousNodeArray) {
+ // Before deleting or replacing a set of nodes that were previously outputted by the "map" function, we have to reconcile
+ // them against what is in the DOM right now. It may be that some of the nodes have already been removed from the document,
+ // or that new nodes might have been inserted in the middle, for example by a binding. Also, there may previously have been
+ // leading comment nodes (created by rewritten string-based templates) that have since been removed during binding.
+ // So, this function translates the old "map" output array into its best guess of what set of current DOM nodes should be removed.
+ //
+ // Rules:
+ // [A] Any leading nodes that aren't in the document any more should be ignored
+ // These most likely correspond to memoization nodes that were already removed during binding
+ // See https://github.com/SteveSanderson/knockout/pull/440
+ // [B] We want to output a contiguous series of nodes that are still in the document. So, ignore any nodes that
+ // have already been removed, and include any nodes that have been inserted among the previous collection
+
+ // Rule [A]
while (contiguousNodeArray.length && !ko.utils.domNodeIsAttachedToDocument(contiguousNodeArray[0]))
contiguousNodeArray.splice(0, 1);
- // Ensures that contiguousNodeArray really *is* an array of contiguous siblings, even if some of the interior
- // ones have changed since your array was first built (e.g., because your array contains virtual elements, and
- // their virtual children changed when binding was applied to them).
- // This is needed so that we can reliably remove or update the nodes corresponding to a given array item
-
+ // Rule [B]
if (contiguousNodeArray.length > 1) {
// Build up the actual new contiguous node set
var current = contiguousNodeArray[0], last = contiguousNodeArray[contiguousNodeArray.length - 1], newContiguousSet = [current];
@@ -46,7 +53,7 @@
// On subsequent evaluations, just replace the previously-inserted DOM nodes
if (mappedNodes.length > 0) {
- fixUpVirtualElements(mappedNodes);
+ fixUpNodesToBeRemoved(mappedNodes);
ko.utils.replaceDomNodes(mappedNodes, newMappedNodes);
if (callbackAfterAddingNodes)
callbackAfterAddingNodes(valueToMap, newMappedNodes);
@@ -95,7 +102,7 @@
lastMappingResult[lastMappingResultIndex].dependentObservable.dispose();
// Queue these nodes for later removal
- fixUpVirtualElements(lastMappingResult[lastMappingResultIndex].domNodes);
+ fixUpNodesToBeRemoved(lastMappingResult[lastMappingResultIndex].domNodes);
ko.utils.arrayForEach(lastMappingResult[lastMappingResultIndex].domNodes, function (node) {
nodesToDelete.push({
element: node,
Please sign in to comment.
Something went wrong with that request. Please try again.