Skip to content

Commit

Permalink
Fixes #78 - Elements under onBeforeElChildrenUpdated element remove…
Browse files Browse the repository at this point in the history 
…d if they have `id` set
  • Loading branch information
@patrick-steele-idem
patrick-steele-idem committed Aug 10, 2016
1 parent d7ec47f commit dd1d8c1
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 16 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ Changelog

## 2.0.x

### 2.0.2

- Fixed [#78](https://github.com/patrick-steele-idem/morphdom/issues/78) - Elements under `onBeforeElChildrenUpdated` element removed if they have `id` set

### 2.0.1

- Small optimization and more tests

### 2.0.0

- Fixed [#47](https://github.com/patrick-steele-idem/morphdom/issues/47) - Detect and handle reorder of siblings
Expand Down
84 changes: 68 additions & 16 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,18 +237,34 @@ function morphdom(fromNode, toNode, options) {

// This object is used as a lookup to quickly find all keyed elements in the original DOM tree.
var fromNodesLookup = {};
var keyedRemovalList;

function walkDiscardedChildNodes(node) {
function addKeyedRemoval(key) {
if (keyedRemovalList) {
keyedRemovalList.push(key);
} else {
keyedRemovalList = [key];
}
}

function walkDiscardedChildNodes(node, skipKeyedNodes) {
if (node.nodeType === ELEMENT_NODE) {
var curChild = node.firstChild;
while (curChild) {
if (!getNodeKey(curChild)) {

var key = undefined;

if (skipKeyedNodes && (key = getNodeKey(curChild))) {
// If we are skipping keyed nodes then we add the key
// to a list so that it can be handled at the very end.
addKeyedRemoval(key);
} else {
// Only report the node as discarded if it is not keyed. We do this because
// at the end we loop through all keyed elements that were unmatched
// and then discard them in one final pass.
onNodeDiscarded(curChild);
if (curChild.firstChild) {
walkDiscardedChildNodes(curChild);
walkDiscardedChildNodes(curChild, skipKeyedNodes);
}
}

Expand All @@ -257,7 +273,15 @@ function morphdom(fromNode, toNode, options) {
}
}

function removeNode(node, parentNode) {
/**
* Removes a DOM node out of the original DOM
*
* @param {Node} node The node to remove
* @param {Node} parentNode The nodes parent
* @param {Boolean} skipKeyedNodes If true then elements with keys will be skipped and not discarded.
* @return {undefined}
*/
function removeNode(node, parentNode, skipKeyedNodes) {
if (onBeforeNodeDiscarded(node) === false) {
return;
}
Expand All @@ -267,7 +291,7 @@ function morphdom(fromNode, toNode, options) {
}

onNodeDiscarded(node);
walkDiscardedChildNodes(node);
walkDiscardedChildNodes(node, skipKeyedNodes);
}

// // TreeWalker implementation is no faster, but keeping this around in case this changes in the future
Expand Down Expand Up @@ -340,6 +364,8 @@ function morphdom(fromNode, toNode, options) {

function morphEl(fromEl, toEl, childrenOnly) {
var toElKey = getNodeKey(toEl);
var curFromNodeKey;

if (toElKey) {
// If an element with an ID is being morphed then it is will be in the final
// DOM so clear it out of the saved elements collection
Expand Down Expand Up @@ -373,7 +399,7 @@ function morphdom(fromNode, toNode, options) {
curToNodeKey = getNodeKey(curToNodeChild);

while (curFromNodeChild) {
var curFromNodeKey = getNodeKey(curFromNodeChild);
curFromNodeKey = getNodeKey(curFromNodeChild);
fromNextSibling = curFromNodeChild.nextSibling;

var curFromNodeType = curFromNodeChild.nodeType;
Expand Down Expand Up @@ -409,8 +435,15 @@ function morphdom(fromNode, toNode, options) {
// all lifecycle hooks are correctly invoked
fromEl.insertBefore(matchingFromEl, curFromNodeChild);

if (!curFromNodeKey) {
removeNode(curFromNodeChild, fromEl);
if (curFromNodeKey) {
// Since the node is keyed it might be matched up later so we defer
// the actual removal to later
addKeyedRemoval(curFromNodeKey);
} else {
// NOTE: we skip nested keyed nodes from being removed since there is
// still a chance they will be matched up later
removeNode(curFromNodeChild, fromEl, true /* skip keyed nodes */);

}
fromNextSibling = curFromNodeChild.nextSibling;
curFromNodeChild = matchingFromEl;
Expand Down Expand Up @@ -456,8 +489,14 @@ function morphdom(fromNode, toNode, options) {
// target tree and we don't want to discard it just yet since it still might find a
// home in the final DOM tree. After everything is done we will remove any keyed nodes
// that didn't find a home
if (!curFromNodeKey) {
removeNode(curFromNodeChild, fromEl);
if (curFromNodeKey) {
// Since the node is keyed it might be matched up later so we defer
// the actual removal to later
addKeyedRemoval(curFromNodeKey);
} else {
// NOTE: we skip nested keyed nodes from being removed since there is
// still a chance they will be matched up later
removeNode(curFromNodeChild, fromEl, true /* skip keyed nodes */);
}

curFromNodeChild = fromNextSibling;
Expand Down Expand Up @@ -486,8 +525,14 @@ function morphdom(fromNode, toNode, options) {
// to be removed
while (curFromNodeChild) {
fromNextSibling = curFromNodeChild.nextSibling;
if (!getNodeKey(curFromNodeChild)) {
removeNode(curFromNodeChild, fromEl);
if ((curFromNodeKey = getNodeKey(curFromNodeChild))) {
// Since the node is keyed it might be matched up later so we defer
// the actual removal to later
addKeyedRemoval(curFromNodeKey);
} else {
// NOTE: we skip nested keyed nodes from being removed since there is
// still a chance they will be matched up later
removeNode(curFromNodeChild, fromEl, true /* skip keyed nodes */);
}
curFromNodeChild = fromNextSibling;
}
Expand Down Expand Up @@ -534,10 +579,17 @@ function morphdom(fromNode, toNode, options) {
} else {
morphEl(morphedNode, toNode, childrenOnly);

for (var k in fromNodesLookup) {
var elToRemove = fromNodesLookup[k];
if (elToRemove) {
removeNode(elToRemove, elToRemove.parentNode);
// We now need to loop over any keyed nodes that might need to be
// removed. We only do the removal if we know that the keyed node
// never found a match. When a keyed node is matched up we remove
// it out of fromNodesLookup and we use fromNodesLookup to determine
// if a keyed node has been matched up or not
if (keyedRemovalList) {
for (var i=0, len=keyedRemovalList.length; i<len; i++) {
var elToRemove = fromNodesLookup[keyedRemovalList[i]];
if (elToRemove) {
removeNode(elToRemove, elToRemove.parentNode, false);
}
}
}
}
Expand Down
36 changes: 36 additions & 0 deletions test/browser/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,42 @@ function addTests() {
expect(finalEl.innerHTML).to.equal('<div id="boo"></div>');
});

it('should not remove keyed elements that are part of a DOM subtree that is skipped using onBeforeElUpdated', function() {
var el1 = document.createElement('div');
el1.innerHTML = '<span id="skipMe"><span id="skipMeChild"></span></span>';

var el2 = document.createElement('div');
el2.innerHTML = '<span id="skipMe"></div>';

morphdom(el1, el2, {
onBeforeElUpdated: function(el) {
if (el.id === 'skipMe') {
return false;
}
}
});

expect(el1.querySelector('#skipMeChild') != null).to.equal(true);
});

it('should not remove keyed elements that are part of a DOM subtree that is skipped using onBeforeElChildrenUpdated', function() {
var el1 = document.createElement('div');
el1.innerHTML = '<span id="skipMe"><span id="skipMeChild"></span></span>';

var el2 = document.createElement('div');
el2.innerHTML = '<span id="skipMe"></div>';

morphdom(el1, el2, {
onBeforeElChildrenUpdated: function(el) {
if (el.id === 'skipMe') {
return false;
}
}
});

expect(el1.querySelector('#skipMeChild') != null).to.equal(true);
});

// xit('should reuse DOM element with matching ID and class name (2)', function() {
// // NOTE: This test is currently failing. We need to improve the special case code
// // for handling incompatible root nodes.
Expand Down

0 comments on commit dd1d8c1

Please sign in to comment.