[PR Stack] Refactor mutation phase to use recursion #24308

I'm about to refactor part of the commit phase to use recursion instead of iteration. As part of that change, we will no longer assign the `return` pointer when traversing into a subtree. So I'm disabling the internal warning that fires if the return pointer is not consistent with the parent during the commit phase. I had originally added this warning to help prevent mistakes when traversing the tree iteratively, but since we're intentionally switching to recursion instead, we don't need it.

commitWork is forked into a separate implementation for mutation mode (DOM) and persistent mode (React Native). But unlike when it was first introduced, there's more overlap than differences between the forks, mainly because we've added new types of fibers. So this joins the two forks and adds more local branches where the behavior actually diverges: host nodes, host containers, and portals.

There's not really any reason these should be separate functions. The factoring has gotten sloppy and redundant because there's similar logic in both places, which is more obvious now that they're combined. Next I'll start combining the redundant branches.

The fiber tag is more specific than the effect flag, so we should always refine the type of work first, to minimize redundant checks. In the next step I'll move all other other flag checks in this function into the same switch statement.

We should always refine the type of fiber before checking the effect flag, because the fiber tag is more specific. Now we have a single switch statement for all mutation effects.

reportUncaughtErrorInDev is always followed by captureCommitPhaseError, so we can move it into that function.

This moves the try-catch from around each fiber's mutation 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.

@trueadm

Most of the commit phase uses iterative loops to traverse the tree. Originally we thought this would be faster than using recursion, but a while back @trueadm did some performance testing and found that the loop was slower because we assign to the `return` pointer before entering a subtree (which we have to do because the `return` pointer is not always consistent; it could point to one of two fibers). The other motivation is so we can take advantage of the JS stack to track contextual information, like the nearest host parent. We already use recursion in a few places; this changes the mutation phase to use it, too.

Similar to the previous step, this converts the deletion phase into a single recursive function. Although there's less code, this one is a bit trickier because it's already contains some stack-like logic for tracking the nearest host parent. But instead of using the actual stack, it repeatedly searches up the fiber return path to find the nearest host parent. Instead, I've changed it to track the nearest host parent on the JS stack. (We still search up the return path once, to set the initial host parent right before entering a deleted tree. As a follow up, we can instead push this to the stack as we traverse during the main mutation phase.)

When a tree goes offscreen, we unmount all the effects just like we would in a normal deletion. (Conceptually it _is_ a deletion; we keep the fiber around so we can reuse its state if the tree mounts again.) If an offscreen component gets deleted "for real", we shouldn't unmount it again. The fix is to track on the stack whether we're inside a hidden tree. We already had a stack variable for this purpose, called `offscreenSubtreeWasHidden`, in another part of the commit phase, so I reused that variable instead of creating a new one. (The name is a bit confusing: "was" refers to the current tree before this commit. So, the "previous current".) Co-authored-by: dan <dan.abramov@me.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PR Stack] Refactor mutation phase to use recursion #24308

[PR Stack] Refactor mutation phase to use recursion #24308

Commits on Apr 8, 2022