From 63b53c6fc1036c654b6f4dd5e2946beb00417cec Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Mon, 20 Feb 2023 19:03:09 +1100 Subject: [PATCH] fix rrdom bug If a rrnode appends the same child twice, the child nodes will become an infinite loop --- packages/rrdom/src/diff.ts | 2 +- packages/rrdom/src/document.ts | 5 ++++ packages/rrdom/test/document.test.ts | 35 ++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index e4e06e8db5..78628988e0 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -491,7 +491,7 @@ function diffChildren( } else if (newStartIndex > newEndIndex) { for (; oldStartIndex <= oldEndIndex; oldStartIndex++) { const node = oldChildren[oldStartIndex]; - if (!node || !parentNode.contains(node)) continue; + if (!node || node.parentNode !== parentNode) continue; try { parentNode.removeChild(node); replayer.mirror.removeNodeFromMap(node); diff --git a/packages/rrdom/src/document.ts b/packages/rrdom/src/document.ts index 98ee1696e4..a053009a76 100644 --- a/packages/rrdom/src/document.ts +++ b/packages/rrdom/src/document.ts @@ -713,6 +713,8 @@ export type CSSStyleDeclaration = Record & { }; function appendChild(parent: IRRNode, newChild: IRRNode) { + if (newChild.parentNode) newChild.parentNode.removeChild(newChild); + if (parent.lastChild) { parent.lastChild.nextSibling = newChild; newChild.previousSibling = parent.lastChild; @@ -740,6 +742,9 @@ function insertBefore( "Failed to execute 'insertBefore' on 'RRNode': The RRNode before which the new node is to be inserted is not a child of this RRNode.", ); + if (newChild === refChild) return newChild; + if (newChild.parentNode) newChild.parentNode.removeChild(newChild); + newChild.previousSibling = refChild.previousSibling; refChild.previousSibling = newChild; newChild.nextSibling = refChild; diff --git a/packages/rrdom/test/document.test.ts b/packages/rrdom/test/document.test.ts index 421dc036e2..38a35e505f 100644 --- a/packages/rrdom/test/document.test.ts +++ b/packages/rrdom/test/document.test.ts @@ -766,6 +766,22 @@ describe('Basic RRDocument implementation', () => { expect(node.contains(child2)).toBeTruthy(); }); + it('can append a child with parent node', () => { + const node = document.createElement('div'); + const child = document.createElement('span'); + expect(node.appendChild(child)).toBe(child); + expect(node.childNodes).toEqual([child]); + expect(node.appendChild(child)).toBe(child); + expect(node.childNodes).toEqual([child]); + expect(child.parentNode).toBe(node); + + const node1 = document.createElement('div'); + expect(node1.appendChild(child)).toBe(child); + expect(node1.childNodes).toEqual([child]); + expect(child.parentNode).toBe(node1); + expect(node.childNodes).toEqual([]); + }); + it('can insert new child before an existing child', () => { const node = document.createElement('div'); const child1 = document.createElement('h1'); @@ -820,6 +836,25 @@ describe('Basic RRDocument implementation', () => { expect(node.contains(child1)).toBeTruthy(); }); + it('can insert a child with parent node', () => { + const node = document.createElement('div'); + const child1 = document.createElement('h1'); + expect(node.insertBefore(child1, null)).toBe(child1); + expect(node.childNodes).toEqual([child1]); + expect(node.insertBefore(child1, child1)).toBe(child1); + expect(node.childNodes).toEqual([child1]); + expect(child1.parentNode).toEqual(node); + + const node2 = document.createElement('div'); + const child2 = document.createElement('h2'); + expect(node2.insertBefore(child2, null)).toBe(child2); + expect(node2.childNodes).toEqual([child2]); + expect(node2.insertBefore(child1, child2)).toBe(child1); + expect(node2.childNodes).toEqual([child1, child2]); + expect(child1.parentNode).toEqual(node2); + expect(node.childNodes).toEqual([]); + }); + it('can remove an existing child', () => { const node = document.createElement('div'); const child1 = document.createElement('h1');