Skip to content

Commit

Permalink
Wrap try-catch directly around each user function
Browse files Browse the repository at this point in the history
(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.
  • Loading branch information
acdlite committed Jul 12, 2022
1 parent de3c069 commit bb1357b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 26 deletions.
30 changes: 17 additions & 13 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
30 changes: 17 additions & 13 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit bb1357b

Please sign in to comment.