From a86f80df7d0517bd585443d4a4c3f2ac9cbafa3b Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 13 Sep 2022 18:03:20 +0100 Subject: [PATCH 1/6] Expose ref to Offscreen if mode is manual --- .../src/ReactFiberBeginWork.new.js | 4 ++ .../src/ReactFiberBeginWork.old.js | 4 ++ .../src/ReactFiberCommitWork.new.js | 8 ++++ .../src/ReactFiberCommitWork.old.js | 8 ++++ .../src/__tests__/ReactOffscreen-test.js | 48 +++++++++++++++++++ packages/shared/ReactTypes.js | 3 +- 6 files changed, 74 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index fa417e08114c..2c6bdb6e25e4 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -677,6 +677,10 @@ function updateOffscreenComponent( const prevState: OffscreenState | null = current !== null ? current.memoizedState : null; + if (nextProps.mode === 'manual') { + markRef(current, workInProgress); + } + if ( nextProps.mode === 'hidden' || (enableLegacyHidden && nextProps.mode === 'unstable-defer-without-hiding') diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 449f306e7ef8..49fa8405fb63 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -677,6 +677,10 @@ function updateOffscreenComponent( const prevState: OffscreenState | null = current !== null ? current.memoizedState : null; + if (nextProps.mode === 'manual') { + markRef(current, workInProgress); + } + if ( nextProps.mode === 'hidden' || (enableLegacyHidden && nextProps.mode === 'unstable-defer-without-hiding') diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index c70e4ae3470e..524d0d21d5a1 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1116,6 +1116,14 @@ function commitLayoutEffectOnFiber( offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; } + + if (finishedWork.pendingProps.mode === 'manual') { + if (flags & Ref) { + safelyAttachRef(finishedWork, finishedWork.return); + } + } else if (finishedWork.pendingProps.mode !== undefined) { + safelyDetachRef(finishedWork, finishedWork.return); + } } else { recursivelyTraverseLayoutEffects( finishedRoot, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 4b5076a61c51..e1ded497ccd7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1116,6 +1116,14 @@ function commitLayoutEffectOnFiber( offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; } + + if (finishedWork.pendingProps.mode === 'manual') { + if (flags & Ref) { + safelyAttachRef(finishedWork, finishedWork.return); + } + } else if (finishedWork.pendingProps.mode !== undefined) { + safelyDetachRef(finishedWork, finishedWork.return); + } } else { recursivelyTraverseLayoutEffects( finishedRoot, diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index 4e77f831f928..6180c8a1e710 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -8,6 +8,7 @@ let useState; let useLayoutEffect; let useEffect; let useMemo; +let useRef; let startTransition; describe('ReactOffscreen', () => { @@ -24,6 +25,7 @@ describe('ReactOffscreen', () => { useLayoutEffect = React.useLayoutEffect; useEffect = React.useEffect; useMemo = React.useMemo; + useRef = React.useRef; startTransition = React.startTransition; }); @@ -1259,4 +1261,50 @@ describe('ReactOffscreen', () => { , ); }); + + describe('manual interactivity', () => { + // @gate enableOffscreen + it('should attach ref only for mode null', async () => { + let offscreenRef; + + function App({mode}) { + offscreenRef = useRef(null); + return ( + { + offscreenRef.current = ref; + }}> +
+ + ); + } + + const root = ReactNoop.createRoot(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).not.toBeNull(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).toBeNull(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).toBeNull(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).not.toBeNull(); + }); + }); }); diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 5ef83306706a..48b4ce11c1b6 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -209,7 +209,8 @@ export type Thenable = export type OffscreenMode = | 'hidden' | 'unstable-defer-without-hiding' - | 'visible'; + | 'visible' + | 'manual'; export type StartTransitionOptions = { name?: string, From d8611accad946cc681073a17503f9717558cdeaf Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 13 Sep 2022 18:03:59 +0100 Subject: [PATCH 2/6] Prepend private fields on OffscreenInstance with underscore --- .../react-reconciler/src/ReactFiber.new.js | 16 +++--- .../react-reconciler/src/ReactFiber.old.js | 16 +++--- .../src/ReactFiberBeginWork.new.js | 4 +- .../src/ReactFiberBeginWork.old.js | 4 +- .../src/ReactFiberCommitWork.new.js | 56 +++++++++---------- .../src/ReactFiberCommitWork.old.js | 56 +++++++++---------- .../src/ReactFiberConcurrentUpdates.new.js | 2 +- .../src/ReactFiberConcurrentUpdates.old.js | 2 +- .../src/ReactFiberOffscreenComponent.js | 8 +-- .../src/ReactFiberWorkLoop.new.js | 2 +- .../src/ReactFiberWorkLoop.old.js | 2 +- 11 files changed, 84 insertions(+), 84 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index d6a6c8f7adc1..46f35fabf1fe 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -719,10 +719,10 @@ export function createFiberFromOffscreen( fiber.elementType = REACT_OFFSCREEN_TYPE; fiber.lanes = lanes; const primaryChildInstance: OffscreenInstance = { - visibility: OffscreenVisible, - pendingMarkers: null, - retryCache: null, - transitions: null, + _visibility: OffscreenVisible, + _pendingMarkers: null, + _retryCache: null, + _transitions: null, }; fiber.stateNode = primaryChildInstance; return fiber; @@ -740,10 +740,10 @@ 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 = { - visibility: OffscreenVisible, - pendingMarkers: null, - transitions: null, - retryCache: null, + _visibility: OffscreenVisible, + _pendingMarkers: null, + _transitions: null, + _retryCache: null, }; fiber.stateNode = instance; return fiber; diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index d8f1ef424bd1..22712b6e18ee 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -719,10 +719,10 @@ export function createFiberFromOffscreen( fiber.elementType = REACT_OFFSCREEN_TYPE; fiber.lanes = lanes; const primaryChildInstance: OffscreenInstance = { - visibility: OffscreenVisible, - pendingMarkers: null, - retryCache: null, - transitions: null, + _visibility: OffscreenVisible, + _pendingMarkers: null, + _retryCache: null, + _transitions: null, }; fiber.stateNode = primaryChildInstance; return fiber; @@ -740,10 +740,10 @@ 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 = { - visibility: OffscreenVisible, - pendingMarkers: null, - transitions: null, - retryCache: null, + _visibility: OffscreenVisible, + _pendingMarkers: null, + _transitions: null, + _retryCache: null, }; fiber.stateNode = instance; return fiber; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 2c6bdb6e25e4..81909edf5beb 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -815,8 +815,8 @@ function updateOffscreenComponent( // We have now gone from hidden to visible, so any transitions should // be added to the stack to get added to any Offscreen/suspense children const instance: OffscreenInstance | null = workInProgress.stateNode; - if (instance !== null && instance.transitions != null) { - transitions = Array.from(instance.transitions); + if (instance !== null && instance._transitions != null) { + transitions = Array.from(instance._transitions); } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 49fa8405fb63..cfe7a45a80ae 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -815,8 +815,8 @@ function updateOffscreenComponent( // We have now gone from hidden to visible, so any transitions should // be added to the stack to get added to any Offscreen/suspense children const instance: OffscreenInstance | null = workInProgress.stateNode; - if (instance !== null && instance.transitions != null) { - transitions = Array.from(instance.transitions); + if (instance !== null && instance._transitions != null) { + transitions = Array.from(instance._transitions); } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 524d0d21d5a1..5f8de4fab94e 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1304,7 +1304,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) { const wasHidden = prevState !== null; const isHidden = nextState !== null; - const pendingMarkers = offscreenInstance.pendingMarkers; + const pendingMarkers = offscreenInstance._pendingMarkers; // If there is a name on the suspense boundary, store that in // the pending boundaries. let name = null; @@ -2240,9 +2240,9 @@ function getRetryCache(finishedWork) { } case OffscreenComponent: { const instance: OffscreenInstance = finishedWork.stateNode; - let retryCache = instance.retryCache; + let retryCache = instance._retryCache; if (retryCache === null) { - retryCache = instance.retryCache = new PossiblyWeakSet(); + retryCache = instance._retryCache = new PossiblyWeakSet(); } return retryCache; } @@ -2641,9 +2641,9 @@ function commitMutationEffectsOnFiber( // Track the current state on the Offscreen instance so we can // read it during an event if (isHidden) { - offscreenInstance.visibility &= ~OffscreenVisible; + offscreenInstance._visibility &= ~OffscreenVisible; } else { - offscreenInstance.visibility |= OffscreenVisible; + offscreenInstance._visibility |= OffscreenVisible; } if (isHidden) { @@ -3088,10 +3088,10 @@ function commitOffscreenPassiveMountEffects( // 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(); + if (instance._transitions === null) { + instance._transitions = new Set(); } - instance.transitions.add(transition); + instance._transitions.add(transition); }); } @@ -3104,17 +3104,17 @@ function commitOffscreenPassiveMountEffects( // caused them if (markerTransitions !== null) { markerTransitions.forEach(transition => { - if (instance.transitions === null) { - instance.transitions = new Set(); - } else if (instance.transitions.has(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(); + if (instance._pendingMarkers === null) { + instance._pendingMarkers = new Set(); } - instance.pendingMarkers.add(markerInstance); + instance._pendingMarkers.add(markerInstance); } }); } @@ -3129,8 +3129,8 @@ function commitOffscreenPassiveMountEffects( // TODO: Refactor this into an if/else branch if (!isHidden) { - instance.transitions = null; - instance.pendingMarkers = null; + instance._transitions = null; + instance._pendingMarkers = null; } } } @@ -3310,7 +3310,7 @@ function commitPassiveMountOnFiber( const isHidden = nextState !== null; if (isHidden) { - if (instance.visibility & OffscreenPassiveEffectsConnected) { + if (instance._visibility & OffscreenPassiveEffectsConnected) { // The effects are currently connected. Update them. recursivelyTraversePassiveMountEffects( finishedRoot, @@ -3335,7 +3335,7 @@ function commitPassiveMountOnFiber( } } else { // Legacy Mode: Fire the effects even if the tree is hidden. - instance.visibility |= OffscreenPassiveEffectsConnected; + instance._visibility |= OffscreenPassiveEffectsConnected; recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, @@ -3346,7 +3346,7 @@ function commitPassiveMountOnFiber( } } else { // Tree is visible - if (instance.visibility & OffscreenPassiveEffectsConnected) { + if (instance._visibility & OffscreenPassiveEffectsConnected) { // The effects are currently connected. Update them. recursivelyTraversePassiveMountEffects( finishedRoot, @@ -3358,7 +3358,7 @@ function commitPassiveMountOnFiber( // 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; + instance._visibility |= OffscreenPassiveEffectsConnected; const includeWorkInProgressEffects = (finishedWork.subtreeFlags & PassiveMask) !== NoFlags; @@ -3490,7 +3490,7 @@ export function reconnectPassiveEffects( const isHidden = nextState !== null; if (isHidden) { - if (instance.visibility & OffscreenPassiveEffectsConnected) { + if (instance._visibility & OffscreenPassiveEffectsConnected) { // The effects are currently connected. Update them. recursivelyTraverseReconnectPassiveEffects( finishedRoot, @@ -3516,7 +3516,7 @@ export function reconnectPassiveEffects( } } else { // Legacy Mode: Fire the effects even if the tree is hidden. - instance.visibility |= OffscreenPassiveEffectsConnected; + instance._visibility |= OffscreenPassiveEffectsConnected; recursivelyTraverseReconnectPassiveEffects( finishedRoot, finishedWork, @@ -3534,7 +3534,7 @@ export function reconnectPassiveEffects( // continue traversing the tree and firing all the effects. // // We do need to set the "connected" flag on the instance, though. - instance.visibility |= OffscreenPassiveEffectsConnected; + instance._visibility |= OffscreenPassiveEffectsConnected; recursivelyTraverseReconnectPassiveEffects( finishedRoot, @@ -3789,7 +3789,7 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { if ( isHidden && - instance.visibility & OffscreenPassiveEffectsConnected && + instance._visibility & OffscreenPassiveEffectsConnected && // For backwards compatibility, don't unmount when a tree suspends. In // the future we may change this to unmount after a delay. (finishedWork.return === null || @@ -3799,7 +3799,7 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { // TODO: Add option or heuristic to delay before disconnecting the // effects. Then if the tree reappears before the delay has elapsed, we // can skip toggling the effects entirely. - instance.visibility &= ~OffscreenPassiveEffectsConnected; + instance._visibility &= ~OffscreenPassiveEffectsConnected; recursivelyTraverseDisconnectPassiveEffects(finishedWork); } else { recursivelyTraversePassiveUnmountEffects(finishedWork); @@ -3863,8 +3863,8 @@ export function disconnectPassiveEffect(finishedWork: Fiber): void { } case OffscreenComponent: { const instance: OffscreenInstance = finishedWork.stateNode; - if (instance.visibility & OffscreenPassiveEffectsConnected) { - instance.visibility &= ~OffscreenPassiveEffectsConnected; + if (instance._visibility & OffscreenPassiveEffectsConnected) { + instance._visibility &= ~OffscreenPassiveEffectsConnected; recursivelyTraverseDisconnectPassiveEffects(finishedWork); } else { // The effects are already disconnected. @@ -3992,7 +3992,7 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( // We need to mark this fiber's parents as deleted const offscreenFiber: Fiber = (current.child: any); const instance: OffscreenInstance = offscreenFiber.stateNode; - const transitions = instance.transitions; + const transitions = instance._transitions; if (transitions !== null) { const abortReason = { reason: 'suspense', diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index e1ded497ccd7..c3c2e964ad49 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1304,7 +1304,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) { const wasHidden = prevState !== null; const isHidden = nextState !== null; - const pendingMarkers = offscreenInstance.pendingMarkers; + const pendingMarkers = offscreenInstance._pendingMarkers; // If there is a name on the suspense boundary, store that in // the pending boundaries. let name = null; @@ -2240,9 +2240,9 @@ function getRetryCache(finishedWork) { } case OffscreenComponent: { const instance: OffscreenInstance = finishedWork.stateNode; - let retryCache = instance.retryCache; + let retryCache = instance._retryCache; if (retryCache === null) { - retryCache = instance.retryCache = new PossiblyWeakSet(); + retryCache = instance._retryCache = new PossiblyWeakSet(); } return retryCache; } @@ -2641,9 +2641,9 @@ function commitMutationEffectsOnFiber( // Track the current state on the Offscreen instance so we can // read it during an event if (isHidden) { - offscreenInstance.visibility &= ~OffscreenVisible; + offscreenInstance._visibility &= ~OffscreenVisible; } else { - offscreenInstance.visibility |= OffscreenVisible; + offscreenInstance._visibility |= OffscreenVisible; } if (isHidden) { @@ -3088,10 +3088,10 @@ function commitOffscreenPassiveMountEffects( // 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(); + if (instance._transitions === null) { + instance._transitions = new Set(); } - instance.transitions.add(transition); + instance._transitions.add(transition); }); } @@ -3104,17 +3104,17 @@ function commitOffscreenPassiveMountEffects( // caused them if (markerTransitions !== null) { markerTransitions.forEach(transition => { - if (instance.transitions === null) { - instance.transitions = new Set(); - } else if (instance.transitions.has(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(); + if (instance._pendingMarkers === null) { + instance._pendingMarkers = new Set(); } - instance.pendingMarkers.add(markerInstance); + instance._pendingMarkers.add(markerInstance); } }); } @@ -3129,8 +3129,8 @@ function commitOffscreenPassiveMountEffects( // TODO: Refactor this into an if/else branch if (!isHidden) { - instance.transitions = null; - instance.pendingMarkers = null; + instance._transitions = null; + instance._pendingMarkers = null; } } } @@ -3310,7 +3310,7 @@ function commitPassiveMountOnFiber( const isHidden = nextState !== null; if (isHidden) { - if (instance.visibility & OffscreenPassiveEffectsConnected) { + if (instance._visibility & OffscreenPassiveEffectsConnected) { // The effects are currently connected. Update them. recursivelyTraversePassiveMountEffects( finishedRoot, @@ -3335,7 +3335,7 @@ function commitPassiveMountOnFiber( } } else { // Legacy Mode: Fire the effects even if the tree is hidden. - instance.visibility |= OffscreenPassiveEffectsConnected; + instance._visibility |= OffscreenPassiveEffectsConnected; recursivelyTraversePassiveMountEffects( finishedRoot, finishedWork, @@ -3346,7 +3346,7 @@ function commitPassiveMountOnFiber( } } else { // Tree is visible - if (instance.visibility & OffscreenPassiveEffectsConnected) { + if (instance._visibility & OffscreenPassiveEffectsConnected) { // The effects are currently connected. Update them. recursivelyTraversePassiveMountEffects( finishedRoot, @@ -3358,7 +3358,7 @@ function commitPassiveMountOnFiber( // 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; + instance._visibility |= OffscreenPassiveEffectsConnected; const includeWorkInProgressEffects = (finishedWork.subtreeFlags & PassiveMask) !== NoFlags; @@ -3490,7 +3490,7 @@ export function reconnectPassiveEffects( const isHidden = nextState !== null; if (isHidden) { - if (instance.visibility & OffscreenPassiveEffectsConnected) { + if (instance._visibility & OffscreenPassiveEffectsConnected) { // The effects are currently connected. Update them. recursivelyTraverseReconnectPassiveEffects( finishedRoot, @@ -3516,7 +3516,7 @@ export function reconnectPassiveEffects( } } else { // Legacy Mode: Fire the effects even if the tree is hidden. - instance.visibility |= OffscreenPassiveEffectsConnected; + instance._visibility |= OffscreenPassiveEffectsConnected; recursivelyTraverseReconnectPassiveEffects( finishedRoot, finishedWork, @@ -3534,7 +3534,7 @@ export function reconnectPassiveEffects( // continue traversing the tree and firing all the effects. // // We do need to set the "connected" flag on the instance, though. - instance.visibility |= OffscreenPassiveEffectsConnected; + instance._visibility |= OffscreenPassiveEffectsConnected; recursivelyTraverseReconnectPassiveEffects( finishedRoot, @@ -3789,7 +3789,7 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { if ( isHidden && - instance.visibility & OffscreenPassiveEffectsConnected && + instance._visibility & OffscreenPassiveEffectsConnected && // For backwards compatibility, don't unmount when a tree suspends. In // the future we may change this to unmount after a delay. (finishedWork.return === null || @@ -3799,7 +3799,7 @@ function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { // TODO: Add option or heuristic to delay before disconnecting the // effects. Then if the tree reappears before the delay has elapsed, we // can skip toggling the effects entirely. - instance.visibility &= ~OffscreenPassiveEffectsConnected; + instance._visibility &= ~OffscreenPassiveEffectsConnected; recursivelyTraverseDisconnectPassiveEffects(finishedWork); } else { recursivelyTraversePassiveUnmountEffects(finishedWork); @@ -3863,8 +3863,8 @@ export function disconnectPassiveEffect(finishedWork: Fiber): void { } case OffscreenComponent: { const instance: OffscreenInstance = finishedWork.stateNode; - if (instance.visibility & OffscreenPassiveEffectsConnected) { - instance.visibility &= ~OffscreenPassiveEffectsConnected; + if (instance._visibility & OffscreenPassiveEffectsConnected) { + instance._visibility &= ~OffscreenPassiveEffectsConnected; recursivelyTraverseDisconnectPassiveEffects(finishedWork); } else { // The effects are already disconnected. @@ -3992,7 +3992,7 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( // We need to mark this fiber's parents as deleted const offscreenFiber: Fiber = (current.child: any); const instance: OffscreenInstance = offscreenFiber.stateNode; - const transitions = instance.transitions; + const transitions = instance._transitions; if (transitions !== null) { const abortReason = { reason: 'suspense', diff --git a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js index 6b1ecbde7f01..ba5c68fb0d0c 100644 --- a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js +++ b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js @@ -220,7 +220,7 @@ function markUpdateLaneFromFiberToRoot( const offscreenInstance: OffscreenInstance | null = parent.stateNode; if ( offscreenInstance !== null && - !(offscreenInstance.visibility & OffscreenVisible) + !(offscreenInstance._visibility & OffscreenVisible) ) { isHidden = true; } diff --git a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js index 798506fd9b5e..de1450fc228a 100644 --- a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js +++ b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.old.js @@ -220,7 +220,7 @@ function markUpdateLaneFromFiberToRoot( const offscreenInstance: OffscreenInstance | null = parent.stateNode; if ( offscreenInstance !== null && - !(offscreenInstance.visibility & OffscreenVisible) + !(offscreenInstance._visibility & OffscreenVisible) ) { isHidden = true; } diff --git a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js index 1c45f6135d68..081f6a5a519e 100644 --- a/packages/react-reconciler/src/ReactFiberOffscreenComponent.js +++ b/packages/react-reconciler/src/ReactFiberOffscreenComponent.js @@ -48,8 +48,8 @@ export const OffscreenVisible = /* */ 0b01; export const OffscreenPassiveEffectsConnected = /* */ 0b10; export type OffscreenInstance = { - visibility: OffscreenVisibility, - pendingMarkers: Set | null, - transitions: Set | null, - retryCache: WeakSet | Set | null, + _visibility: OffscreenVisibility, + _pendingMarkers: Set | null, + _transitions: Set | null, + _retryCache: WeakSet | Set | null, }; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 6f62aa4b1c0b..b7169f5493c6 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -3085,7 +3085,7 @@ export function resolveRetryWakeable(boundaryFiber: Fiber, wakeable: Wakeable) { break; case OffscreenComponent: { const instance: OffscreenInstance = boundaryFiber.stateNode; - retryCache = instance.retryCache; + retryCache = instance._retryCache; break; } default: diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 414e2e3343ed..3071dc5a1626 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -3085,7 +3085,7 @@ export function resolveRetryWakeable(boundaryFiber: Fiber, wakeable: Wakeable) { break; case OffscreenComponent: { const instance: OffscreenInstance = boundaryFiber.stateNode; - retryCache = instance.retryCache; + retryCache = instance._retryCache; break; } default: From a8f7a96fac81c141fd9777a55f593beec66aeb48 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 21 Sep 2022 10:59:31 +0100 Subject: [PATCH 3/6] Schedule Ref effect unconditionally on Offscreen --- packages/react-reconciler/src/ReactFiberBeginWork.new.js | 4 +--- packages/react-reconciler/src/ReactFiberBeginWork.old.js | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 81909edf5beb..e0e605b14df3 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -677,9 +677,7 @@ function updateOffscreenComponent( const prevState: OffscreenState | null = current !== null ? current.memoizedState : null; - if (nextProps.mode === 'manual') { - markRef(current, workInProgress); - } + markRef(current, workInProgress); if ( nextProps.mode === 'hidden' || diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index cfe7a45a80ae..acbba6a6bedd 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -677,9 +677,7 @@ function updateOffscreenComponent( const prevState: OffscreenState | null = current !== null ? current.memoizedState : null; - if (nextProps.mode === 'manual') { - markRef(current, workInProgress); - } + markRef(current, workInProgress); if ( nextProps.mode === 'hidden' || From 1608017e77274b378a421db9c071295fbe65fbae Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 21 Sep 2022 11:18:53 +0100 Subject: [PATCH 4/6] Make sure Offscreen's ref is detached when unmounted --- .../src/ReactFiberCommitWork.new.js | 7 +- .../src/ReactFiberCommitWork.old.js | 7 +- .../src/__tests__/ReactOffscreen-test.js | 80 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 5f8de4fab94e..dcf07571ded4 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1121,7 +1121,7 @@ function commitLayoutEffectOnFiber( if (flags & Ref) { safelyAttachRef(finishedWork, finishedWork.return); } - } else if (finishedWork.pendingProps.mode !== undefined) { + } else { safelyDetachRef(finishedWork, finishedWork.return); } } else { @@ -2148,6 +2148,8 @@ function commitDeletionEffectsOnFiber( offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null; + safelyDetachRef(deletedFiber, nearestMountedAncestor); + recursivelyTraverseDeletionEffects( finishedRoot, nearestMountedAncestor, @@ -2833,6 +2835,7 @@ export function disappearLayoutEffects(finishedWork: Fiber) { // Nested Offscreen tree is already hidden. Don't disappear // its effects. } else { + safelyDetachRef(finishedWork, finishedWork.return); recursivelyTraverseDisappearLayoutEffects(finishedWork); } break; @@ -2974,6 +2977,8 @@ export function reappearLayoutEffects( finishedWork, includeWorkInProgressEffects, ); + + safelyAttachRef(finishedWork, finishedWork.return); } break; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index c3c2e964ad49..0fb539a5b478 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1121,7 +1121,7 @@ function commitLayoutEffectOnFiber( if (flags & Ref) { safelyAttachRef(finishedWork, finishedWork.return); } - } else if (finishedWork.pendingProps.mode !== undefined) { + } else { safelyDetachRef(finishedWork, finishedWork.return); } } else { @@ -2148,6 +2148,8 @@ function commitDeletionEffectsOnFiber( offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null; + safelyDetachRef(deletedFiber, nearestMountedAncestor); + recursivelyTraverseDeletionEffects( finishedRoot, nearestMountedAncestor, @@ -2833,6 +2835,7 @@ export function disappearLayoutEffects(finishedWork: Fiber) { // Nested Offscreen tree is already hidden. Don't disappear // its effects. } else { + safelyDetachRef(finishedWork, finishedWork.return); recursivelyTraverseDisappearLayoutEffects(finishedWork); } break; @@ -2974,6 +2977,8 @@ export function reappearLayoutEffects( finishedWork, includeWorkInProgressEffects, ); + + safelyAttachRef(finishedWork, finishedWork.return); } break; } diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index 6180c8a1e710..c3dcb38e5ec5 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -1307,4 +1307,84 @@ describe('ReactOffscreen', () => { expect(offscreenRef.current).not.toBeNull(); }); }); + + // @gate enableOffscreen + it('should detach ref if Offscreen is unmounted', async () => { + let offscreenRef; + + function App({showOffscreen}) { + offscreenRef = useRef(null); + return showOffscreen ? ( + { + offscreenRef.current = ref; + }}> +
+ + ) : null; + } + + const root = ReactNoop.createRoot(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).not.toBeNull(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).toBeNull(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).not.toBeNull(); + }); + + // @gate enableOffscreen + it('should detach ref if Offscreen is unmounted', async () => { + let offscreenRef; + let divRef; + + function App({mode}) { + offscreenRef = useRef(null); + divRef = useRef(null); + return ( + + +
+ + + ); + } + + const root = ReactNoop.createRoot(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).toBeNull(); + expect(divRef.current).toBeNull(); + + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).not.toBeNull(); + expect(divRef.current).not.toBeNull(); + + console.log('Becoming Hidden'); + await act(async () => { + root.render(); + }); + + expect(offscreenRef.current).toBeNull(); + expect(divRef.current).toBeNull(); + }); }); From 4320341cf06d4b47946cd2191258f20b3aaeedba Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 21 Sep 2022 17:42:38 +0100 Subject: [PATCH 5/6] Make sure ref is mounted/unmounted in all scenarious --- .../src/ReactFiberCommitWork.new.js | 32 +++++++++++-------- .../src/ReactFiberCommitWork.old.js | 32 +++++++++++-------- .../src/__tests__/ReactOffscreen-test.js | 12 +++---- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index dcf07571ded4..13997e4e0c98 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1116,14 +1116,6 @@ function commitLayoutEffectOnFiber( offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; } - - if (finishedWork.pendingProps.mode === 'manual') { - if (flags & Ref) { - safelyAttachRef(finishedWork, finishedWork.return); - } - } else { - safelyDetachRef(finishedWork, finishedWork.return); - } } else { recursivelyTraverseLayoutEffects( finishedRoot, @@ -1131,6 +1123,13 @@ function commitLayoutEffectOnFiber( committedLanes, ); } + if (flags & Ref) { + if (finishedWork.pendingProps.mode === 'manual') { + safelyAttachRef(finishedWork, finishedWork.return); + } else { + safelyDetachRef(finishedWork, finishedWork.return); + } + } break; } default: { @@ -2134,6 +2133,7 @@ function commitDeletionEffectsOnFiber( return; } case OffscreenComponent: { + safelyDetachRef(deletedFiber, nearestMountedAncestor); if (deletedFiber.mode & ConcurrentMode) { // If this offscreen component is hidden, we already unmounted it. Before // deleting the children, track that it's already unmounted so that we @@ -2148,8 +2148,6 @@ function commitDeletionEffectsOnFiber( offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null; - safelyDetachRef(deletedFiber, nearestMountedAncestor); - recursivelyTraverseDeletionEffects( finishedRoot, nearestMountedAncestor, @@ -2615,6 +2613,12 @@ function commitMutationEffectsOnFiber( return; } case OffscreenComponent: { + if (flags & Ref) { + if (current !== null) { + safelyDetachRef(current, current.return); + } + } + const newState: OffscreenState | null = finishedWork.memoizedState; const isHidden = newState !== null; const wasHidden = current !== null && current.memoizedState !== null; @@ -2830,12 +2834,14 @@ export function disappearLayoutEffects(finishedWork: Fiber) { break; } case OffscreenComponent: { + // TODO (Offscreen) Check: flags & RefStatic + safelyDetachRef(finishedWork, finishedWork.return); + const isHidden = finishedWork.memoizedState !== null; if (isHidden) { // Nested Offscreen tree is already hidden. Don't disappear // its effects. } else { - safelyDetachRef(finishedWork, finishedWork.return); recursivelyTraverseDisappearLayoutEffects(finishedWork); } break; @@ -2977,9 +2983,9 @@ export function reappearLayoutEffects( finishedWork, includeWorkInProgressEffects, ); - - safelyAttachRef(finishedWork, finishedWork.return); } + // TODO: Check flags & Ref + safelyAttachRef(finishedWork, finishedWork.return); break; } default: { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 0fb539a5b478..30b88294fdac 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1116,14 +1116,6 @@ function commitLayoutEffectOnFiber( offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; } - - if (finishedWork.pendingProps.mode === 'manual') { - if (flags & Ref) { - safelyAttachRef(finishedWork, finishedWork.return); - } - } else { - safelyDetachRef(finishedWork, finishedWork.return); - } } else { recursivelyTraverseLayoutEffects( finishedRoot, @@ -1131,6 +1123,13 @@ function commitLayoutEffectOnFiber( committedLanes, ); } + if (flags & Ref) { + if (finishedWork.pendingProps.mode === 'manual') { + safelyAttachRef(finishedWork, finishedWork.return); + } else { + safelyDetachRef(finishedWork, finishedWork.return); + } + } break; } default: { @@ -2134,6 +2133,7 @@ function commitDeletionEffectsOnFiber( return; } case OffscreenComponent: { + safelyDetachRef(deletedFiber, nearestMountedAncestor); if (deletedFiber.mode & ConcurrentMode) { // If this offscreen component is hidden, we already unmounted it. Before // deleting the children, track that it's already unmounted so that we @@ -2148,8 +2148,6 @@ function commitDeletionEffectsOnFiber( offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || deletedFiber.memoizedState !== null; - safelyDetachRef(deletedFiber, nearestMountedAncestor); - recursivelyTraverseDeletionEffects( finishedRoot, nearestMountedAncestor, @@ -2615,6 +2613,12 @@ function commitMutationEffectsOnFiber( return; } case OffscreenComponent: { + if (flags & Ref) { + if (current !== null) { + safelyDetachRef(current, current.return); + } + } + const newState: OffscreenState | null = finishedWork.memoizedState; const isHidden = newState !== null; const wasHidden = current !== null && current.memoizedState !== null; @@ -2830,12 +2834,14 @@ export function disappearLayoutEffects(finishedWork: Fiber) { break; } case OffscreenComponent: { + // TODO (Offscreen) Check: flags & RefStatic + safelyDetachRef(finishedWork, finishedWork.return); + const isHidden = finishedWork.memoizedState !== null; if (isHidden) { // Nested Offscreen tree is already hidden. Don't disappear // its effects. } else { - safelyDetachRef(finishedWork, finishedWork.return); recursivelyTraverseDisappearLayoutEffects(finishedWork); } break; @@ -2977,9 +2983,9 @@ export function reappearLayoutEffects( finishedWork, includeWorkInProgressEffects, ); - - safelyAttachRef(finishedWork, finishedWork.return); } + // TODO: Check flags & Ref + safelyAttachRef(finishedWork, finishedWork.return); break; } default: { diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index c3dcb38e5ec5..30bbaea05899 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -1347,17 +1347,15 @@ describe('ReactOffscreen', () => { }); // @gate enableOffscreen - it('should detach ref if Offscreen is unmounted', async () => { + it('should detach ref when parent Offscreen is hidden', async () => { let offscreenRef; - let divRef; function App({mode}) { offscreenRef = useRef(null); - divRef = useRef(null); return ( -
+
); @@ -1370,21 +1368,19 @@ describe('ReactOffscreen', () => { }); expect(offscreenRef.current).toBeNull(); - expect(divRef.current).toBeNull(); await act(async () => { root.render(); }); expect(offscreenRef.current).not.toBeNull(); - expect(divRef.current).not.toBeNull(); - console.log('Becoming Hidden'); await act(async () => { root.render(); }); expect(offscreenRef.current).toBeNull(); - expect(divRef.current).toBeNull(); }); + + // TODO: When attach/detach methods are implemented. Add tests for nested Offscreen case. }); From c7e962ffb09e20732d2d2ec47cfeafd5b711d758 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 23 Sep 2022 11:25:13 -0400 Subject: [PATCH 6/6] Nit: pendingProps -> memoizedProps --- packages/react-reconciler/src/ReactFiberCommitWork.new.js | 4 +++- packages/react-reconciler/src/ReactFiberCommitWork.old.js | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 13997e4e0c98..dabc8b1987e8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -26,6 +26,7 @@ import type { OffscreenState, OffscreenInstance, OffscreenQueue, + OffscreenProps, } from './ReactFiberOffscreenComponent'; import type {HookFlags} from './ReactHookEffectTags'; import type {Cache} from './ReactFiberCacheComponent.new'; @@ -1124,7 +1125,8 @@ function commitLayoutEffectOnFiber( ); } if (flags & Ref) { - if (finishedWork.pendingProps.mode === 'manual') { + const props: OffscreenProps = finishedWork.memoizedProps; + if (props.mode === 'manual') { safelyAttachRef(finishedWork, finishedWork.return); } else { safelyDetachRef(finishedWork, finishedWork.return); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 30b88294fdac..a5cad4407b63 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -26,6 +26,7 @@ import type { OffscreenState, OffscreenInstance, OffscreenQueue, + OffscreenProps, } from './ReactFiberOffscreenComponent'; import type {HookFlags} from './ReactHookEffectTags'; import type {Cache} from './ReactFiberCacheComponent.old'; @@ -1124,7 +1125,8 @@ function commitLayoutEffectOnFiber( ); } if (flags & Ref) { - if (finishedWork.pendingProps.mode === 'manual') { + const props: OffscreenProps = finishedWork.memoizedProps; + if (props.mode === 'manual') { safelyAttachRef(finishedWork, finishedWork.return); } else { safelyDetachRef(finishedWork, finishedWork.return);