From f622ddd2cba79e0c63b2100ae46a9e75e7fdccd1 Mon Sep 17 00:00:00 2001 From: Ben Delaney Date: Thu, 4 Mar 2021 23:02:45 +1100 Subject: [PATCH 1/4] chore: remove unnecessary check from appendChild --- platform/nativescript/renderer/ViewNode.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/platform/nativescript/renderer/ViewNode.js b/platform/nativescript/renderer/ViewNode.js index c5eddb98..f76848bb 100644 --- a/platform/nativescript/renderer/ViewNode.js +++ b/platform/nativescript/renderer/ViewNode.js @@ -220,16 +220,7 @@ export default class ViewNode { ) } - if (childNode.parentNode === this) { - // we don't need to throw an error here, because it is a valid case - // for example when switching the order of elements in the tree - // fixes #127 - see for more details - // fixes #240 - // throw new Error(`Can't append child, because it is already a child.`) - } - childNode.parentNode = this - if (this.lastChild) { childNode.prevSibling = this.lastChild this.lastChild.nextSibling = childNode From b8d11410b6e3892224e803bcbe2ad6ce6d11cd4d Mon Sep 17 00:00:00 2001 From: Ben Delaney Date: Thu, 4 Mar 2021 23:04:10 +1100 Subject: [PATCH 2/4] fix: remove node before appending if already a child --- platform/nativescript/renderer/ViewNode.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/platform/nativescript/renderer/ViewNode.js b/platform/nativescript/renderer/ViewNode.js index f76848bb..57819994 100644 --- a/platform/nativescript/renderer/ViewNode.js +++ b/platform/nativescript/renderer/ViewNode.js @@ -165,13 +165,11 @@ export default class ViewNode { throw new Error(`Can't insert child.`) } - // in some rare cases insertBefore is called with a null referenceNode - // this makes sure that it get's appended as the last child - if (!referenceNode) { - return this.appendChild(childNode) - } - - if (referenceNode.parentNode && referenceNode.parentNode !== this) { + if ( + referenceNode && + referenceNode.parentNode && + referenceNode.parentNode !== this + ) { throw new Error( `Can't insert child, because the reference node has a different parent.` ) @@ -197,6 +195,12 @@ export default class ViewNode { // throw new Error(`Can't insert child, because it is already a child.`) } + // in some rare cases insertBefore is called with a null referenceNode + // this makes sure that it get's appended as the last child + if (!referenceNode) { + return this.appendChild(childNode) + } + let index = this.childNodes.indexOf(referenceNode) childNode.parentNode = this From b46498cf682a103a3f3051f53655ed89addb617f Mon Sep 17 00:00:00 2001 From: Ben Delaney Date: Fri, 5 Mar 2021 15:47:39 +1100 Subject: [PATCH 3/4] fix: set nextSibling of prevSibling upon insertion (fixed) --- platform/nativescript/renderer/ViewNode.js | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/nativescript/renderer/ViewNode.js b/platform/nativescript/renderer/ViewNode.js index 57819994..291ea3ea 100644 --- a/platform/nativescript/renderer/ViewNode.js +++ b/platform/nativescript/renderer/ViewNode.js @@ -206,6 +206,7 @@ export default class ViewNode { childNode.parentNode = this childNode.nextSibling = referenceNode childNode.prevSibling = this.childNodes[index - 1] + if (childNode.prevSibling) childNode.prevSibling.nextSibling = childNode referenceNode.prevSibling = childNode this.childNodes.splice(index, 0, childNode) From a2165a17f8753ec0cef26305cdfc98b40dbfa8ce Mon Sep 17 00:00:00 2001 From: Ben Delaney Date: Wed, 10 Mar 2021 19:50:16 +1100 Subject: [PATCH 4/4] test: add test case for #809 --- __tests__/renderer/ViewNode.test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/__tests__/renderer/ViewNode.test.js b/__tests__/renderer/ViewNode.test.js index 7456c10b..beedcdc0 100644 --- a/__tests__/renderer/ViewNode.test.js +++ b/__tests__/renderer/ViewNode.test.js @@ -120,6 +120,27 @@ describe('ViewNode', () => { expect(refNode.nextSibling).toBeFalsy() }) + test('insertBefore sets siblings of both siblings', () => { + let parentNode = new ViewNode() + let firstNode = new ViewNode() + let lastNode = new ViewNode() + let childNode = new ViewNode() + parentNode.childNodes = [firstNode, lastNode] + firstNode.parentNode = parentNode + lastNode.parentNode = parentNode + + parentNode.insertBefore(childNode, lastNode) + + expect(parentNode.childNodes.length).toBe(3) + expect(childNode.parentNode).toEqual(parentNode) + expect(firstNode.nextSibling).toEqual(childNode) + expect(lastNode.prevSibling).toEqual(childNode) + expect(childNode.prevSibling).toEqual(firstNode) + expect(childNode.nextSibling).toEqual(lastNode) + expect(firstNode.prevSibling).toBeFalsy() + expect(lastNode.nextSibling).toBeFalsy() + }) + test('appendChild throws if no childNode is given', () => { let node = new ViewNode()