From 2eb7dcf1b656ba231ee4335cba4c39cbdda07338 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 18 Jul 2022 10:30:54 -0400 Subject: [PATCH 1/4] Change OffscreenInstance isHidden to bitmask The isHidden field of OffscreenInstance is a boolean that represents whether the tree is currently hidden. To implement resuable effects, we need to also track whether the passive effects are currently connected. So I've changed this field to a bitmask. No other behavior has changed in this commit. I'll update the effects behavior in the following steps. --- packages/react-reconciler/src/ReactFiber.new.js | 5 +++-- packages/react-reconciler/src/ReactFiber.old.js | 5 +++-- .../src/ReactFiberCommitWork.new.js | 13 ++++++------- .../src/ReactFiberCommitWork.old.js | 13 ++++++------- .../src/ReactFiberConcurrentUpdates.new.js | 6 +++++- .../src/ReactFiberConcurrentUpdates.old.js | 6 +++++- .../src/ReactFiberOffscreenComponent.js | 9 ++++++++- 7 files changed, 36 insertions(+), 21 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index b8bbb0d07ac85..560a3d0ec3321 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -61,6 +61,7 @@ import { CacheComponent, TracingMarkerComponent, } from './ReactWorkTags'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import {isDevToolsPresent} from './ReactFiberDevToolsHook.new'; @@ -717,7 +718,7 @@ export function createFiberFromOffscreen( fiber.elementType = REACT_OFFSCREEN_TYPE; fiber.lanes = lanes; const primaryChildInstance: OffscreenInstance = { - isHidden: false, + visibility: OffscreenVisible, pendingMarkers: null, retryCache: null, transitions: null, @@ -738,7 +739,7 @@ export function createFiberFromLegacyHidden( // Adding a stateNode for legacy hidden because it's currently using // the offscreen implementation, which depends on a state node const instance: OffscreenInstance = { - isHidden: false, + visibility: OffscreenVisible, pendingMarkers: null, transitions: null, retryCache: null, diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index da24de2f63ab6..0a29c4dce782f 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -61,6 +61,7 @@ import { CacheComponent, TracingMarkerComponent, } from './ReactWorkTags'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import {isDevToolsPresent} from './ReactFiberDevToolsHook.old'; @@ -717,7 +718,7 @@ export function createFiberFromOffscreen( fiber.elementType = REACT_OFFSCREEN_TYPE; fiber.lanes = lanes; const primaryChildInstance: OffscreenInstance = { - isHidden: false, + visibility: OffscreenVisible, pendingMarkers: null, retryCache: null, transitions: null, @@ -738,7 +739,7 @@ export function createFiberFromLegacyHidden( // Adding a stateNode for legacy hidden because it's currently using // the offscreen implementation, which depends on a state node const instance: OffscreenInstance = { - isHidden: false, + visibility: OffscreenVisible, pendingMarkers: null, transitions: null, retryCache: null, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 200e661c7017a..7d0aad4c2af8f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -173,6 +173,7 @@ import { } from './ReactFiberDevToolsHook.new'; import {releaseCache, retainCache} from './ReactFiberCacheComponent.new'; import {clearTransitionsForLanes} from './ReactFiberLane.new'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -2424,14 +2425,8 @@ function commitMutationEffectsOnFiber( const offscreenFiber: Fiber = (finishedWork.child: any); if (offscreenFiber.flags & Visibility) { - const offscreenInstance: OffscreenInstance = offscreenFiber.stateNode; const newState: OffscreenState | null = offscreenFiber.memoizedState; const isHidden = newState !== null; - - // Track the current state on the Offscreen instance so we can - // read it during an event - offscreenInstance.isHidden = isHidden; - if (isHidden) { const wasHidden = offscreenFiber.alternate !== null && @@ -2485,7 +2480,11 @@ function commitMutationEffectsOnFiber( // Track the current state on the Offscreen instance so we can // read it during an event - offscreenInstance.isHidden = isHidden; + if (isHidden) { + offscreenInstance.visibility &= ~OffscreenVisible; + } else { + offscreenInstance.visibility |= OffscreenVisible; + } if (isHidden) { if (!wasHidden) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 4b979151da999..5e287090664b9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -173,6 +173,7 @@ import { } from './ReactFiberDevToolsHook.old'; import {releaseCache, retainCache} from './ReactFiberCacheComponent.old'; import {clearTransitionsForLanes} from './ReactFiberLane.old'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -2424,14 +2425,8 @@ function commitMutationEffectsOnFiber( const offscreenFiber: Fiber = (finishedWork.child: any); if (offscreenFiber.flags & Visibility) { - const offscreenInstance: OffscreenInstance = offscreenFiber.stateNode; const newState: OffscreenState | null = offscreenFiber.memoizedState; const isHidden = newState !== null; - - // Track the current state on the Offscreen instance so we can - // read it during an event - offscreenInstance.isHidden = isHidden; - if (isHidden) { const wasHidden = offscreenFiber.alternate !== null && @@ -2485,7 +2480,11 @@ function commitMutationEffectsOnFiber( // Track the current state on the Offscreen instance so we can // read it during an event - offscreenInstance.isHidden = isHidden; + if (isHidden) { + offscreenInstance.visibility &= ~OffscreenVisible; + } else { + offscreenInstance.visibility |= OffscreenVisible; + } if (isHidden) { if (!wasHidden) { diff --git a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js index 7a29e8166b7f3..e496389782025 100644 --- a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js +++ b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js @@ -31,6 +31,7 @@ import { } from './ReactFiberLane.new'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; import {HostRoot, OffscreenComponent} from './ReactWorkTags'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; export type ConcurrentUpdate = { next: ConcurrentUpdate, @@ -217,7 +218,10 @@ function markUpdateLaneFromFiberToRoot( // account for it. (There may be other cases that we haven't discovered, // too.) const offscreenInstance: OffscreenInstance | null = parent.stateNode; - if (offscreenInstance !== null && offscreenInstance.isHidden) { + if ( + offscreenInstance !== null && + !(offscreenInstance.visibility & OffscreenVisible) + ) { isHidden = true; } } diff --git a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js index 0d2cbb44c96a1..7dd87358cf6a1 100644 --- a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js +++ b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js @@ -31,6 +31,7 @@ import { } from './ReactFiberLane.old'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; import {HostRoot, OffscreenComponent} from './ReactWorkTags'; +import {OffscreenVisible} from './ReactFiberOffscreenComponent'; export type ConcurrentUpdate = { next: ConcurrentUpdate, @@ -217,7 +218,10 @@ function markUpdateLaneFromFiberToRoot( // account for it. (There may be other cases that we haven't discovered, // too.) const offscreenInstance: OffscreenInstance | null = parent.stateNode; - if (offscreenInstance !== null && offscreenInstance.isHidden) { + if ( + offscreenInstance !== null && + !(offscreenInstance.visibility & OffscreenVisible) + ) { isHidden = true; } } diff --git a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js index ca1a3938410f7..7655387ced19c 100644 --- a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js +++ b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js @@ -42,8 +42,15 @@ export type OffscreenQueue = {| wakeables: Set | null, |}; +type OffscreenVisibility = number; + +export const OffscreenVisible = /* */ 0b01; + +// TODO +// export const OffscreenPassiveEffectsConnected = /* */ 0b01; + export type OffscreenInstance = {| - isHidden: boolean, + visibility: OffscreenVisibility, pendingMarkers: Set | null, transitions: Set | null, retryCache: WeakSet | Set | null, From 6078e6a016c02e989f90896956931365cf68d1cc Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 18 Jul 2022 10:54:23 -0400 Subject: [PATCH 2/4] Extract passive mount effects to separate functions I'm about to add a "reappear passive effects" function that will share much of the same code as commitPassiveMountEffectOnFiber. To minimize the duplicated code, I've extracted the shared parts into separate functions, similar to what I did for commitLayoutEffectOnFiber and reappearLayoutEffects. This may not save much on code size because Closure will likely inline some of it, anyway, but it makes it harder for the two paths to accidentally diverge. --- .../src/ReactFiberCommitWork.new.js | 314 ++++++++++-------- .../src/ReactFiberCommitWork.old.js | 314 ++++++++++-------- 2 files changed, 344 insertions(+), 284 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 7d0aad4c2af8f..9fe7e331fa517 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -2870,6 +2870,167 @@ function recursivelyTraverseReappearLayoutEffects( setCurrentDebugFiberInDEV(prevDebugFiber); } +function commitHookPassiveMountEffects( + finishedWork: Fiber, + hookFlags: HookFlags, +) { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + finishedWork.mode & ProfileMode + ) { + 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, +) { + if (enableCache) { + let previousCache: Cache | null = null; + if ( + current !== null && + current.memoizedState !== null && + current.memoizedState.cachePool !== null + ) { + previousCache = current.memoizedState.cachePool.pool; + } + let nextCache: Cache | null = null; + if ( + finishedWork.memoizedState !== null && + finishedWork.memoizedState.cachePool !== null + ) { + nextCache = finishedWork.memoizedState.cachePool.pool; + } + // Retain/release the cache used for pending (suspended) nodes. + // Note that this is only reached in the non-suspended/visible case: + // when the content is suspended/hidden, the retain/release occurs + // via the parent Suspense component (see case above). + if (nextCache !== previousCache) { + if (nextCache != null) { + retainCache(nextCache); + } + if (previousCache != null) { + releaseCache(previousCache); + } + } + } + + if (enableTransitionTracing) { + // TODO: Pre-rendering should not be counted as part of a transition. We + // may add separate logs for pre-rendering, but it's not part of the + // primary metrics. + const offscreenState: OffscreenState = finishedWork.memoizedState; + const queue: OffscreenQueue | null = (finishedWork.updateQueue: any); + const instance: OffscreenInstance = finishedWork.stateNode; + + const isHidden = offscreenState !== null; + if (queue !== null) { + if (isHidden) { + const transitions = queue.transitions; + if (transitions !== null) { + transitions.forEach(transition => { + // Add all the transitions saved in the update queue during + // the render phase (ie the transitions associated with this boundary) + // into the transitions set. + if (instance.transitions === null) { + instance.transitions = new Set(); + } + instance.transitions.add(transition); + }); + } + + const markerInstances = queue.markerInstances; + if (markerInstances !== null) { + markerInstances.forEach(markerInstance => { + const markerTransitions = markerInstance.transitions; + // There should only be a few tracing marker transitions because + // they should be only associated with the transition that + // caused them + if (markerTransitions !== null) { + markerTransitions.forEach(transition => { + if (instance.transitions === null) { + instance.transitions = new Set(); + } else if (instance.transitions.has(transition)) { + if (markerInstance.pendingBoundaries === null) { + markerInstance.pendingBoundaries = new Map(); + } + if (instance.pendingMarkers === null) { + instance.pendingMarkers = new Set(); + } + + instance.pendingMarkers.add(markerInstance); + } + }); + } + }); + } + } + + finishedWork.updateQueue = null; + } + + commitTransitionProgress(finishedWork); + } +} + +function commitCachePassiveMountEffect( + current: Fiber | null, + finishedWork: Fiber, +) { + if (enableCache) { + let previousCache: Cache | null = null; + if (finishedWork.alternate !== null) { + previousCache = finishedWork.alternate.memoizedState.cache; + } + const nextCache = finishedWork.memoizedState.cache; + // Retain/release the cache. In theory the cache component + // could be "borrowing" a cache instance owned by some parent, + // in which case we could avoid retaining/releasing. But it + // is non-trivial to determine when that is the case, so we + // always retain/release. + if (nextCache !== previousCache) { + retainCache(nextCache); + if (previousCache != null) { + releaseCache(previousCache); + } + } + } +} + +function commitTracingMarkerPassiveMountEffect(finishedWork: Fiber) { + // Get the transitions that were initiatized during the render + // and add a start transition callback for each of them + const instance = finishedWork.stateNode; + if ( + instance.transitions !== null && + (instance.pendingBoundaries === null || + instance.pendingBoundaries.size === 0) + ) { + instance.transitions.forEach(transition => { + addMarkerCompleteCallbackToPendingTransition( + finishedWork.memoizedProps.name, + instance.transitions, + ); + }); + instance.transitions = null; + instance.pendingBoundaries = null; + } +} + export function commitPassiveMountEffects( root: FiberRoot, finishedWork: Fiber, @@ -2927,31 +3088,10 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { - startPassiveEffectTimer(); - try { - commitHookEffectListMount( - HookPassive | HookHasEffect, - finishedWork, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - recordPassiveEffectDuration(finishedWork); - } else { - try { - commitHookEffectListMount( - HookPassive | HookHasEffect, - finishedWork, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } + commitHookPassiveMountEffects( + finishedWork, + HookPassive | HookHasEffect, + ); } break; } @@ -3019,88 +3159,9 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - if (enableCache) { - let previousCache: Cache | null = null; - if ( - finishedWork.alternate !== null && - finishedWork.alternate.memoizedState !== null && - finishedWork.alternate.memoizedState.cachePool !== null - ) { - previousCache = finishedWork.alternate.memoizedState.cachePool.pool; - } - let nextCache: Cache | null = null; - if ( - finishedWork.memoizedState !== null && - finishedWork.memoizedState.cachePool !== null - ) { - nextCache = finishedWork.memoizedState.cachePool.pool; - } - // Retain/release the cache used for pending (suspended) nodes. - // Note that this is only reached in the non-suspended/visible case: - // when the content is suspended/hidden, the retain/release occurs - // via the parent Suspense component (see case above). - if (nextCache !== previousCache) { - if (nextCache != null) { - retainCache(nextCache); - } - if (previousCache != null) { - releaseCache(previousCache); - } - } - } - - if (enableTransitionTracing) { - const isFallback = finishedWork.memoizedState; - const queue: OffscreenQueue | null = (finishedWork.updateQueue: any); - const instance: OffscreenInstance = finishedWork.stateNode; - - if (queue !== null) { - if (isFallback) { - const transitions = queue.transitions; - if (transitions !== null) { - transitions.forEach(transition => { - // Add all the transitions saved in the update queue during - // the render phase (ie the transitions associated with this boundary) - // into the transitions set. - if (instance.transitions === null) { - instance.transitions = new Set(); - } - instance.transitions.add(transition); - }); - } - - const markerInstances = queue.markerInstances; - if (markerInstances !== null) { - markerInstances.forEach(markerInstance => { - const markerTransitions = markerInstance.transitions; - // There should only be a few tracing marker transitions because - // they should be only associated with the transition that - // caused them - if (markerTransitions !== null) { - markerTransitions.forEach(transition => { - if (instance.transitions === null) { - instance.transitions = new Set(); - } else if (instance.transitions.has(transition)) { - if (markerInstance.pendingBoundaries === null) { - markerInstance.pendingBoundaries = new Map(); - } - if (instance.pendingMarkers === null) { - instance.pendingMarkers = new Set(); - } - - instance.pendingMarkers.add(markerInstance); - } - }); - } - }); - } - } - - finishedWork.updateQueue = null; - } - - commitTransitionProgress(finishedWork); - } + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + commitOffscreenPassiveMountEffects(current, finishedWork); } break; } @@ -3112,24 +3173,9 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - if (enableCache) { - let previousCache: Cache | null = null; - if (finishedWork.alternate !== null) { - previousCache = finishedWork.alternate.memoizedState.cache; - } - const nextCache = finishedWork.memoizedState.cache; - // Retain/release the cache. In theory the cache component - // could be "borrowing" a cache instance owned by some parent, - // in which case we could avoid retaining/releasing. But it - // is non-trivial to determine when that is the case, so we - // always retain/release. - if (nextCache !== previousCache) { - retainCache(nextCache); - if (previousCache != null) { - releaseCache(previousCache); - } - } - } + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + commitCachePassiveMountEffect(current, finishedWork); } break; } @@ -3142,23 +3188,7 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - // Get the transitions that were initiatized during the render - // and add a start transition callback for each of them - const instance = finishedWork.stateNode; - if ( - instance.transitions !== null && - (instance.pendingBoundaries === null || - instance.pendingBoundaries.size === 0) - ) { - instance.transitions.forEach(transition => { - addMarkerCompleteCallbackToPendingTransition( - finishedWork.memoizedProps.name, - instance.transitions, - ); - }); - instance.transitions = null; - instance.pendingBoundaries = null; - } + commitTracingMarkerPassiveMountEffect(finishedWork); } break; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 5e287090664b9..abb4cdd055c43 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -2870,6 +2870,167 @@ function recursivelyTraverseReappearLayoutEffects( setCurrentDebugFiberInDEV(prevDebugFiber); } +function commitHookPassiveMountEffects( + finishedWork: Fiber, + hookFlags: HookFlags, +) { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + finishedWork.mode & ProfileMode + ) { + 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, +) { + if (enableCache) { + let previousCache: Cache | null = null; + if ( + current !== null && + current.memoizedState !== null && + current.memoizedState.cachePool !== null + ) { + previousCache = current.memoizedState.cachePool.pool; + } + let nextCache: Cache | null = null; + if ( + finishedWork.memoizedState !== null && + finishedWork.memoizedState.cachePool !== null + ) { + nextCache = finishedWork.memoizedState.cachePool.pool; + } + // Retain/release the cache used for pending (suspended) nodes. + // Note that this is only reached in the non-suspended/visible case: + // when the content is suspended/hidden, the retain/release occurs + // via the parent Suspense component (see case above). + if (nextCache !== previousCache) { + if (nextCache != null) { + retainCache(nextCache); + } + if (previousCache != null) { + releaseCache(previousCache); + } + } + } + + if (enableTransitionTracing) { + // TODO: Pre-rendering should not be counted as part of a transition. We + // may add separate logs for pre-rendering, but it's not part of the + // primary metrics. + const offscreenState: OffscreenState = finishedWork.memoizedState; + const queue: OffscreenQueue | null = (finishedWork.updateQueue: any); + const instance: OffscreenInstance = finishedWork.stateNode; + + const isHidden = offscreenState !== null; + if (queue !== null) { + if (isHidden) { + const transitions = queue.transitions; + if (transitions !== null) { + transitions.forEach(transition => { + // Add all the transitions saved in the update queue during + // the render phase (ie the transitions associated with this boundary) + // into the transitions set. + if (instance.transitions === null) { + instance.transitions = new Set(); + } + instance.transitions.add(transition); + }); + } + + const markerInstances = queue.markerInstances; + if (markerInstances !== null) { + markerInstances.forEach(markerInstance => { + const markerTransitions = markerInstance.transitions; + // There should only be a few tracing marker transitions because + // they should be only associated with the transition that + // caused them + if (markerTransitions !== null) { + markerTransitions.forEach(transition => { + if (instance.transitions === null) { + instance.transitions = new Set(); + } else if (instance.transitions.has(transition)) { + if (markerInstance.pendingBoundaries === null) { + markerInstance.pendingBoundaries = new Map(); + } + if (instance.pendingMarkers === null) { + instance.pendingMarkers = new Set(); + } + + instance.pendingMarkers.add(markerInstance); + } + }); + } + }); + } + } + + finishedWork.updateQueue = null; + } + + commitTransitionProgress(finishedWork); + } +} + +function commitCachePassiveMountEffect( + current: Fiber | null, + finishedWork: Fiber, +) { + if (enableCache) { + let previousCache: Cache | null = null; + if (finishedWork.alternate !== null) { + previousCache = finishedWork.alternate.memoizedState.cache; + } + const nextCache = finishedWork.memoizedState.cache; + // Retain/release the cache. In theory the cache component + // could be "borrowing" a cache instance owned by some parent, + // in which case we could avoid retaining/releasing. But it + // is non-trivial to determine when that is the case, so we + // always retain/release. + if (nextCache !== previousCache) { + retainCache(nextCache); + if (previousCache != null) { + releaseCache(previousCache); + } + } + } +} + +function commitTracingMarkerPassiveMountEffect(finishedWork: Fiber) { + // Get the transitions that were initiatized during the render + // and add a start transition callback for each of them + const instance = finishedWork.stateNode; + if ( + instance.transitions !== null && + (instance.pendingBoundaries === null || + instance.pendingBoundaries.size === 0) + ) { + instance.transitions.forEach(transition => { + addMarkerCompleteCallbackToPendingTransition( + finishedWork.memoizedProps.name, + instance.transitions, + ); + }); + instance.transitions = null; + instance.pendingBoundaries = null; + } +} + export function commitPassiveMountEffects( root: FiberRoot, finishedWork: Fiber, @@ -2927,31 +3088,10 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { - startPassiveEffectTimer(); - try { - commitHookEffectListMount( - HookPassive | HookHasEffect, - finishedWork, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - recordPassiveEffectDuration(finishedWork); - } else { - try { - commitHookEffectListMount( - HookPassive | HookHasEffect, - finishedWork, - ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); - } - } + commitHookPassiveMountEffects( + finishedWork, + HookPassive | HookHasEffect, + ); } break; } @@ -3019,88 +3159,9 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - if (enableCache) { - let previousCache: Cache | null = null; - if ( - finishedWork.alternate !== null && - finishedWork.alternate.memoizedState !== null && - finishedWork.alternate.memoizedState.cachePool !== null - ) { - previousCache = finishedWork.alternate.memoizedState.cachePool.pool; - } - let nextCache: Cache | null = null; - if ( - finishedWork.memoizedState !== null && - finishedWork.memoizedState.cachePool !== null - ) { - nextCache = finishedWork.memoizedState.cachePool.pool; - } - // Retain/release the cache used for pending (suspended) nodes. - // Note that this is only reached in the non-suspended/visible case: - // when the content is suspended/hidden, the retain/release occurs - // via the parent Suspense component (see case above). - if (nextCache !== previousCache) { - if (nextCache != null) { - retainCache(nextCache); - } - if (previousCache != null) { - releaseCache(previousCache); - } - } - } - - if (enableTransitionTracing) { - const isFallback = finishedWork.memoizedState; - const queue: OffscreenQueue | null = (finishedWork.updateQueue: any); - const instance: OffscreenInstance = finishedWork.stateNode; - - if (queue !== null) { - if (isFallback) { - const transitions = queue.transitions; - if (transitions !== null) { - transitions.forEach(transition => { - // Add all the transitions saved in the update queue during - // the render phase (ie the transitions associated with this boundary) - // into the transitions set. - if (instance.transitions === null) { - instance.transitions = new Set(); - } - instance.transitions.add(transition); - }); - } - - const markerInstances = queue.markerInstances; - if (markerInstances !== null) { - markerInstances.forEach(markerInstance => { - const markerTransitions = markerInstance.transitions; - // There should only be a few tracing marker transitions because - // they should be only associated with the transition that - // caused them - if (markerTransitions !== null) { - markerTransitions.forEach(transition => { - if (instance.transitions === null) { - instance.transitions = new Set(); - } else if (instance.transitions.has(transition)) { - if (markerInstance.pendingBoundaries === null) { - markerInstance.pendingBoundaries = new Map(); - } - if (instance.pendingMarkers === null) { - instance.pendingMarkers = new Set(); - } - - instance.pendingMarkers.add(markerInstance); - } - }); - } - }); - } - } - - finishedWork.updateQueue = null; - } - - commitTransitionProgress(finishedWork); - } + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + commitOffscreenPassiveMountEffects(current, finishedWork); } break; } @@ -3112,24 +3173,9 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - if (enableCache) { - let previousCache: Cache | null = null; - if (finishedWork.alternate !== null) { - previousCache = finishedWork.alternate.memoizedState.cache; - } - const nextCache = finishedWork.memoizedState.cache; - // Retain/release the cache. In theory the cache component - // could be "borrowing" a cache instance owned by some parent, - // in which case we could avoid retaining/releasing. But it - // is non-trivial to determine when that is the case, so we - // always retain/release. - if (nextCache !== previousCache) { - retainCache(nextCache); - if (previousCache != null) { - releaseCache(previousCache); - } - } - } + // TODO: Pass `current` as argument to this function + const current = finishedWork.alternate; + commitCachePassiveMountEffect(current, finishedWork); } break; } @@ -3142,23 +3188,7 @@ function commitPassiveMountOnFiber( committedTransitions, ); if (flags & Passive) { - // Get the transitions that were initiatized during the render - // and add a start transition callback for each of them - const instance = finishedWork.stateNode; - if ( - instance.transitions !== null && - (instance.pendingBoundaries === null || - instance.pendingBoundaries.size === 0) - ) { - instance.transitions.forEach(transition => { - addMarkerCompleteCallbackToPendingTransition( - finishedWork.memoizedProps.name, - instance.transitions, - ); - }); - instance.transitions = null; - instance.pendingBoundaries = null; - } + commitTracingMarkerPassiveMountEffect(finishedWork); } break; } From 7f00ca30ec80aed9bf10bf1f304f09c834c456c1 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 20 Jul 2022 14:55:55 -0400 Subject: [PATCH 3/4] Don't mount passive effects in a new hidden tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes the behavior of Offscreen so that passive effects do not fire when prerendering a brand new tree. Previously, Offscreen did not affect passive effects at all — only layout effects, which mount or unmount whenever the visibility of the tree changes. When hiding an already visible tree, the behavior of passive effects is unchanged, for now; unlike layout effects, the passive effects will not get unmounted. Pre-rendered updates to a hidden tree in this state will also fire normally. This is only temporary, though — the plan is for passive effects to act more like layout effects, and unmount them when the tree is hidden. Perhaps after a delay so that if the visibility toggles quickly back and forth, the effects don't need to remount. I'll implement this separately. --- .../src/ReactFiberCommitWork.new.js | 268 +++++++++++++++++- .../src/ReactFiberCommitWork.old.js | 268 +++++++++++++++++- .../react-reconciler/src/ReactFiberFlags.js | 2 +- .../src/ReactFiberOffscreenComponent.js | 4 +- .../src/__tests__/ReactOffscreen-test.js | 177 ++++++++++-- 5 files changed, 675 insertions(+), 44 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 9fe7e331fa517..5bb01e1807836 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -173,7 +173,10 @@ import { } from './ReactFiberDevToolsHook.new'; import {releaseCache, retainCache} from './ReactFiberCacheComponent.new'; import {clearTransitionsForLanes} from './ReactFiberLane.new'; -import {OffscreenVisible} from './ReactFiberOffscreenComponent'; +import { + OffscreenVisible, + OffscreenPassiveEffectsConnected, +} from './ReactFiberOffscreenComponent'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -2898,6 +2901,7 @@ function commitHookPassiveMountEffects( function commitOffscreenPassiveMountEffects( current: Fiber | null, finishedWork: Fiber, + instance: OffscreenInstance, ) { if (enableCache) { let previousCache: Cache | null = null; @@ -2935,7 +2939,6 @@ function commitOffscreenPassiveMountEffects( // primary metrics. const offscreenState: OffscreenState = finishedWork.memoizedState; const queue: OffscreenQueue | null = (finishedWork.updateQueue: any); - const instance: OffscreenInstance = finishedWork.stateNode; const isHidden = offscreenState !== null; if (queue !== null) { @@ -3076,6 +3079,9 @@ function commitPassiveMountOnFiber( committedLanes: Lanes, committedTransitions: Array | null, ): void { + // When updating this function, also update reconnectPassiveEffects, which does + // most of the same things when an offscreen tree goes from hidden -> visible, + // or when toggling effects inside a hidden tree. const flags = finishedWork.flags; switch (finishedWork.tag) { case FunctionComponent: @@ -3152,6 +3158,83 @@ function commitPassiveMountOnFiber( } case LegacyHiddenComponent: case OffscreenComponent: { + // TODO: Pass `current` as argument to this function + const instance: OffscreenInstance = finishedWork.stateNode; + const nextState: OffscreenState | null = finishedWork.memoizedState; + + const isHidden = nextState !== null; + + if (isHidden) { + if (instance.visibility & OffscreenPassiveEffectsConnected) { + // The effects are currently connected. Update them. + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + } else { + if (finishedWork.mode & ConcurrentMode) { + // The effects are currently disconnected. Since the tree is hidden, + // don't connect them. This also applies to the initial render. + if (enableCache || enableTransitionTracing) { + // "Atomic" effects are ones that need to fire on every commit, + // even during pre-rendering. An example is updating the reference + // count on cache instances. + // TODO: Not yet implemented + // recursivelyTraverseAtomicPassiveEffects( + // finishedRoot, + // finishedWork, + // committedLanes, + // committedTransitions, + // ); + } + } else { + // Legacy Mode: Fire the effects even if the tree is hidden. + instance.visibility |= OffscreenPassiveEffectsConnected; + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + } + } + } else { + // Tree is visible + if (instance.visibility & OffscreenPassiveEffectsConnected) { + // The effects are currently connected. Update them. + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + } else { + // The effects are currently disconnected. Reconnect them, while also + // firing effects inside newly mounted trees. This also applies to + // the initial render. + instance.visibility |= OffscreenPassiveEffectsConnected; + + const includeWorkInProgressEffects = + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags; + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } + } + + if (flags & Passive) { + const current = finishedWork.alternate; + commitOffscreenPassiveMountEffects(current, finishedWork, instance); + } + break; + } + case CacheComponent: { recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, @@ -3161,18 +3244,180 @@ function commitPassiveMountOnFiber( if (flags & Passive) { // TODO: Pass `current` as argument to this function const current = finishedWork.alternate; - commitOffscreenPassiveMountEffects(current, finishedWork); + commitCachePassiveMountEffect(current, finishedWork); } break; } - case CacheComponent: { + case TracingMarkerComponent: { + if (enableTransitionTracing) { + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + commitTracingMarkerPassiveMountEffect(finishedWork); + } + break; + } + // Intentional fallthrough to next branch + } + // eslint-disable-next-line-no-fallthrough + default: { recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, committedLanes, committedTransitions, ); - if (flags & Passive) { + break; + } + } +} + +function recursivelyTraverseReconnectPassiveEffects( + finishedRoot: FiberRoot, + parentFiber: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, + includeWorkInProgressEffects: boolean, +) { + // This function visits both newly finished work and nodes that were re-used + // from a previously committed tree. We cannot check non-static flags if the + // node was reused. + const childShouldIncludeWorkInProgressEffects = + includeWorkInProgressEffects && + (parentFiber.subtreeFlags & PassiveMask) !== NoFlags; + + // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic) + const prevDebugFiber = getCurrentDebugFiberInDEV(); + let child = parentFiber.child; + while (child !== null) { + reconnectPassiveEffects( + finishedRoot, + child, + committedLanes, + committedTransitions, + childShouldIncludeWorkInProgressEffects, + ); + child = child.sibling; + } + setCurrentDebugFiberInDEV(prevDebugFiber); +} + +function reconnectPassiveEffects( + finishedRoot: FiberRoot, + finishedWork: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, + // This function visits both newly finished work and nodes that were re-used + // from a previously committed tree. We cannot check non-static flags if the + // node was reused. + includeWorkInProgressEffects: boolean, +) { + const flags = finishedWork.flags; + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + // TODO: Check for PassiveStatic flag + commitHookPassiveMountEffects(finishedWork, HookPassive); + break; + } + // Unlike commitPassiveMountOnFiber, we don't need to handle HostRoot + // because this function only visits nodes that are inside an + // Offscreen fiber. + // case HostRoot: { + // ... + // } + case LegacyHiddenComponent: + case OffscreenComponent: { + const instance: OffscreenInstance = finishedWork.stateNode; + const nextState: OffscreenState | null = finishedWork.memoizedState; + + const isHidden = nextState !== null; + + if (isHidden) { + if (instance.visibility & OffscreenPassiveEffectsConnected) { + // The effects are currently connected. Update them. + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } else { + if (finishedWork.mode & ConcurrentMode) { + // The effects are currently disconnected. Since the tree is hidden, + // don't connect them. This also applies to the initial render. + if (enableCache || enableTransitionTracing) { + // "Atomic" effects are ones that need to fire on every commit, + // even during pre-rendering. An example is updating the reference + // count on cache instances. + // TODO: Not yet implemented + // recursivelyTraverseAtomicPassiveEffects( + // finishedRoot, + // finishedWork, + // committedLanes, + // committedTransitions, + // ); + } + } else { + // Legacy Mode: Fire the effects even if the tree is hidden. + instance.visibility |= OffscreenPassiveEffectsConnected; + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } + } + } else { + // Tree is visible + + // Since we're already inside a reconnecting tree, it doesn't matter + // whether the effects are currently connected. In either case, we'll + // continue traversing the tree and firing all the effects. + // + // We do need to set the "connected" flag on the instance, though. + instance.visibility |= OffscreenPassiveEffectsConnected; + + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } + + if (includeWorkInProgressEffects && flags & Passive) { + // TODO: Pass `current` as argument to this function + const current: Fiber | null = finishedWork.alternate; + commitOffscreenPassiveMountEffects(current, finishedWork, instance); + } + break; + } + case CacheComponent: { + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + if (includeWorkInProgressEffects && flags & Passive) { // TODO: Pass `current` as argument to this function const current = finishedWork.alternate; commitCachePassiveMountEffect(current, finishedWork); @@ -3181,13 +3426,14 @@ function commitPassiveMountOnFiber( } case TracingMarkerComponent: { if (enableTransitionTracing) { - recursivelyTraversePassiveMountEffects( + recursivelyTraverseReconnectPassiveEffects( finishedRoot, finishedWork, committedLanes, committedTransitions, + includeWorkInProgressEffects, ); - if (flags & Passive) { + if (includeWorkInProgressEffects && flags & Passive) { commitTracingMarkerPassiveMountEffect(finishedWork); } break; @@ -3196,11 +3442,12 @@ function commitPassiveMountOnFiber( } // eslint-disable-next-line-no-fallthrough default: { - recursivelyTraversePassiveMountEffects( + recursivelyTraverseReconnectPassiveEffects( finishedRoot, finishedWork, committedLanes, committedTransitions, + includeWorkInProgressEffects, ); break; } @@ -3304,6 +3551,11 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { } break; } + // TODO: Disconnect passive effects when a tree is hidden, perhaps after + // a delay. + // case OffscreenComponent: { + // ... + // } default: { recursivelyTraversePassiveUnmountEffects(finishedWork); break; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index abb4cdd055c43..97cb9c2808e32 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -173,7 +173,10 @@ import { } from './ReactFiberDevToolsHook.old'; import {releaseCache, retainCache} from './ReactFiberCacheComponent.old'; import {clearTransitionsForLanes} from './ReactFiberLane.old'; -import {OffscreenVisible} from './ReactFiberOffscreenComponent'; +import { + OffscreenVisible, + OffscreenPassiveEffectsConnected, +} from './ReactFiberOffscreenComponent'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -2898,6 +2901,7 @@ function commitHookPassiveMountEffects( function commitOffscreenPassiveMountEffects( current: Fiber | null, finishedWork: Fiber, + instance: OffscreenInstance, ) { if (enableCache) { let previousCache: Cache | null = null; @@ -2935,7 +2939,6 @@ function commitOffscreenPassiveMountEffects( // primary metrics. const offscreenState: OffscreenState = finishedWork.memoizedState; const queue: OffscreenQueue | null = (finishedWork.updateQueue: any); - const instance: OffscreenInstance = finishedWork.stateNode; const isHidden = offscreenState !== null; if (queue !== null) { @@ -3076,6 +3079,9 @@ function commitPassiveMountOnFiber( committedLanes: Lanes, committedTransitions: Array | null, ): void { + // When updating this function, also update reconnectPassiveEffects, which does + // most of the same things when an offscreen tree goes from hidden -> visible, + // or when toggling effects inside a hidden tree. const flags = finishedWork.flags; switch (finishedWork.tag) { case FunctionComponent: @@ -3152,6 +3158,83 @@ function commitPassiveMountOnFiber( } case LegacyHiddenComponent: case OffscreenComponent: { + // TODO: Pass `current` as argument to this function + const instance: OffscreenInstance = finishedWork.stateNode; + const nextState: OffscreenState | null = finishedWork.memoizedState; + + const isHidden = nextState !== null; + + if (isHidden) { + if (instance.visibility & OffscreenPassiveEffectsConnected) { + // The effects are currently connected. Update them. + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + } else { + if (finishedWork.mode & ConcurrentMode) { + // The effects are currently disconnected. Since the tree is hidden, + // don't connect them. This also applies to the initial render. + if (enableCache || enableTransitionTracing) { + // "Atomic" effects are ones that need to fire on every commit, + // even during pre-rendering. An example is updating the reference + // count on cache instances. + // TODO: Not yet implemented + // recursivelyTraverseAtomicPassiveEffects( + // finishedRoot, + // finishedWork, + // committedLanes, + // committedTransitions, + // ); + } + } else { + // Legacy Mode: Fire the effects even if the tree is hidden. + instance.visibility |= OffscreenPassiveEffectsConnected; + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + } + } + } else { + // Tree is visible + if (instance.visibility & OffscreenPassiveEffectsConnected) { + // The effects are currently connected. Update them. + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + } else { + // The effects are currently disconnected. Reconnect them, while also + // firing effects inside newly mounted trees. This also applies to + // the initial render. + instance.visibility |= OffscreenPassiveEffectsConnected; + + const includeWorkInProgressEffects = + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags; + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } + } + + if (flags & Passive) { + const current = finishedWork.alternate; + commitOffscreenPassiveMountEffects(current, finishedWork, instance); + } + break; + } + case CacheComponent: { recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, @@ -3161,18 +3244,180 @@ function commitPassiveMountOnFiber( if (flags & Passive) { // TODO: Pass `current` as argument to this function const current = finishedWork.alternate; - commitOffscreenPassiveMountEffects(current, finishedWork); + commitCachePassiveMountEffect(current, finishedWork); } break; } - case CacheComponent: { + case TracingMarkerComponent: { + if (enableTransitionTracing) { + recursivelyTraversePassiveMountEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + ); + if (flags & Passive) { + commitTracingMarkerPassiveMountEffect(finishedWork); + } + break; + } + // Intentional fallthrough to next branch + } + // eslint-disable-next-line-no-fallthrough + default: { recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, committedLanes, committedTransitions, ); - if (flags & Passive) { + break; + } + } +} + +function recursivelyTraverseReconnectPassiveEffects( + finishedRoot: FiberRoot, + parentFiber: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, + includeWorkInProgressEffects: boolean, +) { + // This function visits both newly finished work and nodes that were re-used + // from a previously committed tree. We cannot check non-static flags if the + // node was reused. + const childShouldIncludeWorkInProgressEffects = + includeWorkInProgressEffects && + (parentFiber.subtreeFlags & PassiveMask) !== NoFlags; + + // TODO (Offscreen) Check: flags & (RefStatic | LayoutStatic) + const prevDebugFiber = getCurrentDebugFiberInDEV(); + let child = parentFiber.child; + while (child !== null) { + reconnectPassiveEffects( + finishedRoot, + child, + committedLanes, + committedTransitions, + childShouldIncludeWorkInProgressEffects, + ); + child = child.sibling; + } + setCurrentDebugFiberInDEV(prevDebugFiber); +} + +function reconnectPassiveEffects( + finishedRoot: FiberRoot, + finishedWork: Fiber, + committedLanes: Lanes, + committedTransitions: Array | null, + // This function visits both newly finished work and nodes that were re-used + // from a previously committed tree. We cannot check non-static flags if the + // node was reused. + includeWorkInProgressEffects: boolean, +) { + const flags = finishedWork.flags; + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + // TODO: Check for PassiveStatic flag + commitHookPassiveMountEffects(finishedWork, HookPassive); + break; + } + // Unlike commitPassiveMountOnFiber, we don't need to handle HostRoot + // because this function only visits nodes that are inside an + // Offscreen fiber. + // case HostRoot: { + // ... + // } + case LegacyHiddenComponent: + case OffscreenComponent: { + const instance: OffscreenInstance = finishedWork.stateNode; + const nextState: OffscreenState | null = finishedWork.memoizedState; + + const isHidden = nextState !== null; + + if (isHidden) { + if (instance.visibility & OffscreenPassiveEffectsConnected) { + // The effects are currently connected. Update them. + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } else { + if (finishedWork.mode & ConcurrentMode) { + // The effects are currently disconnected. Since the tree is hidden, + // don't connect them. This also applies to the initial render. + if (enableCache || enableTransitionTracing) { + // "Atomic" effects are ones that need to fire on every commit, + // even during pre-rendering. An example is updating the reference + // count on cache instances. + // TODO: Not yet implemented + // recursivelyTraverseAtomicPassiveEffects( + // finishedRoot, + // finishedWork, + // committedLanes, + // committedTransitions, + // ); + } + } else { + // Legacy Mode: Fire the effects even if the tree is hidden. + instance.visibility |= OffscreenPassiveEffectsConnected; + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } + } + } else { + // Tree is visible + + // Since we're already inside a reconnecting tree, it doesn't matter + // whether the effects are currently connected. In either case, we'll + // continue traversing the tree and firing all the effects. + // + // We do need to set the "connected" flag on the instance, though. + instance.visibility |= OffscreenPassiveEffectsConnected; + + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + } + + if (includeWorkInProgressEffects && flags & Passive) { + // TODO: Pass `current` as argument to this function + const current: Fiber | null = finishedWork.alternate; + commitOffscreenPassiveMountEffects(current, finishedWork, instance); + } + break; + } + case CacheComponent: { + recursivelyTraverseReconnectPassiveEffects( + finishedRoot, + finishedWork, + committedLanes, + committedTransitions, + includeWorkInProgressEffects, + ); + if (includeWorkInProgressEffects && flags & Passive) { // TODO: Pass `current` as argument to this function const current = finishedWork.alternate; commitCachePassiveMountEffect(current, finishedWork); @@ -3181,13 +3426,14 @@ function commitPassiveMountOnFiber( } case TracingMarkerComponent: { if (enableTransitionTracing) { - recursivelyTraversePassiveMountEffects( + recursivelyTraverseReconnectPassiveEffects( finishedRoot, finishedWork, committedLanes, committedTransitions, + includeWorkInProgressEffects, ); - if (flags & Passive) { + if (includeWorkInProgressEffects && flags & Passive) { commitTracingMarkerPassiveMountEffect(finishedWork); } break; @@ -3196,11 +3442,12 @@ function commitPassiveMountOnFiber( } // eslint-disable-next-line-no-fallthrough default: { - recursivelyTraversePassiveMountEffects( + recursivelyTraverseReconnectPassiveEffects( finishedRoot, finishedWork, committedLanes, committedTransitions, + includeWorkInProgressEffects, ); break; } @@ -3304,6 +3551,11 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { } break; } + // TODO: Disconnect passive effects when a tree is hidden, perhaps after + // a delay. + // case OffscreenComponent: { + // ... + // } default: { recursivelyTraversePassiveUnmountEffects(finishedWork); break; diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index d22d9a189b6e5..961c94bdd1685 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -87,7 +87,7 @@ export const MutationMask = export const LayoutMask = Update | Callback | Ref | Visibility; // TODO: Split into PassiveMountMask and PassiveUnmountMask -export const PassiveMask = Passive | ChildDeletion; +export const PassiveMask = Passive | Visibility | ChildDeletion; // Union of tags that don't get reset on clones. // This allows certain concepts to persist without recalculating them, diff --git a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js index 7655387ced19c..7bdaf83b90320 100644 --- a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js +++ b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js @@ -45,9 +45,7 @@ export type OffscreenQueue = {| type OffscreenVisibility = number; export const OffscreenVisible = /* */ 0b01; - -// TODO -// export const OffscreenPassiveEffectsConnected = /* */ 0b01; +export const OffscreenPassiveEffectsConnected = /* */ 0b10; export type OffscreenInstance = {| visibility: OffscreenVisibility, diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index c2fe2f77552b0..5e5675bb64c7f 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -486,22 +486,29 @@ describe('ReactOffscreen', () => { // hidden tree share the same lane, but are processed at different times // because of the timing of when they were scheduled. + // This functions checks whether the "outer" and "inner" states are + // consistent in the rendered output. + let currentOuter = null; + let currentInner = null; + function areOuterAndInnerConsistent() { + return ( + currentOuter === null || + currentInner === null || + currentOuter === currentInner + ); + } + let setInner; - function Child({outer}) { + function Child() { const [inner, _setInner] = useState(0); setInner = _setInner; useEffect(() => { - // Inner and outer values are always updated simultaneously, so they - // should always be consistent. - if (inner !== outer) { - Scheduler.unstable_yieldValue( - 'Tearing! Inner and outer are inconsistent!', - ); - } else { - Scheduler.unstable_yieldValue('Inner and outer are consistent'); - } - }, [inner, outer]); + currentInner = inner; + return () => { + currentInner = null; + }; + }, [inner]); return ; } @@ -510,11 +517,19 @@ describe('ReactOffscreen', () => { function App({show}) { const [outer, _setOuter] = useState(0); setOuter = _setOuter; + + useEffect(() => { + currentOuter = outer; + return () => { + currentOuter = null; + }; + }, [outer]); + return ( <> - + ); @@ -525,17 +540,14 @@ describe('ReactOffscreen', () => { await act(async () => { root.render(); }); - expect(Scheduler).toHaveYielded([ - 'Outer: 0', - 'Inner: 0', - 'Inner and outer are consistent', - ]); + expect(Scheduler).toHaveYielded(['Outer: 0', 'Inner: 0']); expect(root).toMatchRenderedOutput( <>