Add try/catch to places used to be covered by commitDeletionEffects

facebook · Sep 4, 2024 · 7275edb · 7275edb
1 parent 6201d60
commit 7275edb
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 26 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js
@@ -45,6 +45,8 @@ import {
   unhideTextInstance,
   commitHydratedContainer,
   commitHydratedSuspenseInstance,
+  removeChildFromContainer,
+  removeChild,
 } from './ReactFiberConfig';
 import {captureCommitPhaseError} from './ReactFiberWorkLoop';
 
@@ -334,6 +336,32 @@ export function commitHostPlacement(finishedWork: Fiber) {
   }
 }
 
+export function commitHostRemoveChildFromContainer(
+  deletedFiber: Fiber,
+  nearestMountedAncestor: Fiber,
+  parentContainer: Container,
+  hostInstance: Instance | TextInstance,
+) {
+  try {
+    removeChildFromContainer(parentContainer, hostInstance);
+  } catch (error) {
+    captureCommitPhaseError(deletedFiber, nearestMountedAncestor, error);
+  }
+}
+
+export function commitHostRemoveChild(
+  deletedFiber: Fiber,
+  nearestMountedAncestor: Fiber,
+  parentInstance: Instance,
+  hostInstance: Instance | TextInstance,
+) {
+  try {
+    removeChild(parentInstance, hostInstance);
+  } catch (error) {
+    captureCommitPhaseError(deletedFiber, nearestMountedAncestor, error);
+  }
+}
+
 export function commitHostRootContainerChildren(
   root: FiberRoot,
   finishedWork: Fiber,
@@ -354,9 +382,9 @@ export function commitHostPortalContainerChildren(
     ...
   },
   finishedWork: Fiber,
+  pendingChildren: ChildSet,
 ) {
   const containerInfo = portal.containerInfo;
-  const pendingChildren = portal.pendingChildren;
   try {
     replaceContainerChildren(containerInfo, pendingChildren);
   } catch (error) {

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -12,7 +12,6 @@ import type {
   TextInstance,
   SuspenseInstance,
   Container,
-  ChildSet,
   HoistableRoot,
   FormInstance,
 } from './ReactFiberConfig';
@@ -114,11 +113,8 @@ import {
   supportsHydration,
   supportsResources,
   supportsSingletons,
-  removeChild,
-  removeChildFromContainer,
   clearSuspenseBoundary,
   clearSuspenseBoundaryFromContainer,
-  replaceContainerChildren,
   createContainerChildSet,
   clearContainer,
   prepareScopeUpdate,
@@ -212,6 +208,8 @@ import {
   commitHostPortalContainerChildren,
   commitHostHydratedContainer,
   commitHostHydratedSuspense,
+  commitHostRemoveChildFromContainer,
+  commitHostRemoveChild,
 } from './ReactFiberCommitHostEffects';
 
 // Used during the commit phase to track the state of the Offscreen component stack.
@@ -1064,21 +1062,6 @@ function detachFiberAfterEffects(fiber: Fiber) {
   fiber.updateQueue = null;
 }
 
-function emptyPortalContainer(current: Fiber) {
-  if (!supportsPersistence) {
-    return;
-  }
-
-  const portal: {
-    containerInfo: Container,
-    pendingChildren: ChildSet,
-    ...
-  } = current.stateNode;
-  const {containerInfo} = portal;
-  const emptyChildSet = createContainerChildSet();
-  replaceContainerChildren(containerInfo, emptyChildSet);
-}
-
 // These are tracked on the stack as we recursively traverse a
 // deleted subtree.
 // TODO: Update these during the whole mutation phase, not just during
@@ -1249,12 +1232,16 @@ function commitDeletionEffectsOnFiber(
           // Now that all the child effects have unmounted, we can remove the
           // node from the tree.
           if (hostParentIsContainer) {
-            removeChildFromContainer(
+            commitHostRemoveChildFromContainer(
+              deletedFiber,
+              nearestMountedAncestor,
               ((hostParent: any): Container),
               (deletedFiber.stateNode: Instance | TextInstance),
             );
           } else {
-            removeChild(
+            commitHostRemoveChild(
+              deletedFiber,
+              nearestMountedAncestor,
               ((hostParent: any): Instance),
               (deletedFiber.stateNode: Instance | TextInstance),
             );
@@ -1273,9 +1260,17 @@ function commitDeletionEffectsOnFiber(
       if (enableSuspenseCallback) {
         const hydrationCallbacks = finishedRoot.hydrationCallbacks;
         if (hydrationCallbacks !== null) {
-          const onDeleted = hydrationCallbacks.onDeleted;
-          if (onDeleted) {
-            onDeleted((deletedFiber.stateNode: SuspenseInstance));
+          try {
+            const onDeleted = hydrationCallbacks.onDeleted;
+            if (onDeleted) {
+              onDeleted((deletedFiber.stateNode: SuspenseInstance));
+            }
+          } catch (error) {
+            captureCommitPhaseError(
+              deletedFiber,
+              nearestMountedAncestor,
+              error,
+            );
           }
         }
       }
@@ -1315,7 +1310,13 @@ function commitDeletionEffectsOnFiber(
         hostParent = prevHostParent;
         hostParentIsContainer = prevHostParentIsContainer;
       } else {
-        emptyPortalContainer(deletedFiber);
+        if (supportsPersistence) {
+          commitHostPortalContainerChildren(
+            deletedFiber.stateNode,
+            deletedFiber,
+            createContainerChildSet(),
+          );
+        }
 
         recursivelyTraverseDeletionEffects(
           finishedRoot,
@@ -2006,6 +2007,7 @@ function commitMutationEffectsOnFiber(
           commitHostPortalContainerChildren(
             finishedWork.stateNode,
             finishedWork,
+            finishedWork.stateNode.pendingChildren,
           );
         }
       }