Consolidate commit phase hook functions (#19864)

There were a few pairs of commit phase functions that were almost identical except for one detail. I've refactored them a bit to consolidate their implementations: - Lifted error handling logic when mounting a fiber's passive hook effects to surround the entire list, instead of surrounding each effect. - Lifted profiler duration tracking to surround the entire list. In both cases, this matches the corresponding code for the layout phase. The naming is still a bit of a mess but I'm not too concerned because my next step is to refactor each commit sub-phase (layout, mutation) so that we can store values on the JS stack. So the existing function boundaries are about to change, anyway.

Author: acdlite

packages/react-reconciler/src/ReactFiberCommitWork.new.js

@@ -20,10 +20,7 @@ import type {FiberRoot} from './ReactInternalTypes';
 importtype{Lanes}from'./ReactFiberLane';
 importtype{SuspenseState}from'./ReactFiberSuspenseComponent.new';
 importtype{UpdateQueue}from'./ReactUpdateQueue.new';
-importtype{
-EffectasHookEffect,
-FunctionComponentUpdateQueue,
-}from'./ReactFiberHooks.new';
+importtype{FunctionComponentUpdateQueue}from'./ReactFiberHooks.new';
 importtype{Wakeable}from'shared/ReactTypes';
 importtype{ReactPriorityLevel}from'./ReactInternalTypes';
 importtype{OffscreenState}from'./ReactFiberOffscreenComponent';
@@ -343,42 +340,6 @@ function commitHookEffectListUnmount(
 }
 }
 
-// TODO: Remove this duplication.
-functioncommitHookEffectListUnmount2(
-// Tags to check for when deciding whether to unmount. e.g. to skip over layout effects
-hookFlags: HookFlags,
-fiber: Fiber,
-nearestMountedAncestor: Fiber|null,
-): void{
-constupdateQueue: FunctionComponentUpdateQueue|null=(fiber.updateQueue: any);
-constlastEffect=updateQueue!==null ? updateQueue.lastEffect : null;
-if(lastEffect!==null){
-const firstEffect =lastEffect.next;
-leteffect=firstEffect;
-do{
-const{next, tag}=effect;
-if((tag&hookFlags)===hookFlags){
-const destroy =effect.destroy;
-if(destroy!==undefined){
-effect.destroy=undefined;
-if(
-enableProfilerTimer&&
-enableProfilerCommitHooks&&
-fiber.mode&ProfileMode
-){
-startPassiveEffectTimer();
-safelyCallDestroy(fiber,nearestMountedAncestor,destroy);
-recordPassiveEffectDuration(fiber);
-}else{
-safelyCallDestroy(fiber,nearestMountedAncestor,destroy);
-}
-}
-}
-effect=next;
-}while(effect!==firstEffect);
-}
-}
-
 functioncommitHookEffectListMount(tag: HookFlags,finishedWork: Fiber){
 constupdateQueue: FunctionComponentUpdateQueue|null=(finishedWork.updateQueue: any);
 constlastEffect=updateQueue!==null ? updateQueue.lastEffect : null;
@@ -429,83 +390,6 @@ function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) {
 }
 }
 
