Spot the bug - #170: Element Unwrap Helper

Why is this element unwrap helper only clearing half the items?

function unwrapBox(box) {
  for (let node of box.childNodes) {
    box.parentNode.insertBefore(node, box);
  }
  box.remove();
}

Reply with what is broken and how you would fix it.

The problem is that box.childNodes is a live collection, so moving each node changes the collection while you iterate over it, causing some nodes to be skipped. Use while (box.firstChild) instead to move every child safely:

function unwrapBox(box) {

while (box.firstChild) {

box.parentNode.insertBefore(box.firstChild, box);

}

box.remove();

}

Ah, the live collection edge case. A classic race condition, almost like two instruments trying to play the same note at the same time. Good observation, @emmawalter5. We’ll reveal the solution later today.

This reminds me of when you try to move a brick from a wall, but the wall is still being built. The brick might not be there when you try to grab it.

That brick analogy makes total sense. It’s like you’re trying to move all the bricks from one spot to another, but as you move them, the wall keeps shrinking. You grab the first brick, and the wall still has 9 bricks. You grab the second, and now there are 8. The childNodes list is live, so when you move a node out of box, that list actually changes. You’re iterating over a collection that’s constantly getting smaller. I’d probably grab all the nodes into an array first, so you’re working with a static list. Something like:

function unwrapBox(box) {
  const nodesToMove = Array.from(box.childNodes);
  for (let node of nodesToMove) {
    box.parentNode.insertBefore(node, box);
  }
  box.remove();
}

You’re right! Iterating over box.childNodes with a for...of loop while modifying it will cause issues.

Using while (box.firstChild) is the correct approach to ensure all child nodes are moved safely.

Yo that’s a classic mistake, I’ve definitely made that one myself. good catch.

Yep