From bb1357b38170e81cfbf2398cc6fefcf7a1ef2ccb Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Tue, 12 Jul 2022 14:45:21 -0400
Subject: [PATCH] Wrap try-catch directly around each user function

(This is the same as f9e6aef, but for the passive mount phase rather than the
mutation phase.)

This moves the try-catch from around each fiber's passive mount phase to
direclty around each user function (effect function, callback, etc).

We already do this when unmounting because if one unmount function
errors, we still need to call all the others so they can clean up
their resources.

Previously we didn't bother to do this for anything but unmount,
because if a mount effect throws, we're going to delete that whole
tree anyway.

But now that we're switching from an iterative loop to a recursive one,
we don't want every call frame on the stack to have a try-catch, since
the error handling requires additional memory.

Wrapping every user function is a bit tedious, but it's better
for performance. Many of them already had try blocks around
them already.
---
 .../src/ReactFiberCommitWork.new.js           | 30 +++++++++++--------
 .../src/ReactFiberCommitWork.old.js           | 30 +++++++++++--------
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 949f4f29c895c..c009612e811aa 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -2885,16 +2885,12 @@ function commitPassiveMountEffects_complete(
     const fiber = nextEffect;
 
     setCurrentDebugFiberInDEV(fiber);
-    try {
-      commitPassiveMountOnFiber(
-        root,
-        fiber,
-        committedLanes,
-        committedTransitions,
-      );
-    } catch (error) {
-      captureCommitPhaseError(fiber, fiber.return, error);
-    }
+    commitPassiveMountOnFiber(
+      root,
+      fiber,
+      committedLanes,
+      committedTransitions,
+    );
     resetCurrentDebugFiberInDEV();
 
     if (fiber === subtreeRoot) {
@@ -2936,11 +2932,19 @@ function commitPassiveMountOnFiber(
               HookPassive | HookHasEffect,
               finishedWork,
             );
-          } finally {
-            recordPassiveEffectDuration(finishedWork);
+          } catch (error) {
+            captureCommitPhaseError(finishedWork, finishedWork.return, error);
           }
+          recordPassiveEffectDuration(finishedWork);
         } else {
-          commitHookEffectListMount(HookPassive | HookHasEffect, finishedWork);
+          try {
+            commitHookEffectListMount(
+              HookPassive | HookHasEffect,
+              finishedWork,
+            );
+          } catch (error) {
+            captureCommitPhaseError(finishedWork, finishedWork.return, error);
+          }
         }
       }
       break;
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index f5e60b1d45ccd..9eed4a232328f 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -2885,16 +2885,12 @@ function commitPassiveMountEffects_complete(
     const fiber = nextEffect;
 
     setCurrentDebugFiberInDEV(fiber);
-    try {
-      commitPassiveMountOnFiber(
-        root,
-        fiber,
-        committedLanes,
-        committedTransitions,
-      );
-    } catch (error) {
-      captureCommitPhaseError(fiber, fiber.return, error);
-    }
+    commitPassiveMountOnFiber(
+      root,
+      fiber,
+      committedLanes,
+      committedTransitions,
+    );
     resetCurrentDebugFiberInDEV();
 
     if (fiber === subtreeRoot) {
@@ -2936,11 +2932,19 @@ function commitPassiveMountOnFiber(
               HookPassive | HookHasEffect,
               finishedWork,
             );
-          } finally {
-            recordPassiveEffectDuration(finishedWork);
+          } catch (error) {
+            captureCommitPhaseError(finishedWork, finishedWork.return, error);
           }
+          recordPassiveEffectDuration(finishedWork);
         } else {
-          commitHookEffectListMount(HookPassive | HookHasEffect, finishedWork);
+          try {
+            commitHookEffectListMount(
+              HookPassive | HookHasEffect,
+              finishedWork,
+            );
+          } catch (error) {
+            captureCommitPhaseError(finishedWork, finishedWork.return, error);
+          }
         }
       }
       break;