Fix ChildNode.replaceWith throwing instead of replacing the node

.changeset/polyfill-insert-before-sibling-chain.md

@@ -0,0 +1,11 @@
+---
+'@remote-dom/polyfill': patch
+---
+
+Fix `insertBefore()` leaving the previous sibling pointing past the inserted node
+
+`ParentNode.insertBefore()` relinked the new child's `NEXT`/`PREV` and the reference node's `PREV`, but only repointed the previous sibling's `NEXT` when the reference node was the first child. Inserting before any _later_ child therefore left the forward sibling chain skipping the new node.
+
+That chain is what `querySelector()`, `querySelectorAll()`, and `firstChild`/`nextSibling` traversal walk, so an affected node was unreachable from every selector query and sibling walk — while `childNodes`, `parentNode`, `previousSibling`, and the host mirror all still reported it as present. The node rendered correctly on the host and was invisible to the remote environment's own lookups, with no error raised. `replaceChild()`, `prepend()` (of more than one node), and `ChildNode.after()` all route through the same branch and were affected too.
+
+This is easy to hit whenever a framework renders ahead of existing content — for example inserting content before an already-appended `<style>` element — where it silently breaks code as ordinary as `document.querySelector('#my-modal').showModal()`.

.changeset/polyfill-replace-with.md

@@ -0,0 +1,9 @@
+---
+'@remote-dom/polyfill': patch
+---
+
+Fix `ChildNode.replaceWith()` throwing instead of replacing the node
+
+`replaceWith()` passed its arguments to `replaceChild()` in the wrong order — `replaceChild(newChild, oldChild)` was called as `parent.replaceChild(this, node)`, naming the incoming node as the child to replace. Since that node is usually fresh and has no parent, the reference check rejected it and every call threw `reference node is not a child of this parent`. It also read the following sibling off the incoming node rather than off `this`, so the remaining arguments had no correct insertion point to anchor to.
+
+The method now removes `this` and inserts the given nodes at its position, in argument order, anchored on the first following sibling that is not itself being moved. Strings become text nodes, calling it with no arguments removes the node (matching `remove()`), and a node with no parent is still left alone.

packages/polyfill/source/ChildNode.ts

