From 123ae6e7280ef2394a9301f6608b712b368df72f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 17:49:46 -0400 Subject: [PATCH 1/5] Remove runWithFiberInDEV branches around recursive calls --- .../src/ReactFiberCommitWork.js | 207 ++++-------------- 1 file changed, 40 insertions(+), 167 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index c7dfb96617ed7..6d600971d4461 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -98,7 +98,6 @@ import { FormReset, Cloned, } from './ReactFiberFlags'; -import {runWithFiberInDEV} from './ReactCurrentFiber'; import { isCurrentUpdateNested, getCommitTime, @@ -289,11 +288,7 @@ function commitBeforeMutationEffects_begin() { function commitBeforeMutationEffects_complete() { while (nextEffect !== null) { const fiber = nextEffect; - if (__DEV__) { - runWithFiberInDEV(fiber, commitBeforeMutationEffectsOnFiber, fiber); - } else { - commitBeforeMutationEffectsOnFiber(fiber); - } + commitBeforeMutationEffectsOnFiber(fiber); const sibling = fiber.sibling; if (sibling !== null) { @@ -1645,17 +1640,7 @@ export function commitMutationEffects( inProgressLanes = committedLanes; inProgressRoot = root; - if (__DEV__) { - runWithFiberInDEV( - finishedWork, - commitMutationEffectsOnFiber, - finishedWork, - root, - committedLanes, - ); - } else { - commitMutationEffectsOnFiber(finishedWork, root, committedLanes); - } + commitMutationEffectsOnFiber(finishedWork, root, committedLanes); inProgressLanes = null; inProgressRoot = null; @@ -1682,17 +1667,7 @@ function recursivelyTraverseMutationEffects( ) { let child = parentFiber.child; while (child !== null) { - if (__DEV__) { - runWithFiberInDEV( - child, - commitMutationEffectsOnFiber, - child, - root, - lanes, - ); - } else { - commitMutationEffectsOnFiber(child, root, lanes); - } + commitMutationEffectsOnFiber(child, root, lanes); child = child.sibling; } } @@ -2235,18 +2210,7 @@ export function commitLayoutEffects( inProgressRoot = root; const current = finishedWork.alternate; - if (__DEV__) { - runWithFiberInDEV( - finishedWork, - commitLayoutEffectOnFiber, - root, - current, - finishedWork, - committedLanes, - ); - } else { - commitLayoutEffectOnFiber(root, current, finishedWork, committedLanes); - } + commitLayoutEffectOnFiber(root, current, finishedWork, committedLanes); inProgressLanes = null; inProgressRoot = null; @@ -2261,18 +2225,7 @@ function recursivelyTraverseLayoutEffects( let child = parentFiber.child; while (child !== null) { const current = child.alternate; - if (__DEV__) { - runWithFiberInDEV( - child, - commitLayoutEffectOnFiber, - root, - current, - child, - lanes, - ); - } else { - commitLayoutEffectOnFiber(root, current, child, lanes); - } + commitLayoutEffectOnFiber(root, current, child, lanes); child = child.sibling; } } @@ -2529,23 +2482,12 @@ function recursivelyTraverseReappearLayoutEffects( let child = parentFiber.child; while (child !== null) { const current = child.alternate; - if (__DEV__) { - runWithFiberInDEV( - child, - reappearLayoutEffects, - finishedRoot, - current, - child, - childShouldIncludeWorkInProgressEffects, - ); - } else { - reappearLayoutEffects( - finishedRoot, - current, - child, - childShouldIncludeWorkInProgressEffects, - ); - } + reappearLayoutEffects( + finishedRoot, + current, + child, + childShouldIncludeWorkInProgressEffects, + ); child = child.sibling; } } @@ -2696,23 +2638,12 @@ export function commitPassiveMountEffects( committedLanes: Lanes, committedTransitions: Array | null, ): void { - if (__DEV__) { - runWithFiberInDEV( - finishedWork, - commitPassiveMountOnFiber, - root, - finishedWork, - committedLanes, - committedTransitions, - ); - } else { - commitPassiveMountOnFiber( - root, - finishedWork, - committedLanes, - committedTransitions, - ); - } + commitPassiveMountOnFiber( + root, + finishedWork, + committedLanes, + committedTransitions, + ); } function recursivelyTraversePassiveMountEffects( @@ -2724,23 +2655,12 @@ function recursivelyTraversePassiveMountEffects( if (parentFiber.subtreeFlags & PassiveMask) { let child = parentFiber.child; while (child !== null) { - if (__DEV__) { - runWithFiberInDEV( - child, - commitPassiveMountOnFiber, - root, - child, - committedLanes, - committedTransitions, - ); - } else { - commitPassiveMountOnFiber( - root, - child, - committedLanes, - committedTransitions, - ); - } + commitPassiveMountOnFiber( + root, + child, + committedLanes, + committedTransitions, + ); child = child.sibling; } } @@ -2982,25 +2902,13 @@ function recursivelyTraverseReconnectPassiveEffects( // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic) let child = parentFiber.child; while (child !== null) { - if (__DEV__) { - runWithFiberInDEV( - child, - reconnectPassiveEffects, - finishedRoot, - child, - committedLanes, - committedTransitions, - childShouldIncludeWorkInProgressEffects, - ); - } else { - reconnectPassiveEffects( - finishedRoot, - child, - committedLanes, - committedTransitions, - childShouldIncludeWorkInProgressEffects, - ); - } + reconnectPassiveEffects( + finishedRoot, + child, + committedLanes, + committedTransitions, + childShouldIncludeWorkInProgressEffects, + ); child = child.sibling; } } @@ -3182,23 +3090,12 @@ function recursivelyTraverseAtomicPassiveEffects( if (parentFiber.subtreeFlags & PassiveMask) { let child = parentFiber.child; while (child !== null) { - if (__DEV__) { - runWithFiberInDEV( - child, - commitAtomicPassiveEffects, - finishedRoot, - child, - committedLanes, - committedTransitions, - ); - } else { - commitAtomicPassiveEffects( - finishedRoot, - child, - committedLanes, - committedTransitions, - ); - } + commitAtomicPassiveEffects( + finishedRoot, + child, + committedLanes, + committedTransitions, + ); child = child.sibling; } } @@ -3257,11 +3154,7 @@ function commitAtomicPassiveEffects( } export function commitPassiveUnmountEffects(finishedWork: Fiber): void { - if (__DEV__) { - runWithFiberInDEV(finishedWork, commitPassiveUnmountOnFiber, finishedWork); - } else { - commitPassiveUnmountOnFiber(finishedWork); - } + commitPassiveUnmountOnFiber(finishedWork); } // If we're inside a brand new tree, or a tree that was already visible, then we @@ -3412,11 +3305,7 @@ function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void { if (parentFiber.subtreeFlags & PassiveMask) { let child = parentFiber.child; while (child !== null) { - if (__DEV__) { - runWithFiberInDEV(child, commitPassiveUnmountOnFiber, child); - } else { - commitPassiveUnmountOnFiber(child); - } + commitPassiveUnmountOnFiber(child); child = child.sibling; } } @@ -3493,11 +3382,7 @@ function recursivelyTraverseDisconnectPassiveEffects(parentFiber: Fiber): void { // TODO: Check PassiveStatic flag let child = parentFiber.child; while (child !== null) { - if (__DEV__) { - runWithFiberInDEV(child, disconnectPassiveEffect, child); - } else { - disconnectPassiveEffect(child); - } + disconnectPassiveEffect(child); child = child.sibling; } } @@ -3544,19 +3429,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( // Deletion effects fire in parent -> child order // TODO: Check if fiber has a PassiveStatic flag - if (__DEV__) { - runWithFiberInDEV( - fiber, - commitPassiveUnmountInsideDeletedTreeOnFiber, - fiber, - nearestMountedAncestor, - ); - } else { - commitPassiveUnmountInsideDeletedTreeOnFiber( - fiber, - nearestMountedAncestor, - ); - } + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber, nearestMountedAncestor); const child = fiber.child; // TODO: Only traverse subtree if it has a PassiveStatic flag. From daec869aa5d80544747cb4e510e1c34a9d65a03f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 18:24:17 -0400 Subject: [PATCH 2/5] Wrap CommitHostEffects in runWithFiberInDEV at the same place as try/catch --- .../src/ReactFiberCommitHostEffects.js | 181 +++++++++++++++--- 1 file changed, 159 insertions(+), 22 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 99c2c1878db49..59e6503e0ce4c 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -52,12 +52,25 @@ import { } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; +import {runWithFiberInDEV} from './ReactCurrentFiber'; + export function commitHostMount(finishedWork: Fiber) { const type = finishedWork.type; const props = finishedWork.memoizedProps; const instance: Instance = finishedWork.stateNode; try { - commitMount(instance, type, props, finishedWork); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitMount, + instance, + type, + props, + finishedWork, + ); + } else { + commitMount(instance, type, props, finishedWork); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -69,13 +82,25 @@ export function commitHostUpdate( oldProps: any, ) { try { - commitUpdate( - finishedWork.stateNode, - finishedWork.type, - oldProps, - newProps, - finishedWork, - ); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitUpdate, + finishedWork.stateNode, + finishedWork.type, + oldProps, + newProps, + finishedWork, + ); + } else { + commitUpdate( + finishedWork.stateNode, + finishedWork.type, + oldProps, + newProps, + finishedWork, + ); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -88,7 +113,17 @@ export function commitHostTextUpdate( ) { const textInstance: TextInstance = finishedWork.stateNode; try { - commitTextUpdate(textInstance, oldText, newText); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitTextUpdate, + textInstance, + oldText, + newText, + ); + } else { + commitTextUpdate(textInstance, oldText, newText); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -97,7 +132,11 @@ export function commitHostTextUpdate( export function commitHostResetTextContent(finishedWork: Fiber) { const instance: Instance = finishedWork.stateNode; try { - resetTextContent(instance); + if (__DEV__) { + runWithFiberInDEV(finishedWork, resetTextContent, instance); + } else { + resetTextContent(instance); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -107,9 +146,22 @@ export function commitShowHideHostInstance(node: Fiber, isHidden: boolean) { try { const instance = node.stateNode; if (isHidden) { - hideInstance(instance); + if (__DEV__) { + runWithFiberInDEV(node, hideInstance, instance); + } else { + hideInstance(instance); + } } else { - unhideInstance(node.stateNode, node.memoizedProps); + if (__DEV__) { + runWithFiberInDEV( + node, + unhideInstance, + node.stateNode, + node.memoizedProps, + ); + } else { + unhideInstance(node.stateNode, node.memoizedProps); + } } } catch (error) { captureCommitPhaseError(node, node.return, error); @@ -120,9 +172,22 @@ export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) { try { const instance = node.stateNode; if (isHidden) { - hideTextInstance(instance); + if (__DEV__) { + runWithFiberInDEV(node, hideTextInstance, instance); + } else { + hideTextInstance(instance); + } } else { - unhideTextInstance(instance, node.memoizedProps); + if (__DEV__) { + runWithFiberInDEV( + node, + unhideTextInstance, + instance, + node.memoizedProps, + ); + } else { + unhideTextInstance(instance, node.memoizedProps); + } } } catch (error) { captureCommitPhaseError(node, node.return, error); @@ -332,7 +397,11 @@ function commitPlacement(finishedWork: Fiber): void { export function commitHostPlacement(finishedWork: Fiber) { try { - commitPlacement(finishedWork); + if (__DEV__) { + runWithFiberInDEV(finishedWork, commitPlacement, finishedWork); + } else { + commitPlacement(finishedWork); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -345,7 +414,16 @@ export function commitHostRemoveChildFromContainer( hostInstance: Instance | TextInstance, ) { try { - removeChildFromContainer(parentContainer, hostInstance); + if (__DEV__) { + runWithFiberInDEV( + deletedFiber, + removeChildFromContainer, + parentContainer, + hostInstance, + ); + } else { + removeChildFromContainer(parentContainer, hostInstance); + } } catch (error) { captureCommitPhaseError(deletedFiber, nearestMountedAncestor, error); } @@ -358,7 +436,16 @@ export function commitHostRemoveChild( hostInstance: Instance | TextInstance, ) { try { - removeChild(parentInstance, hostInstance); + if (__DEV__) { + runWithFiberInDEV( + deletedFiber, + removeChild, + parentInstance, + hostInstance, + ); + } else { + removeChild(parentInstance, hostInstance); + } } catch (error) { captureCommitPhaseError(deletedFiber, nearestMountedAncestor, error); } @@ -371,7 +458,16 @@ export function commitHostRootContainerChildren( const containerInfo = root.containerInfo; const pendingChildren = root.pendingChildren; try { - replaceContainerChildren(containerInfo, pendingChildren); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + replaceContainerChildren, + containerInfo, + pendingChildren, + ); + } else { + replaceContainerChildren(containerInfo, pendingChildren); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -388,7 +484,16 @@ export function commitHostPortalContainerChildren( ) { const containerInfo = portal.containerInfo; try { - replaceContainerChildren(containerInfo, pendingChildren); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + replaceContainerChildren, + containerInfo, + pendingChildren, + ); + } else { + replaceContainerChildren(containerInfo, pendingChildren); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -399,7 +504,15 @@ export function commitHostHydratedContainer( finishedWork: Fiber, ) { try { - commitHydratedContainer(root.containerInfo); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitHydratedContainer, + root.containerInfo, + ); + } else { + commitHydratedContainer(root.containerInfo); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -410,7 +523,15 @@ export function commitHostHydratedSuspense( finishedWork: Fiber, ) { try { - commitHydratedSuspenseInstance(suspenseInstance); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitHydratedSuspenseInstance, + suspenseInstance, + ); + } else { + commitHydratedSuspenseInstance(suspenseInstance); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -423,7 +544,23 @@ export function commitHostSingleton(finishedWork: Fiber) { try { // This was a new mount, we need to clear and set initial properties clearSingleton(singleton); - acquireSingletonInstance(finishedWork.type, props, singleton, finishedWork); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + acquireSingletonInstance, + finishedWork.type, + props, + singleton, + finishedWork, + ); + } else { + acquireSingletonInstance( + finishedWork.type, + props, + singleton, + finishedWork, + ); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } From 1de0845e15d3fbcc2ad0038cf38c758fda87d261 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 18:29:28 -0400 Subject: [PATCH 3/5] Wrap user space calls in runWithFiberInDEV --- .../src/ReactFiberCommitEffects.js | 57 ++++++++++++++++--- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 04b8120283a61..e26e31af91aa2 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -75,6 +75,8 @@ import { callDestroyInDEV, } from './ReactFiberCallUserSpace'; +import {runWithFiberInDEV} from './ReactCurrentFiber'; + function shouldProfile(current: Fiber): boolean { return ( enableProfilerTimer && @@ -128,7 +130,7 @@ export function commitHookEffectListMount( if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(true); } - destroy = callCreateInDEV(effect); + destroy = runWithFiberInDEV(finishedWork, callCreateInDEV, effect); if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(false); } @@ -330,7 +332,12 @@ export function commitClassLayoutLifecycles( if (shouldProfile(finishedWork)) { startLayoutEffectTimer(); if (__DEV__) { - callComponentDidMountInDEV(finishedWork, instance); + runWithFiberInDEV( + finishedWork, + callComponentDidMountInDEV, + finishedWork, + instance, + ); } else { try { instance.componentDidMount(); @@ -341,7 +348,12 @@ export function commitClassLayoutLifecycles( recordLayoutEffectDuration(finishedWork); } else { if (__DEV__) { - callComponentDidMountInDEV(finishedWork, instance); + runWithFiberInDEV( + finishedWork, + callComponentDidMountInDEV, + finishedWork, + instance, + ); } else { try { instance.componentDidMount(); @@ -391,7 +403,9 @@ export function commitClassLayoutLifecycles( if (shouldProfile(finishedWork)) { startLayoutEffectTimer(); if (__DEV__) { - callComponentDidUpdateInDEV( + runWithFiberInDEV( + finishedWork, + callComponentDidUpdateInDEV, finishedWork, instance, prevProps, @@ -412,7 +426,9 @@ export function commitClassLayoutLifecycles( recordLayoutEffectDuration(finishedWork); } else { if (__DEV__) { - callComponentDidUpdateInDEV( + runWithFiberInDEV( + finishedWork, + callComponentDidUpdateInDEV, finishedWork, instance, prevProps, @@ -439,7 +455,12 @@ export function commitClassDidMount(finishedWork: Fiber) { const instance = finishedWork.stateNode; if (typeof instance.componentDidMount === 'function') { if (__DEV__) { - callComponentDidMountInDEV(finishedWork, instance); + runWithFiberInDEV( + finishedWork, + callComponentDidMountInDEV, + finishedWork, + instance, + ); } else { try { instance.componentDidMount(); @@ -619,7 +640,13 @@ export function safelyCallComponentWillUnmount( if (shouldProfile(current)) { startLayoutEffectTimer(); if (__DEV__) { - callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); + runWithFiberInDEV( + current, + callComponentWillUnmountInDEV, + current, + nearestMountedAncestor, + instance, + ); } else { try { instance.componentWillUnmount(); @@ -630,7 +657,13 @@ export function safelyCallComponentWillUnmount( recordLayoutEffectDuration(current); } else { if (__DEV__) { - callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); + runWithFiberInDEV( + current, + callComponentWillUnmountInDEV, + current, + nearestMountedAncestor, + instance, + ); } else { try { instance.componentWillUnmount(); @@ -761,7 +794,13 @@ export function safelyCallDestroy( destroy: () => void, ) { if (__DEV__) { - callDestroyInDEV(current, nearestMountedAncestor, destroy); + runWithFiberInDEV( + current, + callDestroyInDEV, + current, + nearestMountedAncestor, + destroy, + ); } else { try { destroy(); From d6ea02805ff61c4462be9eb6838b7e3f41dbe575 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 18:52:26 -0400 Subject: [PATCH 4/5] Wrap remaining Commit Effects in runWithFiberInDEV --- .../src/ReactFiberCommitEffects.js | 160 +++++++++++++----- 1 file changed, 118 insertions(+), 42 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index e26e31af91aa2..85eb38ed30522 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -510,7 +510,11 @@ export function commitClassCallbacks(finishedWork: Fiber) { // but instead we rely on them being set during last render. // TODO: revisit this when we implement resuming. try { - commitCallbacks(updateQueue, instance); + if (__DEV__) { + runWithFiberInDEV(finishedWork, commitCallbacks, updateQueue, instance); + } else { + commitCallbacks(updateQueue, instance); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -525,7 +529,16 @@ export function commitClassHiddenCallbacks(finishedWork: Fiber) { if (updateQueue !== null) { const instance = finishedWork.stateNode; try { - commitHiddenCallbacks(updateQueue, instance); + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitHiddenCallbacks, + updateQueue, + instance, + ); + } else { + commitHiddenCallbacks(updateQueue, instance); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -551,7 +564,11 @@ export function commitRootCallbacks(finishedWork: Fiber) { } } try { - commitCallbacks(updateQueue, instance); + if (__DEV__) { + runWithFiberInDEV(finishedWork, commitCallbacks, updateQueue, instance); + } else { + commitCallbacks(updateQueue, instance); + } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); } @@ -563,6 +580,14 @@ if (__DEV__) { didWarnAboutUndefinedSnapshotBeforeUpdate = new Set(); } +function callGetSnapshotBeforeUpdates( + instance: any, + prevProps: any, + prevState: any, +) { + return instance.getSnapshotBeforeUpdate(prevProps, prevState); +} + export function commitClassSnapshot(finishedWork: Fiber, current: Fiber) { const prevProps = current.memoizedProps; const prevState = current.memoizedState; @@ -599,15 +624,20 @@ export function commitClassSnapshot(finishedWork: Fiber, current: Fiber) { } } try { - const snapshot = instance.getSnapshotBeforeUpdate( - resolveClassComponentProps( - finishedWork.type, - prevProps, - finishedWork.elementType === finishedWork.type, - ), - prevState, + const resolvedPrevProps = resolveClassComponentProps( + finishedWork.type, + prevProps, + finishedWork.elementType === finishedWork.type, ); + let snapshot; if (__DEV__) { + snapshot = runWithFiberInDEV( + finishedWork, + callGetSnapshotBeforeUpdates, + instance, + resolvedPrevProps, + prevState, + ); const didWarnSet = ((didWarnAboutUndefinedSnapshotBeforeUpdate: any): Set); if (snapshot === undefined && !didWarnSet.has(finishedWork.type)) { @@ -618,6 +648,12 @@ export function commitClassSnapshot(finishedWork: Fiber, current: Fiber) { getComponentNameFromFiber(finishedWork), ); } + } else { + snapshot = callGetSnapshotBeforeUpdates( + instance, + resolvedPrevProps, + prevState, + ); } instance.__reactInternalSnapshotBeforeUpdate = snapshot; } catch (error) { @@ -730,7 +766,11 @@ export function safelyAttachRef( nearestMountedAncestor: Fiber | null, ) { try { - commitAttachRef(current); + if (__DEV__) { + runWithFiberInDEV(current, commitAttachRef, current); + } else { + commitAttachRef(current); + } } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); } @@ -749,12 +789,20 @@ export function safelyDetachRef( if (shouldProfile(current)) { try { startLayoutEffectTimer(); - refCleanup(); + if (__DEV__) { + runWithFiberInDEV(current, refCleanup); + } else { + refCleanup(); + } } finally { recordLayoutEffectDuration(current); } } else { - refCleanup(); + if (__DEV__) { + runWithFiberInDEV(current, refCleanup); + } else { + refCleanup(); + } } } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); @@ -771,12 +819,20 @@ export function safelyDetachRef( if (shouldProfile(current)) { try { startLayoutEffectTimer(); - ref(null); + if (__DEV__) { + (runWithFiberInDEV(current, ref, null): void); + } else { + ref(null); + } } finally { recordLayoutEffectDuration(current); } } else { - ref(null); + if (__DEV__) { + (runWithFiberInDEV(current, ref, null): void); + } else { + ref(null); + } } } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); @@ -810,6 +866,44 @@ export function safelyCallDestroy( } } +function commitProfiler( + finishedWork: Fiber, + current: Fiber | null, + commitTime: number, + effectDuration: number, +) { + const {onCommit, onRender} = finishedWork.memoizedProps; + + let phase = current === null ? 'mount' : 'update'; + if (enableProfilerNestedUpdatePhase) { + if (isCurrentUpdateNested()) { + phase = 'nested-update'; + } + } + + if (typeof onRender === 'function') { + onRender( + finishedWork.memoizedProps.id, + phase, + finishedWork.actualDuration, + finishedWork.treeBaseDuration, + finishedWork.actualStartTime, + commitTime, + ); + } + + if (enableProfilerCommitHooks) { + if (typeof onCommit === 'function') { + onCommit( + finishedWork.memoizedProps.id, + phase, + effectDuration, + commitTime, + ); + } + } +} + export function commitProfilerUpdate( finishedWork: Fiber, current: Fiber | null, @@ -818,35 +912,17 @@ export function commitProfilerUpdate( ) { if (enableProfilerTimer && getExecutionContext() & CommitContext) { try { - const {onCommit, onRender} = finishedWork.memoizedProps; - - let phase = current === null ? 'mount' : 'update'; - if (enableProfilerNestedUpdatePhase) { - if (isCurrentUpdateNested()) { - phase = 'nested-update'; - } - } - - if (typeof onRender === 'function') { - onRender( - finishedWork.memoizedProps.id, - phase, - finishedWork.actualDuration, - finishedWork.treeBaseDuration, - finishedWork.actualStartTime, + if (__DEV__) { + runWithFiberInDEV( + finishedWork, + commitProfiler, + finishedWork, + current, commitTime, + effectDuration, ); - } - - if (enableProfilerCommitHooks) { - if (typeof onCommit === 'function') { - onCommit( - finishedWork.memoizedProps.id, - phase, - effectDuration, - commitTime, - ); - } + } else { + commitProfiler(finishedWork, current, commitTime, effectDuration); } } catch (error) { captureCommitPhaseError(finishedWork, finishedWork.return, error); From 731d483a56c905637bfc342fa9321e706505bbe4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 19:15:47 -0400 Subject: [PATCH 5/5] Wrap warnings to associate the cause --- .../src/ReactFiberCommitEffects.js | 25 +++++++++++++------ .../__tests__/ReactSuspenseCallback-test.js | 7 +++--- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 85eb38ed30522..1d9d5c65b8aa4 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -184,9 +184,16 @@ export function commitHookEffectListMount( } else { addendum = ' You returned: ' + destroy; } - console.error( - '%s must not return anything besides a function, ' + - 'which is used for clean-up.%s', + runWithFiberInDEV( + finishedWork, + (n, a) => { + console.error( + '%s must not return anything besides a function, ' + + 'which is used for clean-up.%s', + n, + a, + ); + }, hookName, addendum, ); @@ -642,11 +649,13 @@ export function commitClassSnapshot(finishedWork: Fiber, current: Fiber) { ((didWarnAboutUndefinedSnapshotBeforeUpdate: any): Set); if (snapshot === undefined && !didWarnSet.has(finishedWork.type)) { didWarnSet.add(finishedWork.type); - console.error( - '%s.getSnapshotBeforeUpdate(): A snapshot value (or null) ' + - 'must be returned. You have returned undefined.', - getComponentNameFromFiber(finishedWork), - ); + runWithFiberInDEV(finishedWork, () => { + console.error( + '%s.getSnapshotBeforeUpdate(): A snapshot value (or null) ' + + 'must be returned. You have returned undefined.', + getComponentNameFromFiber(finishedWork), + ); + }); } } else { snapshot = callGetSnapshotBeforeUpdates( diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js index 6ddc94905a52a..7f3724fa86490 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js @@ -57,9 +57,10 @@ describe('ReactSuspense', () => { ); ReactNoop.render(elementBadType); - await expect(async () => await waitForAll([])).toErrorDev([ - 'Unexpected type for suspenseCallback.', - ]); + await expect(async () => await waitForAll([])).toErrorDev( + ['Unexpected type for suspenseCallback.'], + {withoutStack: true}, + ); const elementMissingCallback = (