-functioninvokePassiveEffectCreate(effect: HookEffect): void{
-const create =effect.create;
-effect.destroy=create();
-}
-
-// TODO: Remove this duplication.
-functioncommitHookEffectListMount2(fiber: Fiber): void{
-constupdateQueue: FunctionComponentUpdateQueue|null=(fiber.updateQueue: any);
-constlastEffect=updateQueue!==null ? updateQueue.lastEffect : null;
-if(lastEffect!==null){
-constfirstEffect=lastEffect.next;
-leteffect=firstEffect;
-do{
-const{next, tag}=effect;
-
-if(
-(tag&HookPassive)!==NoHookEffect&&
-(tag&HookHasEffect)!==NoHookEffect
-){
-if(__DEV__){
-if(
-enableProfilerTimer&&
-enableProfilerCommitHooks&&
-fiber.mode&ProfileMode
-){
-startPassiveEffectTimer();
-invokeGuardedCallback(
-null,
-invokePassiveEffectCreate,
-null,
-effect,
-);
-recordPassiveEffectDuration(fiber);
-}else{
-invokeGuardedCallback(
-null,
-invokePassiveEffectCreate,
-null,
-effect,
-);
-}
-if(hasCaughtError()){
-invariant(fiber!==null,'Should be working on an effect.');
-consterror=clearCaughtError();
-captureCommitPhaseError(fiber,fiber.return,error);
-}
-}else{
-try{
-constcreate=effect.create;
-if(
-enableProfilerTimer&&
-enableProfilerCommitHooks&&
-fiber.mode&ProfileMode
-){
-try{
-startPassiveEffectTimer();
-effect.destroy=create();
-}finally{
-recordPassiveEffectDuration(fiber);
-}
-}else{
-effect.destroy=create();
-}
-// TODO: This is missing the warning that exists in commitHookEffectListMount.
-// The warning refers to useEffect but only applies to useLayoutEffect.
-}catch(error){
-invariant(fiber!==null,'Should be working on an effect.');
-captureCommitPhaseError(fiber,fiber.return,error);
-}
-}
-}
-
-effect=next;
-}while(effect!==firstEffect);
-}
-}
-
 functioncommitProfilerPassiveEffect(
 finishedRoot: FiberRoot,
 finishedWork: Fiber,
@@ -1894,17 +1778,31 @@ function commitResetTextContent(current: Fiber): void {
 resetTextContent(current.stateNode);
 }
 
-functioncommitPassiveWork(finishedWork: Fiber): void{
+functioncommitPassiveUnmountInsideDeletedTree(finishedWork: Fiber): void{
 switch(finishedWork.tag){
 caseFunctionComponent:
 caseForwardRef:
 caseSimpleMemoComponent:
 caseBlock: {
-commitHookEffectListUnmount2(
-HookPassive|HookHasEffect,
-finishedWork,
-finishedWork.return,
-);
+if(
+enableProfilerTimer&&
+enableProfilerCommitHooks&&
+finishedWork.mode&ProfileMode
+){
+startPassiveEffectTimer();
+commitHookEffectListUnmount(
+HookPassive|HookHasEffect,
+finishedWork,
+finishedWork.return,
+);
+recordPassiveEffectDuration(finishedWork);
+}else{
+commitHookEffectListUnmount(
+HookPassive|HookHasEffect,
+finishedWork,
+finishedWork.return,
+);
+}
 break;
 }
 }
@@ -1918,16 +1816,32 @@ function commitPassiveUnmount(
 caseFunctionComponent:
 caseForwardRef:
 caseSimpleMemoComponent:
-caseBlock:
-commitHookEffectListUnmount2(
-HookPassive,
-current,
-nearestMountedAncestor,
-);
+caseBlock: {
+if(
+enableProfilerTimer&&
+enableProfilerCommitHooks&&
+current.mode&ProfileMode
+){
+startPassiveEffectTimer();
+commitHookEffectListUnmount(
+HookPassive,
+current,
+nearestMountedAncestor,
+);
+recordPassiveEffectDuration(current);
+}else{
+commitHookEffectListUnmount(
+HookPassive,
+current,
+nearestMountedAncestor,
+);
+}
+break;
+}
 }
 }
 
-functioncommitPassiveLifeCycles(
+functioncommitPassiveMount(
 finishedRoot: FiberRoot,
 finishedWork: Fiber,
 ): void{
@@ -1936,7 +1850,20 @@ function commitPassiveLifeCycles(
 caseForwardRef:
 caseSimpleMemoComponent:
 caseBlock: {
-commitHookEffectListMount2(finishedWork);
+if(
+enableProfilerTimer&&
+enableProfilerCommitHooks&&
+finishedWork.mode&ProfileMode
+){
+startPassiveEffectTimer();
+try{
+commitHookEffectListMount(HookPassive|HookHasEffect,finishedWork);
+}finally{
+recordPassiveEffectDuration(finishedWork);
+}
+}else{
+commitHookEffectListMount(HookPassive|HookHasEffect,finishedWork);
+}
 break;
 }
 caseProfiler: {
@@ -1956,6 +1883,6 @@ export {
 commitAttachRef,
 commitDetachRef,
 commitPassiveUnmount,
-commitPassiveWork,
-commitPassiveLifeCycles,
+commitPassiveUnmountInsideDeletedTree,
+commitPassiveMount,
 };

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

@@ -191,9 +191,9 @@ import {
 commitPlacement,
 commitWork,
 commitDeletion,
-commitPassiveUnmount,
-commitPassiveWork,
-commitPassiveLifeCyclesascommitPassiveEffectOnFiber,
+commitPassiveUnmountascommitPassiveUnmountOnFiber,
+commitPassiveUnmountInsideDeletedTreeascommitPassiveUnmountInsideDeletedTreeOnFiber,
+commitPassiveMountascommitPassiveMountOnFiber,
 commitDetachRef,
 commitAttachRef,
 commitResetTextContent,
@@ -2458,9 +2458,27 @@ function flushPassiveMountEffects(root, firstChild: Fiber): void {
 }
 
 if((fiber.flags&Passive)!==NoFlags){
-setCurrentDebugFiberInDEV(fiber);
-commitPassiveEffectOnFiber(root,fiber);
-resetCurrentDebugFiberInDEV();
+if(__DEV__){
+setCurrentDebugFiberInDEV(fiber);
+invokeGuardedCallback(
+null,
+commitPassiveMountOnFiber,
+null,
+root,
+fiber,
+);
+if(hasCaughtError()){
+consterror=clearCaughtError();
+captureCommitPhaseError(fiber,fiber.return,error);
+}
+resetCurrentDebugFiberInDEV();
+}else{
+try{
+commitPassiveMountOnFiber(root,fiber);
+}catch(error){
+captureCommitPhaseError(fiber,fiber.return,error);
+}
+}
 }
 
 fiber=fiber.sibling;
@@ -2496,7 +2514,7 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
 constprimaryFlags=fiber.flags&Passive;
 if(primaryFlags!==NoFlags){
 setCurrentDebugFiberInDEV(fiber);
-commitPassiveWork(fiber);
+commitPassiveUnmountInsideDeletedTreeOnFiber(fiber);
 resetCurrentDebugFiberInDEV();
 }
 
@@ -2525,7 +2543,7 @@ function flushPassiveUnmountEffectsInsideOfDeletedTree(
 
 if((fiberToDelete.flags&PassiveStatic)!==NoFlags){
 setCurrentDebugFiberInDEV(fiberToDelete);
-commitPassiveUnmount(fiberToDelete,nearestMountedAncestor);
+commitPassiveUnmountOnFiber(fiberToDelete,nearestMountedAncestor);
 resetCurrentDebugFiberInDEV();
 }
 }