@@ -12,11 +12,14 @@ export class ChildNode extends Node {
   replaceWith(...nodes: (Node | string)[]) {
     const parent = this.parentNode;
     if (!parent) return;
-    const node = toNode(parent, nodes[0]);
-    const next = node[NEXT];
-    parent.replaceChild(this, node);
-    for (let i = 1; i < nodes.length; i++) {
-      parent.insertBefore(toNode(parent, nodes[i]), next);
+    // Anchor on the first following sibling that isn't itself being moved, so
+    // that replacing a node with one of its own siblings still has a reference
+    // node left to insert before.
+    let next = this[NEXT];
+    while (next && nodes.includes(next)) next = next[NEXT];
+    parent.removeChild(this);
+    for (const node of nodes) {
+      parent.insertBefore(toNode(parent, node), next);
     }
   }
 

packages/polyfill/source/ParentNode.ts

@@ -121,9 +121,11 @@ export class ParentNode extends ChildNode {
       if (before.parentNode !== this) {
         throw Error('reference node is not a child of this parent');
       }
+      const prev = before[PREV];
       child[NEXT] = before;
-      child[PREV] = before[PREV];
-      if (before[PREV] === null) this[CHILD] = child;
+      child[PREV] = prev;
+      if (prev === null) this[CHILD] = child;
+      else prev[NEXT] = child;
       before[PREV] = child;
     } else {
       child[NEXT] = null;

packages/polyfill/source/tests/ChildNode.test.ts

@@ -0,0 +1,178 @@
+import {Window} from '../index.ts';
+
+import {describe, it, expect, beforeEach} from 'vitest';
+
+function siblingChain(parent: ParentNode) {
+  const names: string[] = [];
+  for (let node = parent.firstChild; node; node = node.nextSibling) {
+    names.push(node.nodeName.toLowerCase());
+  }
+  return names;
+}
+
+describe('ChildNode', () => {
+  beforeEach(() => {
+    const window = new Window();
+    Window.setGlobalThis(window);
+  });
+
+  describe('replaceWith', () => {
+    let first: Element;
+    let target: Element;
+    let last: Element;
+
+    beforeEach(() => {
+      first = document.createElement('first-child');
+      target = document.createElement('target-child');
+      last = document.createElement('last-child');
+      document.body.append(first, target, last);
+    });
+
+    it('replaces a middle child with a single node', () => {
+      const replacement = document.createElement('replacement-child');
+
+      target.replaceWith(replacement);
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'replacement-child',
+        'last-child',
+      ]);
+      expect(replacement.parentNode).toBe(document.body);
+      expect(target.parentNode).toBeNull();
+      expect(document.querySelector('replacement-child')).toBe(replacement);
+    });
+
+    it('replaces the last child', () => {
+      const replacement = document.createElement('replacement-child');
+
+      last.replaceWith(replacement);
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'target-child',
+        'replacement-child',
+      ]);
+      expect(document.body.lastChild).toBe(replacement);
+    });
+
+    it('replaces the first child', () => {
+      const replacement = document.createElement('replacement-child');
+
+      first.replaceWith(replacement);
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'replacement-child',
+        'target-child',
+        'last-child',
+      ]);
+      expect(document.body.firstChild).toBe(replacement);
+    });
+
+    it('inserts multiple nodes in argument order', () => {
+      target.replaceWith(
+        document.createElement('replacement-one'),
+        document.createElement('replacement-two'),
+      );
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'replacement-one',
+        'replacement-two',
+        'last-child',
+      ]);
+    });
+
+    it('inserts strings as text nodes', () => {
+      target.replaceWith('replacement text');
+
+      expect(document.body.childNodes[1]!.nodeType).toBe(3);
+      expect(document.body.childNodes[1]!.textContent).toBe('replacement text');
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        '#text',
+        'last-child',
+      ]);
+    });
+
+    it('mixes nodes and strings', () => {
+      target.replaceWith(
+        'before ',
+        document.createElement('mixed-child'),
+        ' after',
+      );
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        '#text',
+        'mixed-child',
+        '#text',
+        'last-child',
+      ]);
+    });
+
+    it('removes the node when called with no arguments', () => {
+      target.replaceWith();
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'last-child',
+      ]);
+      expect(target.parentNode).toBeNull();
+    });
+
+    it('does nothing to a node without a parent', () => {
+      const detached = document.createElement('detached-child');
+
+      expect(() =>
+        detached.replaceWith(document.createElement('replacement-child')),
+      ).not.toThrow();
+      expect(detached.parentNode).toBeNull();
+    });
+
+    it('moves a node that is already in the document', () => {
+      target.replaceWith(first);
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'last-child',
+      ]);
+      expect(first.parentNode).toBe(document.body);
+      expect(target.parentNode).toBeNull();
+    });
+
+    it('replaces a node with its own next sibling', () => {
+      target.replaceWith(last);
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'last-child',
+      ]);
+      expect(target.parentNode).toBeNull();
+    });
+
+    it('replaces a node with itself', () => {
+      target.replaceWith(target);
+
+      expect(siblingChain(document.body)).toStrictEqual([
+        'first-child',
+        'target-child',
+        'last-child',
+      ]);
+      expect(target.parentNode).toBe(document.body);
+    });
+
+    it('keeps the sibling chain and childNodes in agreement', () => {
+      target.replaceWith(
+        document.createElement('replacement-one'),
+        document.createElement('replacement-two'),
+      );
+
+      expect(siblingChain(document.body)).toStrictEqual(
+        Array.from(document.body.childNodes, (node) =>
+          node.nodeName.toLowerCase(),
+        ),
+      );
+    });
+  });
+});