From 1868da7738e2aa9ebcfe76be5dc02429d611931f Mon Sep 17 00:00:00 2001 From: Harry Brundage Date: Thu, 16 Jul 2026 11:51:09 -0400 Subject: [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. That node is usually fresh and has no parent, so the reference check rejected it and every call on a node with a parent 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, and a call with no arguments stringified undefined into a text node instead of removing the node. Remove this and insert the given nodes at its position in argument order, anchored on the first following sibling that is not itself being moved so that replacing a node with one of its own siblings still has a reference node left. Co-Authored-By: Claude --- .changeset/polyfill-replace-with.md | 9 + packages/polyfill/source/ChildNode.ts | 13 +- .../polyfill/source/tests/ChildNode.test.ts | 178 ++++++++++++++++++ 3 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 .changeset/polyfill-replace-with.md create mode 100644 packages/polyfill/source/tests/ChildNode.test.ts diff --git a/.changeset/polyfill-replace-with.md b/.changeset/polyfill-replace-with.md new file mode 100644 index 00000000..30c7f6db --- /dev/null +++ b/.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. diff --git a/packages/polyfill/source/ChildNode.ts b/packages/polyfill/source/ChildNode.ts index 6d983969..4bf81ad5 100644 --- a/packages/polyfill/source/ChildNode.ts +++ b/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); } } diff --git a/packages/polyfill/source/tests/ChildNode.test.ts b/packages/polyfill/source/tests/ChildNode.test.ts new file mode 100644 index 00000000..7b5c0050 --- /dev/null +++ b/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(), + ), + ); + }); + }); +});