Skip to content

Commit 99e1024

Browse files
authored
Check if a child is a new child before calling moveBefore (facebook#32567)
This fixes a critical issue with moveBefore. I was told that the disconnected -> connected case was going to be relaxed and not be an error but apparently that is not the case. This means that we can't use this for initial insertions. Only moves. Unfortunately React's internals doesn't distinguish these cases. This adds a hack that checks each nodes but this is pretty bad for performance. We should only call this in one or the other case. Given that we still need feature detection. Both of which means that these calls are no longer inlined and this extra code. I wonder if it's even worth it given that you can't even rely on it working anyway since not all browsers have it. Kind of don't want to ship this until all browsers have it. Even then we'd ideally refactor React to use separate code paths for initial insertion vs moves. Which leads to some unfortunate code duplication.
1 parent 696950a commit 99e1024

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ export function appendChild(
808808
parentInstance: Instance,
809809
child: Instance | TextInstance,
810810
): void {
811-
if (supportsMoveBefore) {
811+
if (supportsMoveBefore && child.parentNode !== null) {
812812
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
813813
parentInstance.moveBefore(child, null);
814814
} else {
@@ -828,7 +828,7 @@ export function appendChildToContainer(
828828
container.nodeType === COMMENT_NODE
829829
) {
830830
parentNode = (container.parentNode: any);
831-
if (supportsMoveBefore) {
831+
if (supportsMoveBefore && child.parentNode !== null) {
832832
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
833833
parentNode.moveBefore(child, container);
834834
} else {
@@ -840,7 +840,7 @@ export function appendChildToContainer(
840840
} else {
841841
parentNode = (container: any);
842842
}
843-
if (supportsMoveBefore) {
843+
if (supportsMoveBefore && child.parentNode !== null) {
844844
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
845845
parentNode.moveBefore(child, null);
846846
} else {
@@ -870,7 +870,7 @@ export function insertBefore(
870870
child: Instance | TextInstance,
871871
beforeChild: Instance | TextInstance | SuspenseInstance,
872872
): void {
873-
if (supportsMoveBefore) {
873+
if (supportsMoveBefore && child.parentNode !== null) {
874874
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
875875
parentInstance.moveBefore(child, beforeChild);
876876
} else {
@@ -896,7 +896,7 @@ export function insertInContainerBefore(
896896
} else {
897897
parentNode = (container: any);
898898
}
899-
if (supportsMoveBefore) {
899+
if (supportsMoveBefore && child.parentNode !== null) {
900900
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
901901
parentNode.moveBefore(child, beforeChild);
902902
} else {

0 commit comments

Comments
 (0)