From 4dd16fb439d97df81c7b0dfff3ecb16d59af9de0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 13:39:58 -0400 Subject: [PATCH 1/7] Extract functions that call into user space into CommitEffects --- .../src/ReactFiberCommitEffects.js | 763 ++++++++++++++++ .../src/ReactFiberCommitWork.js | 840 ++---------------- 2 files changed, 860 insertions(+), 743 deletions(-) create mode 100644 packages/react-reconciler/src/ReactFiberCommitEffects.js diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js new file mode 100644 index 0000000000000..d981f8279f31e --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -0,0 +1,763 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {Fiber} from './ReactInternalTypes'; +import type {UpdateQueue} from './ReactFiberClassUpdateQueue'; +import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; +import type {HookFlags} from './ReactHookEffectTags'; + +import { + enableProfilerTimer, + enableProfilerCommitHooks, + enableProfilerNestedUpdatePhase, + enableSchedulingProfiler, + enableScopeAPI, + disableStringRefs, +} from 'shared/ReactFeatureFlags'; +import { + HostComponent, + HostHoistable, + HostSingleton, + ScopeComponent, +} from './ReactWorkTags'; +import {NoFlags} from './ReactFiberFlags'; +import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; +import {resolveClassComponentProps} from './ReactFiberClassComponent'; +import { + recordLayoutEffectDuration, + startLayoutEffectTimer, + recordPassiveEffectDuration, + startPassiveEffectTimer, + isCurrentUpdateNested, +} from './ReactProfilerTimer'; +import {NoMode, ProfileMode} from './ReactTypeOfMode'; +import {commitCallbacks} from './ReactFiberClassUpdateQueue'; +import {getPublicInstance} from './ReactFiberConfig'; +import { + captureCommitPhaseError, + setIsRunningInsertionEffect, + getExecutionContext, + CommitContext, + NoContext, +} from './ReactFiberWorkLoop'; +import { + NoFlags as NoHookEffect, + Layout as HookLayout, + Insertion as HookInsertion, + Passive as HookPassive, +} from './ReactHookEffectTags'; +import {didWarnAboutReassigningProps} from './ReactFiberBeginWork'; +import { + markComponentPassiveEffectMountStarted, + markComponentPassiveEffectMountStopped, + markComponentPassiveEffectUnmountStarted, + markComponentPassiveEffectUnmountStopped, + markComponentLayoutEffectMountStarted, + markComponentLayoutEffectMountStopped, + markComponentLayoutEffectUnmountStarted, + markComponentLayoutEffectUnmountStopped, +} from './ReactFiberDevToolsHook'; +import { + callComponentDidMountInDEV, + callComponentDidUpdateInDEV, + callComponentWillUnmountInDEV, + callCreateInDEV, + callDestroyInDEV, +} from './ReactFiberCallUserSpace'; + +function shouldProfile(current: Fiber): boolean { + return ( + enableProfilerTimer && + enableProfilerCommitHooks && + (current.mode & ProfileMode) !== NoMode && + (getExecutionContext() & CommitContext) !== NoContext + ); +} + +export function commitHookLayoutEffects( + finishedWork: Fiber, + hookFlags: HookFlags, +) { + // At this point layout effects have already been destroyed (during mutation phase). + // This is done to prevent sibling component effects from interfering with each other, + // e.g. a destroy function in one component should never override a ref set + // by a create function in another component during the same commit. + if (shouldProfile(finishedWork)) { + try { + startLayoutEffectTimer(); + commitHookEffectListMount(hookFlags, finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + recordLayoutEffectDuration(finishedWork); + } else { + try { + commitHookEffectListMount(hookFlags, finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } +} + +export function commitHookEffectListMount( + flags: HookFlags, + finishedWork: Fiber, +) { + const updateQueue: FunctionComponentUpdateQueue | null = + (finishedWork.updateQueue: any); + const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; + if (lastEffect !== null) { + const firstEffect = lastEffect.next; + let effect = firstEffect; + do { + if ((effect.tag & flags) === flags) { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStarted(finishedWork); + } + } + + // Mount + let destroy; + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + destroy = callCreateInDEV(effect); + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } else { + const create = effect.create; + const inst = effect.inst; + destroy = create(); + inst.destroy = destroy; + } + + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStopped(); + } + } + + if (__DEV__) { + if (destroy !== undefined && typeof destroy !== 'function') { + let hookName; + if ((effect.tag & HookLayout) !== NoFlags) { + hookName = 'useLayoutEffect'; + } else if ((effect.tag & HookInsertion) !== NoFlags) { + hookName = 'useInsertionEffect'; + } else { + hookName = 'useEffect'; + } + let addendum; + if (destroy === null) { + addendum = + ' You returned null. If your effect does not require clean ' + + 'up, return undefined (or nothing).'; + } else if (typeof destroy.then === 'function') { + addendum = + '\n\nIt looks like you wrote ' + + hookName + + '(async () => ...) or returned a Promise. ' + + 'Instead, write the async function inside your effect ' + + 'and call it immediately:\n\n' + + hookName + + '(() => {\n' + + ' async function fetchData() {\n' + + ' // You can await here\n' + + ' const response = await MyAPI.getData(someId);\n' + + ' // ...\n' + + ' }\n' + + ' fetchData();\n' + + `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + + 'Learn more about data fetching with Hooks: https://react.dev/link/hooks-data-fetching'; + } else { + addendum = ' You returned: ' + destroy; + } + console.error( + '%s must not return anything besides a function, ' + + 'which is used for clean-up.%s', + hookName, + addendum, + ); + } + } + } + effect = effect.next; + } while (effect !== firstEffect); + } +} + +export function commitHookEffectListUnmount( + flags: HookFlags, + finishedWork: Fiber, + nearestMountedAncestor: Fiber | null, +) { + const updateQueue: FunctionComponentUpdateQueue | null = + (finishedWork.updateQueue: any); + const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; + if (lastEffect !== null) { + const firstEffect = lastEffect.next; + let effect = firstEffect; + do { + if ((effect.tag & flags) === flags) { + // Unmount + const inst = effect.inst; + const destroy = inst.destroy; + if (destroy !== undefined) { + inst.destroy = undefined; + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStarted(finishedWork); + } + } + + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + } + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } + + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStopped(); + } + } + } + } + effect = effect.next; + } while (effect !== firstEffect); + } +} + +export function commitHookPassiveMountEffects( + finishedWork: Fiber, + hookFlags: HookFlags, +) { + if (shouldProfile(finishedWork)) { + startPassiveEffectTimer(); + try { + commitHookEffectListMount(hookFlags, finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + recordPassiveEffectDuration(finishedWork); + } else { + try { + commitHookEffectListMount(hookFlags, finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } +} + +export function commitHookPassiveUnmountEffects( + finishedWork: Fiber, + nearestMountedAncestor: null | Fiber, + hookFlags: HookFlags, +) { + if (shouldProfile(finishedWork)) { + startPassiveEffectTimer(); + commitHookEffectListUnmount( + hookFlags, + finishedWork, + nearestMountedAncestor, + ); + recordPassiveEffectDuration(finishedWork); + } else { + commitHookEffectListUnmount( + hookFlags, + finishedWork, + nearestMountedAncestor, + ); + } +} + +export function commitClassLayoutLifecycles( + finishedWork: Fiber, + current: Fiber | null, +) { + const instance = finishedWork.stateNode; + if (current === null) { + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + if (__DEV__) { + if ( + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && + !didWarnAboutReassigningProps + ) { + if (instance.props !== finishedWork.memoizedProps) { + console.error( + 'Expected %s props to match memoized props before ' + + 'componentDidMount. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.props`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + if (instance.state !== finishedWork.memoizedState) { + console.error( + 'Expected %s state to match memoized state before ' + + 'componentDidMount. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.state`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + } + } + if (shouldProfile(finishedWork)) { + startLayoutEffectTimer(); + if (__DEV__) { + callComponentDidMountInDEV(finishedWork, instance); + } else { + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } + recordLayoutEffectDuration(finishedWork); + } else { + if (__DEV__) { + callComponentDidMountInDEV(finishedWork, instance); + } else { + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } + } + } else { + const prevProps = resolveClassComponentProps( + finishedWork.type, + current.memoizedProps, + finishedWork.elementType === finishedWork.type, + ); + const prevState = current.memoizedState; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + if (__DEV__) { + if ( + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && + !didWarnAboutReassigningProps + ) { + if (instance.props !== finishedWork.memoizedProps) { + console.error( + 'Expected %s props to match memoized props before ' + + 'componentDidUpdate. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.props`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + if (instance.state !== finishedWork.memoizedState) { + console.error( + 'Expected %s state to match memoized state before ' + + 'componentDidUpdate. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.state`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + } + } + if (shouldProfile(finishedWork)) { + startLayoutEffectTimer(); + if (__DEV__) { + callComponentDidUpdateInDEV( + finishedWork, + instance, + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } else { + try { + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } + recordLayoutEffectDuration(finishedWork); + } else { + if (__DEV__) { + callComponentDidUpdateInDEV( + finishedWork, + instance, + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } else { + try { + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } + } + } +} + +export function commitClassCallbacks(finishedWork: Fiber) { + // TODO: I think this is now always non-null by the time it reaches the + // commit phase. Consider removing the type check. + const updateQueue: UpdateQueue | null = + (finishedWork.updateQueue: any); + if (updateQueue !== null) { + const instance = finishedWork.stateNode; + if (__DEV__) { + if ( + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && + !didWarnAboutReassigningProps + ) { + if (instance.props !== finishedWork.memoizedProps) { + console.error( + 'Expected %s props to match memoized props before ' + + 'processing the update queue. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.props`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + if (instance.state !== finishedWork.memoizedState) { + console.error( + 'Expected %s state to match memoized state before ' + + 'processing the update queue. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.state`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + } + } + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + try { + commitCallbacks(updateQueue, instance); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } +} + +let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; +if (__DEV__) { + didWarnAboutUndefinedSnapshotBeforeUpdate = new Set(); +} + +export function commitClassSnapshot(finishedWork: Fiber, current: Fiber) { + const prevProps = current.memoizedProps; + const prevState = current.memoizedState; + const instance = finishedWork.stateNode; + // We could update instance props and state here, + // but instead we rely on them being set during last render. + // TODO: revisit this when we implement resuming. + if (__DEV__) { + if ( + !finishedWork.type.defaultProps && + !('ref' in finishedWork.memoizedProps) && + !didWarnAboutReassigningProps + ) { + if (instance.props !== finishedWork.memoizedProps) { + console.error( + 'Expected %s props to match memoized props before ' + + 'getSnapshotBeforeUpdate. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.props`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + if (instance.state !== finishedWork.memoizedState) { + console.error( + 'Expected %s state to match memoized state before ' + + 'getSnapshotBeforeUpdate. ' + + 'This might either be because of a bug in React, or because ' + + 'a component reassigns its own `this.state`. ' + + 'Please file an issue.', + getComponentNameFromFiber(finishedWork) || 'instance', + ); + } + } + } + try { + const snapshot = instance.getSnapshotBeforeUpdate( + resolveClassComponentProps( + finishedWork.type, + prevProps, + finishedWork.elementType === finishedWork.type, + ), + prevState, + ); + if (__DEV__) { + const didWarnSet = + ((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), + ); + } + } + instance.__reactInternalSnapshotBeforeUpdate = snapshot; + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +// Capture errors so they don't interrupt unmounting. +export function safelyCallComponentWillUnmount( + current: Fiber, + nearestMountedAncestor: Fiber | null, + instance: any, +) { + instance.props = resolveClassComponentProps( + current.type, + current.memoizedProps, + current.elementType === current.type, + ); + instance.state = current.memoizedState; + if (shouldProfile(current)) { + startLayoutEffectTimer(); + if (__DEV__) { + callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); + } else { + try { + instance.componentWillUnmount(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } + recordLayoutEffectDuration(current); + } else { + if (__DEV__) { + callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); + } else { + try { + instance.componentWillUnmount(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } + } +} + +function commitAttachRef(finishedWork: Fiber) { + const ref = finishedWork.ref; + if (ref !== null) { + const instance = finishedWork.stateNode; + let instanceToUse; + switch (finishedWork.tag) { + case HostHoistable: + case HostSingleton: + case HostComponent: + instanceToUse = getPublicInstance(instance); + break; + default: + instanceToUse = instance; + } + // Moved outside to ensure DCE works with this flag + if (enableScopeAPI && finishedWork.tag === ScopeComponent) { + instanceToUse = instance; + } + if (typeof ref === 'function') { + if (shouldProfile(finishedWork)) { + try { + startLayoutEffectTimer(); + finishedWork.refCleanup = ref(instanceToUse); + } finally { + recordLayoutEffectDuration(finishedWork); + } + } else { + finishedWork.refCleanup = ref(instanceToUse); + } + } else { + if (__DEV__) { + // TODO: We should move these warnings to happen during the render + // phase (markRef). + if (disableStringRefs && typeof ref === 'string') { + console.error('String refs are no longer supported.'); + } else if (!ref.hasOwnProperty('current')) { + console.error( + 'Unexpected ref object provided for %s. ' + + 'Use either a ref-setter function or React.createRef().', + getComponentNameFromFiber(finishedWork), + ); + } + } + + // $FlowFixMe[incompatible-use] unable to narrow type to the non-function case + ref.current = instanceToUse; + } + } +} + +// Capture errors so they don't interrupt mounting. +export function safelyAttachRef( + current: Fiber, + nearestMountedAncestor: Fiber | null, +) { + try { + commitAttachRef(current); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } +} + +export function safelyDetachRef( + current: Fiber, + nearestMountedAncestor: Fiber | null, +) { + const ref = current.ref; + const refCleanup = current.refCleanup; + + if (ref !== null) { + if (typeof refCleanup === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); + refCleanup(); + } finally { + recordLayoutEffectDuration(current); + } + } else { + refCleanup(); + } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } + } + } else if (typeof ref === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); + ref(null); + } finally { + recordLayoutEffectDuration(current); + } + } else { + ref(null); + } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } else { + // $FlowFixMe[incompatible-use] unable to narrow type to RefObject + ref.current = null; + } + } +} + +export function safelyCallDestroy( + current: Fiber, + nearestMountedAncestor: Fiber | null, + destroy: () => void, +) { + if (__DEV__) { + callDestroyInDEV(current, nearestMountedAncestor, destroy); + } else { + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } +} + +export function commitProfilerUpdate( + finishedWork: Fiber, + current: Fiber | null, + commitTime: number, + effectDuration: number, +) { + 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, + commitTime, + ); + } + + if (enableProfilerCommitHooks) { + if (typeof onCommit === 'function') { + onCommit( + finishedWork.memoizedProps.id, + phase, + effectDuration, + commitTime, + ); + } + } + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } +} diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 0f8fa68d0ca79..59f4420137791 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -30,7 +30,6 @@ import type { OffscreenQueue, OffscreenProps, } from './ReactFiberActivityComponent'; -import type {HookFlags} from './ReactHookEffectTags'; import type {Cache} from './ReactFiberCacheComponent'; import type {RootState} from './ReactFiberRoot'; import type { @@ -54,7 +53,6 @@ import { enableTransitionTracing, enableUseEffectEventHook, enableLegacyHidden, - disableStringRefs, disableLegacyMode, } from 'shared/ReactFeatureFlags'; import { @@ -101,16 +99,12 @@ import { FormReset, Cloned, } from './ReactFiberFlags'; -import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import {runWithFiberInDEV} from './ReactCurrentFiber'; -import {resolveClassComponentProps} from './ReactFiberClassComponent'; import { isCurrentUpdateNested, getCommitTime, recordLayoutEffectDuration, startLayoutEffectTimer, - recordPassiveEffectDuration, - startPassiveEffectTimer, } from './ReactProfilerTimer'; import {ConcurrentMode, NoMode, ProfileMode} from './ReactTypeOfMode'; import { @@ -176,7 +170,6 @@ import { addMarkerProgressCallbackToPendingTransition, addMarkerIncompleteCallbackToPendingTransition, addMarkerCompleteCallbackToPendingTransition, - setIsRunningInsertionEffect, getExecutionContext, CommitContext, NoContext, @@ -188,16 +181,9 @@ import { Insertion as HookInsertion, Passive as HookPassive, } from './ReactHookEffectTags'; -import {didWarnAboutReassigningProps} from './ReactFiberBeginWork'; import {doesFiberContain} from './ReactFiberTreeReflection'; import { isDevToolsPresent, - markComponentPassiveEffectMountStarted, - markComponentPassiveEffectMountStopped, - markComponentPassiveEffectUnmountStarted, - markComponentPassiveEffectUnmountStopped, - markComponentLayoutEffectMountStarted, - markComponentLayoutEffectMountStopped, markComponentLayoutEffectUnmountStarted, markComponentLayoutEffectUnmountStopped, onCommitUnmount, @@ -216,17 +202,20 @@ import { import {scheduleUpdateOnFiber} from './ReactFiberWorkLoop'; import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates'; import { - callComponentDidMountInDEV, - callComponentDidUpdateInDEV, - callComponentWillUnmountInDEV, - callCreateInDEV, - callDestroyInDEV, -} from './ReactFiberCallUserSpace'; - -let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; -if (__DEV__) { - didWarnAboutUndefinedSnapshotBeforeUpdate = new Set(); -} + commitHookLayoutEffects, + commitHookEffectListMount, + commitHookEffectListUnmount, + commitHookPassiveMountEffects, + commitHookPassiveUnmountEffects, + commitClassLayoutLifecycles, + commitClassCallbacks, + commitClassSnapshot, + safelyCallComponentWillUnmount, + safelyAttachRef, + safelyDetachRef, + safelyCallDestroy, + commitProfilerUpdate, +} from './ReactFiberCommitEffects'; // Used during the commit phase to track the state of the Offscreen component stack. // Allows us to avoid traversing the return path to find the nearest Offscreen ancestor. @@ -253,117 +242,6 @@ function shouldProfile(current: Fiber): boolean { ); } -// Capture errors so they don't interrupt unmounting. -function safelyCallComponentWillUnmount( - current: Fiber, - nearestMountedAncestor: Fiber | null, - instance: any, -) { - instance.props = resolveClassComponentProps( - current.type, - current.memoizedProps, - current.elementType === current.type, - ); - instance.state = current.memoizedState; - if (shouldProfile(current)) { - startLayoutEffectTimer(); - if (__DEV__) { - callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); - } else { - try { - instance.componentWillUnmount(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } - recordLayoutEffectDuration(current); - } else { - if (__DEV__) { - callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); - } else { - try { - instance.componentWillUnmount(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } - } -} - -// Capture errors so they don't interrupt mounting. -function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { - try { - commitAttachRef(current); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } -} - -function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { - const ref = current.ref; - const refCleanup = current.refCleanup; - - if (ref !== null) { - if (typeof refCleanup === 'function') { - try { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - refCleanup(); - } finally { - recordLayoutEffectDuration(current); - } - } else { - refCleanup(); - } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } finally { - // `refCleanup` has been called. Nullify all references to it to prevent double invocation. - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; - } - } - } else if (typeof ref === 'function') { - try { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - ref(null); - } finally { - recordLayoutEffectDuration(current); - } - } else { - ref(null); - } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } else { - // $FlowFixMe[incompatible-use] unable to narrow type to RefObject - ref.current = null; - } - } -} - -function safelyCallDestroy( - current: Fiber, - nearestMountedAncestor: Fiber | null, - destroy: () => void, -) { - if (__DEV__) { - callDestroyInDEV(current, nearestMountedAncestor, destroy); - } else { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } -} - let focusedInstanceHandle: null | Fiber = null; let shouldFireAfterActiveInstanceBlur: boolean = false; @@ -417,14 +295,10 @@ function commitBeforeMutationEffects_begin() { function commitBeforeMutationEffects_complete() { while (nextEffect !== null) { const fiber = nextEffect; - try { - if (__DEV__) { - runWithFiberInDEV(fiber, commitBeforeMutationEffectsOnFiber, fiber); - } else { - commitBeforeMutationEffectsOnFiber(fiber); - } - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); + if (__DEV__) { + runWithFiberInDEV(fiber, commitBeforeMutationEffectsOnFiber, fiber); + } else { + commitBeforeMutationEffectsOnFiber(fiber); } const sibling = fiber.sibling; @@ -462,7 +336,16 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) { case FunctionComponent: { if (enableUseEffectEventHook) { if ((flags & Update) !== NoFlags) { - commitUseEffectEventMount(finishedWork); + const updateQueue: FunctionComponentUpdateQueue | null = + (finishedWork.updateQueue: any); + const eventPayloads = + updateQueue !== null ? updateQueue.events : null; + if (eventPayloads !== null) { + for (let ii = 0; ii < eventPayloads.length; ii++) { + const {ref, nextImpl} = eventPayloads[ii]; + ref.impl = nextImpl; + } + } } } break; @@ -474,61 +357,7 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) { case ClassComponent: { if ((flags & Snapshot) !== NoFlags) { if (current !== null) { - const prevProps = current.memoizedProps; - const prevState = current.memoizedState; - const instance = finishedWork.stateNode; - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. - if (__DEV__) { - if ( - !finishedWork.type.defaultProps && - !('ref' in finishedWork.memoizedProps) && - !didWarnAboutReassigningProps - ) { - if (instance.props !== finishedWork.memoizedProps) { - console.error( - 'Expected %s props to match memoized props before ' + - 'getSnapshotBeforeUpdate. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.props`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - if (instance.state !== finishedWork.memoizedState) { - console.error( - 'Expected %s state to match memoized state before ' + - 'getSnapshotBeforeUpdate. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.state`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - } - } - const snapshot = instance.getSnapshotBeforeUpdate( - resolveClassComponentProps( - finishedWork.type, - prevProps, - finishedWork.elementType === finishedWork.type, - ), - prevState, - ); - if (__DEV__) { - const didWarnSet = - ((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), - ); - } - } - instance.__reactInternalSnapshotBeforeUpdate = snapshot; + commitClassSnapshot(finishedWork, current); } } break; @@ -574,161 +403,6 @@ function commitBeforeMutationEffectsDeletion(deletion: Fiber) { } } -function commitHookEffectListUnmount( - flags: HookFlags, - finishedWork: Fiber, - nearestMountedAncestor: Fiber | null, -) { - const updateQueue: FunctionComponentUpdateQueue | null = - (finishedWork.updateQueue: any); - const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; - if (lastEffect !== null) { - const firstEffect = lastEffect.next; - let effect = firstEffect; - do { - if ((effect.tag & flags) === flags) { - // Unmount - const inst = effect.inst; - const destroy = inst.destroy; - if (destroy !== undefined) { - inst.destroy = undefined; - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectUnmountStarted(finishedWork); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectUnmountStarted(finishedWork); - } - } - - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(true); - } - } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(false); - } - } - - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectUnmountStopped(); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectUnmountStopped(); - } - } - } - } - effect = effect.next; - } while (effect !== firstEffect); - } -} - -function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { - const updateQueue: FunctionComponentUpdateQueue | null = - (finishedWork.updateQueue: any); - const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; - if (lastEffect !== null) { - const firstEffect = lastEffect.next; - let effect = firstEffect; - do { - if ((effect.tag & flags) === flags) { - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectMountStarted(finishedWork); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectMountStarted(finishedWork); - } - } - - // Mount - let destroy; - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(true); - } - destroy = callCreateInDEV(effect); - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(false); - } - } else { - const create = effect.create; - const inst = effect.inst; - destroy = create(); - inst.destroy = destroy; - } - - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectMountStopped(); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectMountStopped(); - } - } - - if (__DEV__) { - if (destroy !== undefined && typeof destroy !== 'function') { - let hookName; - if ((effect.tag & HookLayout) !== NoFlags) { - hookName = 'useLayoutEffect'; - } else if ((effect.tag & HookInsertion) !== NoFlags) { - hookName = 'useInsertionEffect'; - } else { - hookName = 'useEffect'; - } - let addendum; - if (destroy === null) { - addendum = - ' You returned null. If your effect does not require clean ' + - 'up, return undefined (or nothing).'; - } else if (typeof destroy.then === 'function') { - addendum = - '\n\nIt looks like you wrote ' + - hookName + - '(async () => ...) or returned a Promise. ' + - 'Instead, write the async function inside your effect ' + - 'and call it immediately:\n\n' + - hookName + - '(() => {\n' + - ' async function fetchData() {\n' + - ' // You can await here\n' + - ' const response = await MyAPI.getData(someId);\n' + - ' // ...\n' + - ' }\n' + - ' fetchData();\n' + - `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + - 'Learn more about data fetching with Hooks: https://react.dev/link/hooks-data-fetching'; - } else { - addendum = ' You returned: ' + destroy; - } - console.error( - '%s must not return anything besides a function, ' + - 'which is used for clean-up.%s', - hookName, - addendum, - ); - } - } - } - effect = effect.next; - } while (effect !== firstEffect); - } -} - -function commitUseEffectEventMount(finishedWork: Fiber) { - const updateQueue: FunctionComponentUpdateQueue | null = - (finishedWork.updateQueue: any); - const eventPayloads = updateQueue !== null ? updateQueue.events : null; - if (eventPayloads !== null) { - for (let ii = 0; ii < eventPayloads.length; ii++) { - const {ref, nextImpl} = eventPayloads[ii]; - ref.impl = nextImpl; - } - } -} - export function commitPassiveEffectDurations( finishedRoot: FiberRoot, finishedWork: Fiber, @@ -785,218 +459,6 @@ export function commitPassiveEffectDurations( } } -function commitHookLayoutEffects(finishedWork: Fiber, hookFlags: HookFlags) { - // At this point layout effects have already been destroyed (during mutation phase). - // This is done to prevent sibling component effects from interfering with each other, - // e.g. a destroy function in one component should never override a ref set - // by a create function in another component during the same commit. - if (shouldProfile(finishedWork)) { - try { - startLayoutEffectTimer(); - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - recordLayoutEffectDuration(finishedWork); - } else { - try { - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } -} - -function commitClassLayoutLifecycles( - finishedWork: Fiber, - current: Fiber | null, -) { - const instance = finishedWork.stateNode; - if (current === null) { - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. - if (__DEV__) { - if ( - !finishedWork.type.defaultProps && - !('ref' in finishedWork.memoizedProps) && - !didWarnAboutReassigningProps - ) { - if (instance.props !== finishedWork.memoizedProps) { - console.error( - 'Expected %s props to match memoized props before ' + - 'componentDidMount. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.props`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - if (instance.state !== finishedWork.memoizedState) { - console.error( - 'Expected %s state to match memoized state before ' + - 'componentDidMount. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.state`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - } - } - if (shouldProfile(finishedWork)) { - startLayoutEffectTimer(); - if (__DEV__) { - callComponentDidMountInDEV(finishedWork, instance); - } else { - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } - recordLayoutEffectDuration(finishedWork); - } else { - if (__DEV__) { - callComponentDidMountInDEV(finishedWork, instance); - } else { - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } - } - } else { - const prevProps = resolveClassComponentProps( - finishedWork.type, - current.memoizedProps, - finishedWork.elementType === finishedWork.type, - ); - const prevState = current.memoizedState; - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. - if (__DEV__) { - if ( - !finishedWork.type.defaultProps && - !('ref' in finishedWork.memoizedProps) && - !didWarnAboutReassigningProps - ) { - if (instance.props !== finishedWork.memoizedProps) { - console.error( - 'Expected %s props to match memoized props before ' + - 'componentDidUpdate. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.props`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - if (instance.state !== finishedWork.memoizedState) { - console.error( - 'Expected %s state to match memoized state before ' + - 'componentDidUpdate. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.state`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - } - } - if (shouldProfile(finishedWork)) { - startLayoutEffectTimer(); - if (__DEV__) { - callComponentDidUpdateInDEV( - finishedWork, - instance, - prevProps, - prevState, - instance.__reactInternalSnapshotBeforeUpdate, - ); - } else { - try { - instance.componentDidUpdate( - prevProps, - prevState, - instance.__reactInternalSnapshotBeforeUpdate, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } - recordLayoutEffectDuration(finishedWork); - } else { - if (__DEV__) { - callComponentDidUpdateInDEV( - finishedWork, - instance, - prevProps, - prevState, - instance.__reactInternalSnapshotBeforeUpdate, - ); - } else { - try { - instance.componentDidUpdate( - prevProps, - prevState, - instance.__reactInternalSnapshotBeforeUpdate, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } - } - } -} - -function commitClassCallbacks(finishedWork: Fiber) { - // TODO: I think this is now always non-null by the time it reaches the - // commit phase. Consider removing the type check. - const updateQueue: UpdateQueue | null = - (finishedWork.updateQueue: any); - if (updateQueue !== null) { - const instance = finishedWork.stateNode; - if (__DEV__) { - if ( - !finishedWork.type.defaultProps && - !('ref' in finishedWork.memoizedProps) && - !didWarnAboutReassigningProps - ) { - if (instance.props !== finishedWork.memoizedProps) { - console.error( - 'Expected %s props to match memoized props before ' + - 'processing the update queue. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.props`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - if (instance.state !== finishedWork.memoizedState) { - console.error( - 'Expected %s state to match memoized state before ' + - 'processing the update queue. ' + - 'This might either be because of a bug in React, or because ' + - 'a component reassigns its own `this.state`. ' + - 'Please file an issue.', - getComponentNameFromFiber(finishedWork) || 'instance', - ); - } - } - } - // We could update instance props and state here, - // but instead we rely on them being set during last render. - // TODO: revisit this when we implement resuming. - try { - commitCallbacks(updateQueue, instance); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } -} - function commitHostComponentMount(finishedWork: Fiber) { const type = finishedWork.type; const props = finishedWork.memoizedProps; @@ -1008,70 +470,6 @@ function commitHostComponentMount(finishedWork: Fiber) { } } -function commitProfilerUpdate(finishedWork: Fiber, current: Fiber | null) { - if (enableProfilerTimer && getExecutionContext() & CommitContext) { - try { - const {onCommit, onRender} = finishedWork.memoizedProps; - const {effectDuration} = finishedWork.stateNode; - - const commitTime = getCommitTime(); - - 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, - ); - } - - // Schedule a passive effect for this Profiler to call onPostCommit hooks. - // This effect should be scheduled even if there is no onPostCommit callback for this Profiler, - // because the effect is also where times bubble to parent Profilers. - enqueuePendingPassiveProfilerEffect(finishedWork); - - // Propagate layout effect durations to the next nearest Profiler ancestor. - // Do not reset these values until the next render so DevTools has a chance to read them first. - let parentFiber = finishedWork.return; - outer: while (parentFiber !== null) { - switch (parentFiber.tag) { - case HostRoot: - const root = parentFiber.stateNode; - root.effectDuration += effectDuration; - break outer; - case Profiler: - const parentStateNode = parentFiber.stateNode; - parentStateNode.effectDuration += effectDuration; - break outer; - } - parentFiber = parentFiber.return; - } - } - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } -} - function commitLayoutEffectOnFiber( finishedRoot: FiberRoot, current: Fiber | null, @@ -1192,7 +590,36 @@ function commitLayoutEffectOnFiber( // TODO: Should this fire inside an offscreen tree? Or should it wait to // fire when the tree becomes visible again. if (flags & Update) { - commitProfilerUpdate(finishedWork, current); + const {effectDuration} = finishedWork.stateNode; + + commitProfilerUpdate( + finishedWork, + current, + getCommitTime(), + effectDuration, + ); + + // Schedule a passive effect for this Profiler to call onPostCommit hooks. + // This effect should be scheduled even if there is no onPostCommit callback for this Profiler, + // because the effect is also where times bubble to parent Profilers. + enqueuePendingPassiveProfilerEffect(finishedWork); + + // Propagate layout effect durations to the next nearest Profiler ancestor. + // Do not reset these values until the next render so DevTools has a chance to read them first. + let parentFiber = finishedWork.return; + outer: while (parentFiber !== null) { + switch (parentFiber.tag) { + case HostRoot: + const root = parentFiber.stateNode; + root.effectDuration += effectDuration; + break outer; + case Profiler: + const parentStateNode = parentFiber.stateNode; + parentStateNode.effectDuration += effectDuration; + break outer; + } + parentFiber = parentFiber.return; + } } break; } @@ -1621,56 +1048,6 @@ function hideOrUnhideAllChildren(finishedWork: Fiber, isHidden: boolean) { } } -function commitAttachRef(finishedWork: Fiber) { - const ref = finishedWork.ref; - if (ref !== null) { - const instance = finishedWork.stateNode; - let instanceToUse; - switch (finishedWork.tag) { - case HostHoistable: - case HostSingleton: - case HostComponent: - instanceToUse = getPublicInstance(instance); - break; - default: - instanceToUse = instance; - } - // Moved outside to ensure DCE works with this flag - if (enableScopeAPI && finishedWork.tag === ScopeComponent) { - instanceToUse = instance; - } - if (typeof ref === 'function') { - if (shouldProfile(finishedWork)) { - try { - startLayoutEffectTimer(); - finishedWork.refCleanup = ref(instanceToUse); - } finally { - recordLayoutEffectDuration(finishedWork); - } - } else { - finishedWork.refCleanup = ref(instanceToUse); - } - } else { - if (__DEV__) { - // TODO: We should move these warnings to happen during the render - // phase (markRef). - if (disableStringRefs && typeof ref === 'string') { - console.error('String refs are no longer supported.'); - } else if (!ref.hasOwnProperty('current')) { - console.error( - 'Unexpected ref object provided for %s. ' + - 'Use either a ref-setter function or React.createRef().', - getComponentNameFromFiber(finishedWork), - ); - } - } - - // $FlowFixMe[incompatible-use] unable to narrow type to the non-function case - ref.current = instanceToUse; - } - } -} - function detachFiberMutation(fiber: Fiber) { // Cut off the return pointer to disconnect it from the tree. // This enables us to detect and warn against state updates on an unmounted component. @@ -2340,7 +1717,7 @@ function commitDeletionEffectsOnFiber( } } function commitSuspenseCallback(finishedWork: Fiber) { - // TODO: Move this to passive phase + // TODO: Delete this feature. It's not properly covered by DEV features. const newState: SuspenseState | null = finishedWork.memoizedState; if (enableSuspenseCallback && newState !== null) { const suspenseCallback = finishedWork.memoizedProps.suspenseCallback; @@ -2375,6 +1752,7 @@ function commitSuspenseHydrationCallbacks( try { commitHydratedSuspenseInstance(suspenseInstance); if (enableSuspenseCallback) { + // TODO: Delete this feature. It's not properly covered by DEV features. const hydrationCallbacks = finishedRoot.hydrationCallbacks; if (hydrationCallbacks !== null) { const onHydrated = hydrationCallbacks.onHydrated; @@ -2499,7 +1877,7 @@ function attachSuspenseRetryListeners( // This function detects when a Suspense boundary goes from visible to hidden. // It returns false if the boundary is already hidden. // TODO: Use an effect tag. -export function isSuspenseBoundaryBeingHidden( +function isSuspenseBoundaryBeingHidden( current: Fiber | null, finishedWork: Fiber, ): boolean { @@ -3400,7 +2778,36 @@ export function reappearLayoutEffects( ); // TODO: Figure out how Profiler updates should work with Offscreen if (includeWorkInProgressEffects && flags & Update) { - commitProfilerUpdate(finishedWork, current); + const {effectDuration} = finishedWork.stateNode; + + commitProfilerUpdate( + finishedWork, + current, + getCommitTime(), + effectDuration, + ); + + // Schedule a passive effect for this Profiler to call onPostCommit hooks. + // This effect should be scheduled even if there is no onPostCommit callback for this Profiler, + // because the effect is also where times bubble to parent Profilers. + enqueuePendingPassiveProfilerEffect(finishedWork); + + // Propagate layout effect durations to the next nearest Profiler ancestor. + // Do not reset these values until the next render so DevTools has a chance to read them first. + let parentFiber = finishedWork.return; + outer: while (parentFiber !== null) { + switch (parentFiber.tag) { + case HostRoot: + const root = parentFiber.stateNode; + root.effectDuration += effectDuration; + break outer; + case Profiler: + const parentStateNode = parentFiber.stateNode; + parentStateNode.effectDuration += effectDuration; + break outer; + } + parentFiber = parentFiber.return; + } } break; } @@ -3411,9 +2818,8 @@ export function reappearLayoutEffects( includeWorkInProgressEffects, ); - // TODO: Figure out how Suspense hydration callbacks should work - // with Offscreen. if (includeWorkInProgressEffects && flags & Update) { + // TODO: Delete this feature. commitSuspenseHydrationCallbacks(finishedRoot, finishedWork); } break; @@ -3482,27 +2888,6 @@ function recursivelyTraverseReappearLayoutEffects( } } -function commitHookPassiveMountEffects( - finishedWork: Fiber, - hookFlags: HookFlags, -) { - if (shouldProfile(finishedWork)) { - startPassiveEffectTimer(); - try { - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - recordPassiveEffectDuration(finishedWork); - } else { - try { - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } -} - function commitOffscreenPassiveMountEffects( current: Fiber | null, finishedWork: Fiber, @@ -4341,28 +3726,6 @@ function detachAlternateSiblings(parentFiber: Fiber) { } } -function commitHookPassiveUnmountEffects( - finishedWork: Fiber, - nearestMountedAncestor: null | Fiber, - hookFlags: HookFlags, -) { - if (shouldProfile(finishedWork)) { - startPassiveEffectTimer(); - commitHookEffectListUnmount( - hookFlags, - finishedWork, - nearestMountedAncestor, - ); - recordPassiveEffectDuration(finishedWork); - } else { - commitHookEffectListUnmount( - hookFlags, - finishedWork, - nearestMountedAncestor, - ); - } -} - function recursivelyTraversePassiveUnmountEffects(parentFiber: Fiber): void { // Deletions effects can be scheduled on any fiber type. They need to happen // before the children effects have fired. @@ -4696,7 +4059,7 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( } } -function invokeLayoutEffectMountInDEV(fiber: Fiber): void { +export function invokeLayoutEffectMountInDEV(fiber: Fiber): void { if (__DEV__) { // We don't need to re-check StrictEffectsMode here. // This function is only called if that check has already passed. @@ -4726,7 +4089,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void { } } -function invokePassiveEffectMountInDEV(fiber: Fiber): void { +export function invokePassiveEffectMountInDEV(fiber: Fiber): void { if (__DEV__) { // We don't need to re-check StrictEffectsMode here. // This function is only called if that check has already passed. @@ -4745,7 +4108,7 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void { } } -function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { +export function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { if (__DEV__) { // We don't need to re-check StrictEffectsMode here. // This function is only called if that check has already passed. @@ -4775,7 +4138,7 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { } } -function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { +export function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { if (__DEV__) { // We don't need to re-check StrictEffectsMode here. // This function is only called if that check has already passed. @@ -4796,12 +4159,3 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { } } } - -export { - commitPlacement, - commitAttachRef, - invokeLayoutEffectMountInDEV, - invokeLayoutEffectUnmountInDEV, - invokePassiveEffectMountInDEV, - invokePassiveEffectUnmountInDEV, -}; From 6e36b71252368c991950bbf18474dc0e8f2bdb68 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 14:20:42 -0400 Subject: [PATCH 2/7] Remove unneeded catch This should really be covered by the inner functions. --- packages/react-reconciler/src/ReactFiberCommitWork.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 59f4420137791..98fbcd5031a5a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1423,6 +1423,7 @@ function commitDeletionEffectsOnFiber( nearestMountedAncestor: Fiber, deletedFiber: Fiber, ) { + // TODO: Delete this Hook once new DevTools ships everywhere. No longer needed. onCommitUnmount(deletedFiber); // The cases in this outer switch modify the stack before they traverse @@ -1926,11 +1927,7 @@ function recursivelyTraverseMutationEffects( if (deletions !== null) { for (let i = 0; i < deletions.length; i++) { const childToDelete = deletions[i]; - try { - commitDeletionEffects(root, parentFiber, childToDelete); - } catch (error) { - captureCommitPhaseError(childToDelete, parentFiber, error); - } + commitDeletionEffects(root, parentFiber, childToDelete); } } From 81b377ff2c45cb0d68e85fa98437c07da2ea7c1d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 14:25:41 -0400 Subject: [PATCH 3/7] Extract class methods into helpers --- .../src/ReactFiberCommitEffects.js | 63 ++++++++++++++++++- .../src/ReactFiberCommitWork.js | 61 +++--------------- 2 files changed, 70 insertions(+), 54 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index d981f8279f31e..9f8a4d5e2964b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -21,6 +21,7 @@ import { disableStringRefs, } from 'shared/ReactFeatureFlags'; import { + ClassComponent, HostComponent, HostHoistable, HostSingleton, @@ -37,7 +38,10 @@ import { isCurrentUpdateNested, } from './ReactProfilerTimer'; import {NoMode, ProfileMode} from './ReactTypeOfMode'; -import {commitCallbacks} from './ReactFiberClassUpdateQueue'; +import { + commitCallbacks, + commitHiddenCallbacks, +} from './ReactFiberClassUpdateQueue'; import {getPublicInstance} from './ReactFiberConfig'; import { captureCommitPhaseError, @@ -438,6 +442,22 @@ export function commitClassLayoutLifecycles( } } +export function commitClassDidMount(finishedWork: Fiber) { + // TODO: Check for LayoutStatic flag + const instance = finishedWork.stateNode; + if (typeof instance.componentDidMount === 'function') { + if (__DEV__) { + callComponentDidMountInDEV(finishedWork, instance); + } else { + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } + } +} + export function commitClassCallbacks(finishedWork: Fiber) { // TODO: I think this is now always non-null by the time it reaches the // commit phase. Consider removing the type check. @@ -484,6 +504,47 @@ export function commitClassCallbacks(finishedWork: Fiber) { } } +export function commitClassHiddenCallbacks(finishedWork: Fiber) { + // Commit any callbacks that would have fired while the component + // was hidden. + const updateQueue: UpdateQueue | null = + (finishedWork.updateQueue: any); + if (updateQueue !== null) { + const instance = finishedWork.stateNode; + try { + commitHiddenCallbacks(updateQueue, instance); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } +} + +export function commitRootCallbacks(finishedWork: Fiber) { + // TODO: I think this is now always non-null by the time it reaches the + // commit phase. Consider removing the type check. + const updateQueue: UpdateQueue | null = + (finishedWork.updateQueue: any); + if (updateQueue !== null) { + let instance = null; + if (finishedWork.child !== null) { + switch (finishedWork.child.tag) { + case HostSingleton: + case HostComponent: + instance = getPublicInstance(finishedWork.child.stateNode); + break; + case ClassComponent: + instance = finishedWork.child.stateNode; + break; + } + } + try { + commitCallbacks(updateQueue, instance); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + } +} + let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { didWarnAboutUndefinedSnapshotBeforeUpdate = new Set(); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 98fbcd5031a5a..c19fe484f2ae1 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -107,13 +107,8 @@ import { startLayoutEffectTimer, } from './ReactProfilerTimer'; import {ConcurrentMode, NoMode, ProfileMode} from './ReactTypeOfMode'; +import {deferHiddenCallbacks} from './ReactFiberClassUpdateQueue'; import { - deferHiddenCallbacks, - commitHiddenCallbacks, - commitCallbacks, -} from './ReactFiberClassUpdateQueue'; -import { - getPublicInstance, supportsMutation, supportsPersistence, supportsHydration, @@ -208,13 +203,16 @@ import { commitHookPassiveMountEffects, commitHookPassiveUnmountEffects, commitClassLayoutLifecycles, + commitClassDidMount, commitClassCallbacks, + commitClassHiddenCallbacks, commitClassSnapshot, safelyCallComponentWillUnmount, safelyAttachRef, safelyDetachRef, safelyCallDestroy, commitProfilerUpdate, + commitRootCallbacks, } from './ReactFiberCommitEffects'; // Used during the commit phase to track the state of the Offscreen component stack. @@ -519,29 +517,7 @@ function commitLayoutEffectOnFiber( committedLanes, ); if (flags & Callback) { - // TODO: I think this is now always non-null by the time it reaches the - // commit phase. Consider removing the type check. - const updateQueue: UpdateQueue | null = - (finishedWork.updateQueue: any); - if (updateQueue !== null) { - let instance = null; - if (finishedWork.child !== null) { - switch (finishedWork.child.tag) { - case HostSingleton: - case HostComponent: - instance = getPublicInstance(finishedWork.child.stateNode); - break; - case ClassComponent: - instance = finishedWork.child.stateNode; - break; - } - } - try { - commitCallbacks(updateQueue, instance); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } + commitRootCallbacks(finishedWork); } break; } @@ -2713,23 +2689,9 @@ export function reappearLayoutEffects( includeWorkInProgressEffects, ); - // TODO: Check for LayoutStatic flag - const instance = finishedWork.stateNode; - if (typeof instance.componentDidMount === 'function') { - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } + commitClassDidMount(finishedWork); - // Commit any callbacks that would have fired while the component - // was hidden. - const updateQueue: UpdateQueue | null = - (finishedWork.updateQueue: any); - if (updateQueue !== null) { - commitHiddenCallbacks(updateQueue, instance); - } + commitClassHiddenCallbacks(finishedWork); // If this is newly finished work, check for setState callbacks if (includeWorkInProgressEffects && flags & Callback) { @@ -4072,14 +4034,7 @@ export function invokeLayoutEffectMountInDEV(fiber: Fiber): void { break; } case ClassComponent: { - const instance = fiber.stateNode; - if (typeof instance.componentDidMount === 'function') { - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); - } - } + commitClassDidMount(fiber); break; } } From 03e124c8f00633ec4b90407a6546ad7988ddf2d8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 16:33:53 -0400 Subject: [PATCH 4/7] Move try/catch into commitHookEffectList(Un)Mount The effective change is only that errors in unmount insertion effects don't abort mount insertion effects. We probably should abort future phases when earlier phases abort we don't currently. --- .../src/ReactFiberCommitEffects.js | 262 +++++++++--------- .../src/ReactFiberCommitWork.js | 89 ++---- 2 files changed, 156 insertions(+), 195 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 9f8a4d5e2964b..04b8120283a61 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -93,19 +93,11 @@ export function commitHookLayoutEffects( // e.g. a destroy function in one component should never override a ref set // by a create function in another component during the same commit. if (shouldProfile(finishedWork)) { - try { - startLayoutEffectTimer(); - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + startLayoutEffectTimer(); + commitHookEffectListMount(hookFlags, finishedWork); recordLayoutEffectDuration(finishedWork); } else { - try { - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHookEffectListMount(hookFlags, finishedWork); } } @@ -113,93 +105,97 @@ export function commitHookEffectListMount( flags: HookFlags, finishedWork: Fiber, ) { - const updateQueue: FunctionComponentUpdateQueue | null = - (finishedWork.updateQueue: any); - const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; - if (lastEffect !== null) { - const firstEffect = lastEffect.next; - let effect = firstEffect; - do { - if ((effect.tag & flags) === flags) { - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectMountStarted(finishedWork); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectMountStarted(finishedWork); + try { + const updateQueue: FunctionComponentUpdateQueue | null = + (finishedWork.updateQueue: any); + const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; + if (lastEffect !== null) { + const firstEffect = lastEffect.next; + let effect = firstEffect; + do { + if ((effect.tag & flags) === flags) { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStarted(finishedWork); + } } - } - // Mount - let destroy; - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(true); - } - destroy = callCreateInDEV(effect); - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(false); + // Mount + let destroy; + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + destroy = callCreateInDEV(effect); + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } else { + const create = effect.create; + const inst = effect.inst; + destroy = create(); + inst.destroy = destroy; } - } else { - const create = effect.create; - const inst = effect.inst; - destroy = create(); - inst.destroy = destroy; - } - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectMountStopped(); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectMountStopped(); + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStopped(); + } } - } - if (__DEV__) { - if (destroy !== undefined && typeof destroy !== 'function') { - let hookName; - if ((effect.tag & HookLayout) !== NoFlags) { - hookName = 'useLayoutEffect'; - } else if ((effect.tag & HookInsertion) !== NoFlags) { - hookName = 'useInsertionEffect'; - } else { - hookName = 'useEffect'; - } - let addendum; - if (destroy === null) { - addendum = - ' You returned null. If your effect does not require clean ' + - 'up, return undefined (or nothing).'; - } else if (typeof destroy.then === 'function') { - addendum = - '\n\nIt looks like you wrote ' + - hookName + - '(async () => ...) or returned a Promise. ' + - 'Instead, write the async function inside your effect ' + - 'and call it immediately:\n\n' + - hookName + - '(() => {\n' + - ' async function fetchData() {\n' + - ' // You can await here\n' + - ' const response = await MyAPI.getData(someId);\n' + - ' // ...\n' + - ' }\n' + - ' fetchData();\n' + - `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + - 'Learn more about data fetching with Hooks: https://react.dev/link/hooks-data-fetching'; - } else { - addendum = ' You returned: ' + destroy; + if (__DEV__) { + if (destroy !== undefined && typeof destroy !== 'function') { + let hookName; + if ((effect.tag & HookLayout) !== NoFlags) { + hookName = 'useLayoutEffect'; + } else if ((effect.tag & HookInsertion) !== NoFlags) { + hookName = 'useInsertionEffect'; + } else { + hookName = 'useEffect'; + } + let addendum; + if (destroy === null) { + addendum = + ' You returned null. If your effect does not require clean ' + + 'up, return undefined (or nothing).'; + } else if (typeof destroy.then === 'function') { + addendum = + '\n\nIt looks like you wrote ' + + hookName + + '(async () => ...) or returned a Promise. ' + + 'Instead, write the async function inside your effect ' + + 'and call it immediately:\n\n' + + hookName + + '(() => {\n' + + ' async function fetchData() {\n' + + ' // You can await here\n' + + ' const response = await MyAPI.getData(someId);\n' + + ' // ...\n' + + ' }\n' + + ' fetchData();\n' + + `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + + 'Learn more about data fetching with Hooks: https://react.dev/link/hooks-data-fetching'; + } else { + addendum = ' You returned: ' + destroy; + } + console.error( + '%s must not return anything besides a function, ' + + 'which is used for clean-up.%s', + hookName, + addendum, + ); } - console.error( - '%s must not return anything besides a function, ' + - 'which is used for clean-up.%s', - hookName, - addendum, - ); } } - } - effect = effect.next; - } while (effect !== firstEffect); + effect = effect.next; + } while (effect !== firstEffect); + } + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); } } @@ -208,50 +204,54 @@ export function commitHookEffectListUnmount( finishedWork: Fiber, nearestMountedAncestor: Fiber | null, ) { - const updateQueue: FunctionComponentUpdateQueue | null = - (finishedWork.updateQueue: any); - const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; - if (lastEffect !== null) { - const firstEffect = lastEffect.next; - let effect = firstEffect; - do { - if ((effect.tag & flags) === flags) { - // Unmount - const inst = effect.inst; - const destroy = inst.destroy; - if (destroy !== undefined) { - inst.destroy = undefined; - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectUnmountStarted(finishedWork); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectUnmountStarted(finishedWork); + try { + const updateQueue: FunctionComponentUpdateQueue | null = + (finishedWork.updateQueue: any); + const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; + if (lastEffect !== null) { + const firstEffect = lastEffect.next; + let effect = firstEffect; + do { + if ((effect.tag & flags) === flags) { + // Unmount + const inst = effect.inst; + const destroy = inst.destroy; + if (destroy !== undefined) { + inst.destroy = undefined; + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStarted(finishedWork); + } } - } - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(true); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } } - } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); - if (__DEV__) { - if ((flags & HookInsertion) !== NoHookEffect) { - setIsRunningInsertionEffect(false); + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } } - } - if (enableSchedulingProfiler) { - if ((flags & HookPassive) !== NoHookEffect) { - markComponentPassiveEffectUnmountStopped(); - } else if ((flags & HookLayout) !== NoHookEffect) { - markComponentLayoutEffectUnmountStopped(); + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStopped(); + } } } } - } - effect = effect.next; - } while (effect !== firstEffect); + effect = effect.next; + } while (effect !== firstEffect); + } + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); } } @@ -261,18 +261,10 @@ export function commitHookPassiveMountEffects( ) { if (shouldProfile(finishedWork)) { startPassiveEffectTimer(); - try { - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHookEffectListMount(hookFlags, finishedWork); recordPassiveEffectDuration(finishedWork); } else { - try { - commitHookEffectListMount(hookFlags, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHookEffectListMount(hookFlags, finishedWork); } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index c19fe484f2ae1..8f98c4b7eba88 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1951,46 +1951,31 @@ function commitMutationEffectsOnFiber( commitReconciliationEffects(finishedWork); if (flags & Update) { - try { - commitHookEffectListUnmount( - HookInsertion | HookHasEffect, - finishedWork, - finishedWork.return, - ); - commitHookEffectListMount( - HookInsertion | HookHasEffect, - finishedWork, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHookEffectListUnmount( + HookInsertion | HookHasEffect, + finishedWork, + finishedWork.return, + ); + commitHookEffectListMount(HookInsertion | HookHasEffect, finishedWork); // Layout effects are destroyed during the mutation phase so that all // destroy functions for all fibers are called before any create functions. // This prevents sibling component effects from interfering with each other, // e.g. a destroy function in one component should never override a ref set // by a create function in another component during the same commit. if (shouldProfile(finishedWork)) { - try { - startLayoutEffectTimer(); - commitHookEffectListUnmount( - HookLayout | HookHasEffect, - finishedWork, - finishedWork.return, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + startLayoutEffectTimer(); + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); recordLayoutEffectDuration(finishedWork); } else { - try { - commitHookEffectListUnmount( - HookLayout | HookHasEffect, - finishedWork, - finishedWork.return, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + finishedWork, + finishedWork.return, + ); } } return; @@ -4026,11 +4011,7 @@ export function invokeLayoutEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListMount(HookLayout | HookHasEffect, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); - } + commitHookEffectListMount(HookLayout | HookHasEffect, fiber); break; } case ClassComponent: { @@ -4049,11 +4030,7 @@ export function invokePassiveEffectMountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListMount(HookPassive | HookHasEffect, fiber); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); - } + commitHookEffectListMount(HookPassive | HookHasEffect, fiber); break; } } @@ -4068,15 +4045,11 @@ export function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListUnmount( - HookLayout | HookHasEffect, - fiber, - fiber.return, - ); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); - } + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); break; } case ClassComponent: { @@ -4098,15 +4071,11 @@ export function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - try { - commitHookEffectListUnmount( - HookPassive | HookHasEffect, - fiber, - fiber.return, - ); - } catch (error) { - captureCommitPhaseError(fiber, fiber.return, error); - } + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); } } } From 6201d60297296cceed4714eecf150df75f40ff49 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 17:04:34 -0400 Subject: [PATCH 5/7] Move host effects to ReactFiberCommitHostEffects --- .../src/ReactFiberCommitHostEffects.js | 387 ++++++++++++++++++ .../src/ReactFiberCommitWork.js | 350 ++-------------- 2 files changed, 425 insertions(+), 312 deletions(-) create mode 100644 packages/react-reconciler/src/ReactFiberCommitHostEffects.js diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js new file mode 100644 index 0000000000000..e30e7732529c0 --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -0,0 +1,387 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type { + Instance, + TextInstance, + SuspenseInstance, + Container, + ChildSet, +} from './ReactFiberConfig'; +import type {Fiber, FiberRoot} from './ReactInternalTypes'; + +import { + HostRoot, + HostComponent, + HostHoistable, + HostSingleton, + HostText, + HostPortal, + DehydratedFragment, +} from './ReactWorkTags'; +import {ContentReset, Placement} from './ReactFiberFlags'; +import { + supportsMutation, + supportsResources, + supportsSingletons, + commitMount, + commitUpdate, + resetTextContent, + commitTextUpdate, + appendChild, + appendChildToContainer, + insertBefore, + insertInContainerBefore, + replaceContainerChildren, + hideInstance, + hideTextInstance, + unhideInstance, + unhideTextInstance, + commitHydratedContainer, + commitHydratedSuspenseInstance, +} from './ReactFiberConfig'; +import {captureCommitPhaseError} from './ReactFiberWorkLoop'; + +export function commitHostMount(finishedWork: Fiber) { + const type = finishedWork.type; + const props = finishedWork.memoizedProps; + const instance: Instance = finishedWork.stateNode; + try { + commitMount(instance, type, props, finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostUpdate( + finishedWork: Fiber, + newProps: any, + oldProps: any, +) { + try { + commitUpdate( + finishedWork.stateNode, + finishedWork.type, + oldProps, + newProps, + finishedWork, + ); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostTextUpdate( + finishedWork: Fiber, + newText: string, + oldText: string, +) { + const textInstance: TextInstance = finishedWork.stateNode; + try { + commitTextUpdate(textInstance, oldText, newText); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostResetTextContent(finishedWork: Fiber) { + const instance: Instance = finishedWork.stateNode; + try { + resetTextContent(instance); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitShowHideHostInstance(node: Fiber, isHidden: boolean) { + try { + const instance = node.stateNode; + if (isHidden) { + hideInstance(instance); + } else { + unhideInstance(node.stateNode, node.memoizedProps); + } + } catch (error) { + captureCommitPhaseError(node, node.return, error); + } +} + +export function commitShowHideHostTextInstance(node: Fiber, isHidden: boolean) { + try { + const instance = node.stateNode; + if (isHidden) { + hideTextInstance(instance); + } else { + unhideTextInstance(instance, node.memoizedProps); + } + } catch (error) { + captureCommitPhaseError(node, node.return, error); + } +} + +function getHostParentFiber(fiber: Fiber): Fiber { + let parent = fiber.return; + while (parent !== null) { + if (isHostParent(parent)) { + return parent; + } + parent = parent.return; + } + + throw new Error( + 'Expected to find a host parent. This error is likely caused by a bug ' + + 'in React. Please file an issue.', + ); +} + +function isHostParent(fiber: Fiber): boolean { + return ( + fiber.tag === HostComponent || + fiber.tag === HostRoot || + (supportsResources ? fiber.tag === HostHoistable : false) || + (supportsSingletons ? fiber.tag === HostSingleton : false) || + fiber.tag === HostPortal + ); +} + +function getHostSibling(fiber: Fiber): ?Instance { + // We're going to search forward into the tree until we find a sibling host + // node. Unfortunately, if multiple insertions are done in a row we have to + // search past them. This leads to exponential search for the next sibling. + // TODO: Find a more efficient way to do this. + let node: Fiber = fiber; + siblings: while (true) { + // If we didn't find anything, let's try the next sibling. + while (node.sibling === null) { + if (node.return === null || isHostParent(node.return)) { + // If we pop out of the root or hit the parent the fiber we are the + // last sibling. + return null; + } + // $FlowFixMe[incompatible-type] found when upgrading Flow + node = node.return; + } + node.sibling.return = node.return; + node = node.sibling; + while ( + node.tag !== HostComponent && + node.tag !== HostText && + (!supportsSingletons ? true : node.tag !== HostSingleton) && + node.tag !== DehydratedFragment + ) { + // If it is not host node and, we might have a host node inside it. + // Try to search down until we find one. + if (node.flags & Placement) { + // If we don't have a child, try the siblings instead. + continue siblings; + } + // If we don't have a child, try the siblings instead. + // We also skip portals because they are not part of this host tree. + if (node.child === null || node.tag === HostPortal) { + continue siblings; + } else { + node.child.return = node; + node = node.child; + } + } + // Check if this host node is stable or about to be placed. + if (!(node.flags & Placement)) { + // Found it! + return node.stateNode; + } + } +} + +function insertOrAppendPlacementNodeIntoContainer( + node: Fiber, + before: ?Instance, + parent: Container, +): void { + const {tag} = node; + const isHost = tag === HostComponent || tag === HostText; + if (isHost) { + const stateNode = node.stateNode; + if (before) { + insertInContainerBefore(parent, stateNode, before); + } else { + appendChildToContainer(parent, stateNode); + } + } else if ( + tag === HostPortal || + (supportsSingletons ? tag === HostSingleton : false) + ) { + // If the insertion itself is a portal, then we don't want to traverse + // down its children. Instead, we'll get insertions from each child in + // the portal directly. + // If the insertion is a HostSingleton then it will be placed independently + } else { + const child = node.child; + if (child !== null) { + insertOrAppendPlacementNodeIntoContainer(child, before, parent); + let sibling = child.sibling; + while (sibling !== null) { + insertOrAppendPlacementNodeIntoContainer(sibling, before, parent); + sibling = sibling.sibling; + } + } + } +} + +function insertOrAppendPlacementNode( + node: Fiber, + before: ?Instance, + parent: Instance, +): void { + const {tag} = node; + const isHost = tag === HostComponent || tag === HostText; + if (isHost) { + const stateNode = node.stateNode; + if (before) { + insertBefore(parent, stateNode, before); + } else { + appendChild(parent, stateNode); + } + } else if ( + tag === HostPortal || + (supportsSingletons ? tag === HostSingleton : false) + ) { + // If the insertion itself is a portal, then we don't want to traverse + // down its children. Instead, we'll get insertions from each child in + // the portal directly. + // If the insertion is a HostSingleton then it will be placed independently + } else { + const child = node.child; + if (child !== null) { + insertOrAppendPlacementNode(child, before, parent); + let sibling = child.sibling; + while (sibling !== null) { + insertOrAppendPlacementNode(sibling, before, parent); + sibling = sibling.sibling; + } + } + } +} + +function commitPlacement(finishedWork: Fiber): void { + if (!supportsMutation) { + return; + } + + if (supportsSingletons) { + if (finishedWork.tag === HostSingleton) { + // Singletons are already in the Host and don't need to be placed + // Since they operate somewhat like Portals though their children will + // have Placement and will get placed inside them + return; + } + } + // Recursively insert all host nodes into the parent. + const parentFiber = getHostParentFiber(finishedWork); + + switch (parentFiber.tag) { + case HostSingleton: { + if (supportsSingletons) { + const parent: Instance = parentFiber.stateNode; + const before = getHostSibling(finishedWork); + // We only have the top Fiber that was inserted but we need to recurse down its + // children to find all the terminal nodes. + insertOrAppendPlacementNode(finishedWork, before, parent); + break; + } + // Fall through + } + case HostComponent: { + const parent: Instance = parentFiber.stateNode; + if (parentFiber.flags & ContentReset) { + // Reset the text content of the parent before doing any insertions + resetTextContent(parent); + // Clear ContentReset from the effect tag + parentFiber.flags &= ~ContentReset; + } + + const before = getHostSibling(finishedWork); + // We only have the top Fiber that was inserted but we need to recurse down its + // children to find all the terminal nodes. + insertOrAppendPlacementNode(finishedWork, before, parent); + break; + } + case HostRoot: + case HostPortal: { + const parent: Container = parentFiber.stateNode.containerInfo; + const before = getHostSibling(finishedWork); + insertOrAppendPlacementNodeIntoContainer(finishedWork, before, parent); + break; + } + default: + throw new Error( + 'Invalid host parent fiber. This error is likely caused by a bug ' + + 'in React. Please file an issue.', + ); + } +} + +export function commitHostPlacement(finishedWork: Fiber) { + try { + commitPlacement(finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostRootContainerChildren( + root: FiberRoot, + finishedWork: Fiber, +) { + const containerInfo = root.containerInfo; + const pendingChildren = root.pendingChildren; + try { + replaceContainerChildren(containerInfo, pendingChildren); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostPortalContainerChildren( + portal: { + containerInfo: Container, + pendingChildren: ChildSet, + ... + }, + finishedWork: Fiber, +) { + const containerInfo = portal.containerInfo; + const pendingChildren = portal.pendingChildren; + try { + replaceContainerChildren(containerInfo, pendingChildren); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostHydratedContainer( + root: FiberRoot, + finishedWork: Fiber, +) { + try { + commitHydratedContainer(root.containerInfo); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} + +export function commitHostHydratedSuspense( + suspenseInstance: SuspenseInstance, + finishedWork: Fiber, +) { + try { + commitHydratedSuspenseInstance(suspenseInstance); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 8f98c4b7eba88..92f782f3922e2 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -114,26 +114,12 @@ import { supportsHydration, supportsResources, supportsSingletons, - commitMount, - commitUpdate, - resetTextContent, - commitTextUpdate, - appendChild, - appendChildToContainer, - insertBefore, - insertInContainerBefore, removeChild, removeChildFromContainer, clearSuspenseBoundary, clearSuspenseBoundaryFromContainer, replaceContainerChildren, createContainerChildSet, - hideInstance, - hideTextInstance, - unhideInstance, - unhideTextInstance, - commitHydratedContainer, - commitHydratedSuspenseInstance, clearContainer, prepareScopeUpdate, prepareForCommit, @@ -214,6 +200,19 @@ import { commitProfilerUpdate, commitRootCallbacks, } from './ReactFiberCommitEffects'; +import { + commitHostMount, + commitHostUpdate, + commitHostTextUpdate, + commitHostResetTextContent, + commitShowHideHostInstance, + commitShowHideHostTextInstance, + commitHostPlacement, + commitHostRootContainerChildren, + commitHostPortalContainerChildren, + commitHostHydratedContainer, + commitHostHydratedSuspense, +} from './ReactFiberCommitHostEffects'; // Used during the commit phase to track the state of the Offscreen component stack. // Allows us to avoid traversing the return path to find the nearest Offscreen ancestor. @@ -457,17 +456,6 @@ export function commitPassiveEffectDurations( } } -function commitHostComponentMount(finishedWork: Fiber) { - const type = finishedWork.type; - const props = finishedWork.memoizedProps; - const instance: Instance = finishedWork.stateNode; - try { - commitMount(instance, type, props, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } -} - function commitLayoutEffectOnFiber( finishedRoot: FiberRoot, current: Fiber | null, @@ -549,7 +537,7 @@ function commitLayoutEffectOnFiber( // These effects should only be committed when components are first mounted, // aka when there is no current/alternate. if (current === null && flags & Update) { - commitHostComponentMount(finishedWork); + commitHostMount(finishedWork); } if (flags & Ref) { @@ -961,29 +949,11 @@ function hideOrUnhideAllChildren(finishedWork: Fiber, isHidden: boolean) { ) { if (hostSubtreeRoot === null) { hostSubtreeRoot = node; - try { - const instance = node.stateNode; - if (isHidden) { - hideInstance(instance); - } else { - unhideInstance(node.stateNode, node.memoizedProps); - } - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitShowHideHostInstance(node, isHidden); } } else if (node.tag === HostText) { if (hostSubtreeRoot === null) { - try { - const instance = node.stateNode; - if (isHidden) { - hideTextInstance(instance); - } else { - unhideTextInstance(instance, node.memoizedProps); - } - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitShowHideHostTextInstance(node, isHidden); } } else if ( (node.tag === OffscreenComponent || @@ -1109,207 +1079,6 @@ function emptyPortalContainer(current: Fiber) { replaceContainerChildren(containerInfo, emptyChildSet); } -function getHostParentFiber(fiber: Fiber): Fiber { - let parent = fiber.return; - while (parent !== null) { - if (isHostParent(parent)) { - return parent; - } - parent = parent.return; - } - - throw new Error( - 'Expected to find a host parent. This error is likely caused by a bug ' + - 'in React. Please file an issue.', - ); -} - -function isHostParent(fiber: Fiber): boolean { - return ( - fiber.tag === HostComponent || - fiber.tag === HostRoot || - (supportsResources ? fiber.tag === HostHoistable : false) || - (supportsSingletons ? fiber.tag === HostSingleton : false) || - fiber.tag === HostPortal - ); -} - -function getHostSibling(fiber: Fiber): ?Instance { - // We're going to search forward into the tree until we find a sibling host - // node. Unfortunately, if multiple insertions are done in a row we have to - // search past them. This leads to exponential search for the next sibling. - // TODO: Find a more efficient way to do this. - let node: Fiber = fiber; - siblings: while (true) { - // If we didn't find anything, let's try the next sibling. - while (node.sibling === null) { - if (node.return === null || isHostParent(node.return)) { - // If we pop out of the root or hit the parent the fiber we are the - // last sibling. - return null; - } - // $FlowFixMe[incompatible-type] found when upgrading Flow - node = node.return; - } - node.sibling.return = node.return; - node = node.sibling; - while ( - node.tag !== HostComponent && - node.tag !== HostText && - (!supportsSingletons ? true : node.tag !== HostSingleton) && - node.tag !== DehydratedFragment - ) { - // If it is not host node and, we might have a host node inside it. - // Try to search down until we find one. - if (node.flags & Placement) { - // If we don't have a child, try the siblings instead. - continue siblings; - } - // If we don't have a child, try the siblings instead. - // We also skip portals because they are not part of this host tree. - if (node.child === null || node.tag === HostPortal) { - continue siblings; - } else { - node.child.return = node; - node = node.child; - } - } - // Check if this host node is stable or about to be placed. - if (!(node.flags & Placement)) { - // Found it! - return node.stateNode; - } - } -} - -function commitPlacement(finishedWork: Fiber): void { - if (!supportsMutation) { - return; - } - - if (supportsSingletons) { - if (finishedWork.tag === HostSingleton) { - // Singletons are already in the Host and don't need to be placed - // Since they operate somewhat like Portals though their children will - // have Placement and will get placed inside them - return; - } - } - // Recursively insert all host nodes into the parent. - const parentFiber = getHostParentFiber(finishedWork); - - switch (parentFiber.tag) { - case HostSingleton: { - if (supportsSingletons) { - const parent: Instance = parentFiber.stateNode; - const before = getHostSibling(finishedWork); - // We only have the top Fiber that was inserted but we need to recurse down its - // children to find all the terminal nodes. - insertOrAppendPlacementNode(finishedWork, before, parent); - break; - } - // Fall through - } - case HostComponent: { - const parent: Instance = parentFiber.stateNode; - if (parentFiber.flags & ContentReset) { - // Reset the text content of the parent before doing any insertions - resetTextContent(parent); - // Clear ContentReset from the effect tag - parentFiber.flags &= ~ContentReset; - } - - const before = getHostSibling(finishedWork); - // We only have the top Fiber that was inserted but we need to recurse down its - // children to find all the terminal nodes. - insertOrAppendPlacementNode(finishedWork, before, parent); - break; - } - case HostRoot: - case HostPortal: { - const parent: Container = parentFiber.stateNode.containerInfo; - const before = getHostSibling(finishedWork); - insertOrAppendPlacementNodeIntoContainer(finishedWork, before, parent); - break; - } - default: - throw new Error( - 'Invalid host parent fiber. This error is likely caused by a bug ' + - 'in React. Please file an issue.', - ); - } -} - -function insertOrAppendPlacementNodeIntoContainer( - node: Fiber, - before: ?Instance, - parent: Container, -): void { - const {tag} = node; - const isHost = tag === HostComponent || tag === HostText; - if (isHost) { - const stateNode = node.stateNode; - if (before) { - insertInContainerBefore(parent, stateNode, before); - } else { - appendChildToContainer(parent, stateNode); - } - } else if ( - tag === HostPortal || - (supportsSingletons ? tag === HostSingleton : false) - ) { - // If the insertion itself is a portal, then we don't want to traverse - // down its children. Instead, we'll get insertions from each child in - // the portal directly. - // If the insertion is a HostSingleton then it will be placed independently - } else { - const child = node.child; - if (child !== null) { - insertOrAppendPlacementNodeIntoContainer(child, before, parent); - let sibling = child.sibling; - while (sibling !== null) { - insertOrAppendPlacementNodeIntoContainer(sibling, before, parent); - sibling = sibling.sibling; - } - } - } -} - -function insertOrAppendPlacementNode( - node: Fiber, - before: ?Instance, - parent: Instance, -): void { - const {tag} = node; - const isHost = tag === HostComponent || tag === HostText; - if (isHost) { - const stateNode = node.stateNode; - if (before) { - insertBefore(parent, stateNode, before); - } else { - appendChild(parent, stateNode); - } - } else if ( - tag === HostPortal || - (supportsSingletons ? tag === HostSingleton : false) - ) { - // If the insertion itself is a portal, then we don't want to traverse - // down its children. Instead, we'll get insertions from each child in - // the portal directly. - // If the insertion is a HostSingleton then it will be placed independently - } else { - const child = node.child; - if (child !== null) { - insertOrAppendPlacementNode(child, before, parent); - let sibling = child.sibling; - while (sibling !== null) { - insertOrAppendPlacementNode(sibling, before, parent); - sibling = sibling.sibling; - } - } - } -} - // These are tracked on the stack as we recursively traverse a // deleted subtree. // TODO: Update these during the whole mutation phase, not just during @@ -1726,9 +1495,9 @@ function commitSuspenseHydrationCallbacks( if (prevState !== null) { const suspenseInstance = prevState.dehydrated; if (suspenseInstance !== null) { - try { - commitHydratedSuspenseInstance(suspenseInstance); - if (enableSuspenseCallback) { + commitHostHydratedSuspense(suspenseInstance, finishedWork); + if (enableSuspenseCallback) { + try { // TODO: Delete this feature. It's not properly covered by DEV features. const hydrationCallbacks = finishedRoot.hydrationCallbacks; if (hydrationCallbacks !== null) { @@ -1737,9 +1506,9 @@ function commitSuspenseHydrationCallbacks( onHydrated(suspenseInstance); } } + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); } - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); } } } @@ -2066,17 +1835,11 @@ function commitMutationEffectsOnFiber( ); } } else if (newResource === null && finishedWork.stateNode !== null) { - try { - commitUpdate( - finishedWork.stateNode, - finishedWork.type, - current.memoizedProps, - finishedWork.memoizedProps, - finishedWork, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostUpdate( + finishedWork, + finishedWork.memoizedProps, + current.memoizedProps, + ); } } return; @@ -2120,30 +1883,20 @@ function commitMutationEffectsOnFiber( // rely on mutating the flag during commit. Like by setting a flag // during the render phase instead. if (finishedWork.flags & ContentReset) { - const instance: Instance = finishedWork.stateNode; - try { - resetTextContent(instance); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostResetTextContent(finishedWork); } if (flags & Update) { const instance: Instance = finishedWork.stateNode; if (instance != null) { // Commit the work prepared earlier. - const newProps = finishedWork.memoizedProps; // For hydration we reuse the update path but we treat the oldProps // as the newProps. The updatePayload will contain the real change in // this case. + const newProps = finishedWork.memoizedProps; const oldProps = current !== null ? current.memoizedProps : newProps; - const type = finishedWork.type; - try { - commitUpdate(instance, type, oldProps, newProps, finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostUpdate(finishedWork, newProps, oldProps); } } @@ -2176,7 +1929,6 @@ function commitMutationEffectsOnFiber( ); } - const textInstance: TextInstance = finishedWork.stateNode; const newText: string = finishedWork.memoizedProps; // For hydration we reuse the update path but we treat the oldProps // as the newProps. The updatePayload will contain the real change in @@ -2184,11 +1936,7 @@ function commitMutationEffectsOnFiber( const oldText: string = current !== null ? current.memoizedProps : newText; - try { - commitTextUpdate(textInstance, oldText, newText); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostTextUpdate(finishedWork, newText, oldText); } } return; @@ -2214,26 +1962,12 @@ function commitMutationEffectsOnFiber( if (current !== null) { const prevRootState: RootState = current.memoizedState; if (prevRootState.isDehydrated) { - try { - commitHydratedContainer(root.containerInfo); - } catch (error) { - captureCommitPhaseError( - finishedWork, - finishedWork.return, - error, - ); - } + commitHostHydratedContainer(root, finishedWork); } } } if (supportsPersistence) { - const containerInfo = root.containerInfo; - const pendingChildren = root.pendingChildren; - try { - replaceContainerChildren(containerInfo, pendingChildren); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostRootContainerChildren(root, finishedWork); } } @@ -2269,14 +2003,10 @@ function commitMutationEffectsOnFiber( if (flags & Update) { if (supportsPersistence) { - const portal = finishedWork.stateNode; - const containerInfo = portal.containerInfo; - const pendingChildren = portal.pendingChildren; - try { - replaceContainerChildren(containerInfo, pendingChildren); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostPortalContainerChildren( + finishedWork.stateNode, + finishedWork, + ); } } return; @@ -2474,11 +2204,7 @@ function commitReconciliationEffects(finishedWork: Fiber) { // before the effects on this fiber have fired. const flags = finishedWork.flags; if (flags & Placement) { - try { - commitPlacement(finishedWork); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } + commitHostPlacement(finishedWork); // Clear the "placement" from effect tag so that we know that this is // inserted, before any life-cycles like componentDidMount gets called. // TODO: findDOMNode doesn't rely on this any more but isMounted does @@ -2707,7 +2433,7 @@ export function reappearLayoutEffects( // These effects should only be committed when components are first mounted, // aka when there is no current/alternate. if (includeWorkInProgressEffects && current === null && flags & Update) { - commitHostComponentMount(finishedWork); + commitHostMount(finishedWork); } // TODO: Check flags & Ref From 7275edba7441bd2d86f3edfb94c85197c1556c91 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 17:31:16 -0400 Subject: [PATCH 6/7] Add try/catch to places used to be covered by commitDeletionEffects --- .../src/ReactFiberCommitHostEffects.js | 30 ++++++++++- .../src/ReactFiberCommitWork.js | 52 ++++++++++--------- 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index e30e7732529c0..78867cead6bc7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -45,6 +45,8 @@ import { unhideTextInstance, commitHydratedContainer, commitHydratedSuspenseInstance, + removeChildFromContainer, + removeChild, } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; @@ -334,6 +336,32 @@ export function commitHostPlacement(finishedWork: Fiber) { } } +export function commitHostRemoveChildFromContainer( + deletedFiber: Fiber, + nearestMountedAncestor: Fiber, + parentContainer: Container, + hostInstance: Instance | TextInstance, +) { + try { + removeChildFromContainer(parentContainer, hostInstance); + } catch (error) { + captureCommitPhaseError(deletedFiber, nearestMountedAncestor, error); + } +} + +export function commitHostRemoveChild( + deletedFiber: Fiber, + nearestMountedAncestor: Fiber, + parentInstance: Instance, + hostInstance: Instance | TextInstance, +) { + try { + removeChild(parentInstance, hostInstance); + } catch (error) { + captureCommitPhaseError(deletedFiber, nearestMountedAncestor, error); + } +} + export function commitHostRootContainerChildren( root: FiberRoot, finishedWork: Fiber, @@ -354,9 +382,9 @@ export function commitHostPortalContainerChildren( ... }, finishedWork: Fiber, + pendingChildren: ChildSet, ) { const containerInfo = portal.containerInfo; - const pendingChildren = portal.pendingChildren; try { replaceContainerChildren(containerInfo, pendingChildren); } catch (error) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 92f782f3922e2..3b0dc2f070328 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -12,7 +12,6 @@ import type { TextInstance, SuspenseInstance, Container, - ChildSet, HoistableRoot, FormInstance, } from './ReactFiberConfig'; @@ -114,11 +113,8 @@ import { supportsHydration, supportsResources, supportsSingletons, - removeChild, - removeChildFromContainer, clearSuspenseBoundary, clearSuspenseBoundaryFromContainer, - replaceContainerChildren, createContainerChildSet, clearContainer, prepareScopeUpdate, @@ -212,6 +208,8 @@ import { commitHostPortalContainerChildren, commitHostHydratedContainer, commitHostHydratedSuspense, + commitHostRemoveChildFromContainer, + commitHostRemoveChild, } from './ReactFiberCommitHostEffects'; // Used during the commit phase to track the state of the Offscreen component stack. @@ -1064,21 +1062,6 @@ function detachFiberAfterEffects(fiber: Fiber) { fiber.updateQueue = null; } -function emptyPortalContainer(current: Fiber) { - if (!supportsPersistence) { - return; - } - - const portal: { - containerInfo: Container, - pendingChildren: ChildSet, - ... - } = current.stateNode; - const {containerInfo} = portal; - const emptyChildSet = createContainerChildSet(); - replaceContainerChildren(containerInfo, emptyChildSet); -} - // These are tracked on the stack as we recursively traverse a // deleted subtree. // TODO: Update these during the whole mutation phase, not just during @@ -1249,12 +1232,16 @@ function commitDeletionEffectsOnFiber( // Now that all the child effects have unmounted, we can remove the // node from the tree. if (hostParentIsContainer) { - removeChildFromContainer( + commitHostRemoveChildFromContainer( + deletedFiber, + nearestMountedAncestor, ((hostParent: any): Container), (deletedFiber.stateNode: Instance | TextInstance), ); } else { - removeChild( + commitHostRemoveChild( + deletedFiber, + nearestMountedAncestor, ((hostParent: any): Instance), (deletedFiber.stateNode: Instance | TextInstance), ); @@ -1273,9 +1260,17 @@ function commitDeletionEffectsOnFiber( if (enableSuspenseCallback) { const hydrationCallbacks = finishedRoot.hydrationCallbacks; if (hydrationCallbacks !== null) { - const onDeleted = hydrationCallbacks.onDeleted; - if (onDeleted) { - onDeleted((deletedFiber.stateNode: SuspenseInstance)); + try { + const onDeleted = hydrationCallbacks.onDeleted; + if (onDeleted) { + onDeleted((deletedFiber.stateNode: SuspenseInstance)); + } + } catch (error) { + captureCommitPhaseError( + deletedFiber, + nearestMountedAncestor, + error, + ); } } } @@ -1315,7 +1310,13 @@ function commitDeletionEffectsOnFiber( hostParent = prevHostParent; hostParentIsContainer = prevHostParentIsContainer; } else { - emptyPortalContainer(deletedFiber); + if (supportsPersistence) { + commitHostPortalContainerChildren( + deletedFiber.stateNode, + deletedFiber, + createContainerChildSet(), + ); + } recursivelyTraverseDeletionEffects( finishedRoot, @@ -2006,6 +2007,7 @@ function commitMutationEffectsOnFiber( commitHostPortalContainerChildren( finishedWork.stateNode, finishedWork, + finishedWork.stateNode.pendingChildren, ); } } From 9c64bc7e8d3a034841edb0aa8ab037c1d30759f3 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 4 Sep 2024 19:16:01 -0400 Subject: [PATCH 7/7] Move and add try/catch to Host Single acquire The release is still not wrapped. --- .../src/ReactFiberCommitHostEffects.js | 15 +++++++++++++++ .../react-reconciler/src/ReactFiberCommitWork.js | 14 ++------------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js index 78867cead6bc7..99c2c1878db49 100644 --- a/packages/react-reconciler/src/ReactFiberCommitHostEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitHostEffects.js @@ -47,6 +47,8 @@ import { commitHydratedSuspenseInstance, removeChildFromContainer, removeChild, + clearSingleton, + acquireSingletonInstance, } from './ReactFiberConfig'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; @@ -413,3 +415,16 @@ export function commitHostHydratedSuspense( captureCommitPhaseError(finishedWork, finishedWork.return, error); } } + +export function commitHostSingleton(finishedWork: Fiber) { + const singleton = finishedWork.stateNode; + const props = finishedWork.memoizedProps; + + try { + // This was a new mount, we need to clear and set initial properties + clearSingleton(singleton); + acquireSingletonInstance(finishedWork.type, props, singleton, finishedWork); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } +} diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 3b0dc2f070328..c7dfb96617ed7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -121,8 +121,6 @@ import { prepareForCommit, beforeActiveInstanceBlur, detachDeletedInstance, - clearSingleton, - acquireSingletonInstance, releaseSingletonInstance, getHoistableRoot, acquireResource, @@ -210,6 +208,7 @@ import { commitHostHydratedSuspense, commitHostRemoveChildFromContainer, commitHostRemoveChild, + commitHostSingleton, } from './ReactFiberCommitHostEffects'; // Used during the commit phase to track the state of the Offscreen component stack. @@ -1852,16 +1851,7 @@ function commitMutationEffectsOnFiber( if (flags & Update) { const previousWork = finishedWork.alternate; if (previousWork === null) { - const singleton = finishedWork.stateNode; - const props = finishedWork.memoizedProps; - // This was a new mount, we need to clear and set initial properties - clearSingleton(singleton); - acquireSingletonInstance( - finishedWork.type, - props, - singleton, - finishedWork, - ); + commitHostSingleton(finishedWork); } } }