Skip to content

Commit

Permalink
bugfix: element with an IDed child might not have its removal hook ca…
Browse files Browse the repository at this point in the history
…lled if the child is moved elsewhere.
  • Loading branch information
MichaelWest22 authored and botandrose-machine committed Feb 1, 2025
1 parent 98cfcbd commit 0ab4f9b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
23 changes: 23 additions & 0 deletions src/idiomorph.js
Original file line number Diff line number Diff line change
Expand Up @@ -535,10 +535,33 @@ var Idiomorph = (function () {
ctx.target.querySelector(`#${id}`) ||
ctx.pantry.querySelector(`#${id}`)
);
removeElementFromAncestorsIdMaps(target, ctx);
moveBefore(parentNode, target, after);
return target;
}

/**
* Removes an element from its ancestors' id maps. This is needed when an element is moved from the
* "future" via `moveBeforeId`. Otherwise, its erstwhile ancestors could be mistakenly moved to the
* pantry rather than being deleted, preventing their removal hooks from being called.
*
* @param {Element} element - element to remove from its ancestors' id maps
* @param {MorphContext} ctx
*/
function removeElementFromAncestorsIdMaps(element, ctx) {
const id = element.id;
/** @ts-ignore - safe to loop in this way **/
while ((element = element.parentNode)) {
let idSet = ctx.idMap.get(element);
if (idSet) {
idSet.delete(id);
if (!idSet.size) {
ctx.idMap.delete(element);
}
}
}
}

/**
* Moves an element before another element within the same parent.
* Uses the proposed `moveBefore` API if available (and working), otherwise falls back to `insertBefore`.
Expand Down
18 changes: 18 additions & 0 deletions test/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,24 @@ describe("lifecycle hooks", function () {
initial.outerHTML.should.equal("<ul><li>A</li><li>B</li></ul>");
});

it("returning false to beforeNodeRemoved prevents removing the node when an IDed child is moved elsewhere", function () {
let initial = make(
`<ul><li><a id="a">A</a></li><li><b id="b">B</b></li></ul>`,
);
Idiomorph.morph(
initial,
`<ul><li><a id="a">A</a><b id="b">B</b></li></ul>`,
{
callbacks: {
beforeNodeRemoved: (node) => false,
},
},
);
initial.outerHTML.should.equal(
`<ul><li><a id="a">A</a><b id="b">B</b></li><li></li></ul>`,
);
});

it("returning false to beforeNodeRemoved prevents removing the node with different tag types", function () {
let initial = make("<div><a>A</a><b>B</b><c>C</c></div>");
Idiomorph.morph(initial, "<div><b>B</b></div>", {
Expand Down

0 comments on commit 0ab4f9b

Please sign in to comment.