diff --git a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js index ae9254add8d8d..d3e6996a49a24 100644 --- a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js +++ b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js @@ -102,7 +102,12 @@ import { enterDisallowedContextReadInDEV, exitDisallowedContextReadInDEV, } from './ReactFiberNewContext.new'; -import {Callback, ShouldCapture, DidCapture} from './ReactFiberFlags'; +import { + Callback, + Visibility, + ShouldCapture, + DidCapture, +} from './ReactFiberFlags'; import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags'; @@ -136,6 +141,7 @@ export type Update = {| export type SharedQueue = {| pending: Update | null, lanes: Lanes, + hiddenCallbacks: Array<() => mixed> | null, |}; export type UpdateQueue = {| @@ -143,7 +149,7 @@ export type UpdateQueue = {| firstBaseUpdate: Update | null, lastBaseUpdate: Update | null, shared: SharedQueue, - effects: Array> | null, + callbacks: Array<() => mixed> | null, |}; export const UpdateState = 0; @@ -175,8 +181,9 @@ export function initializeUpdateQueue(fiber: Fiber): void { shared: { pending: null, lanes: NoLanes, + hiddenCallbacks: null, }, - effects: null, + callbacks: null, }; fiber.updateQueue = queue; } @@ -194,7 +201,7 @@ export function cloneUpdateQueue( firstBaseUpdate: currentQueue.firstBaseUpdate, lastBaseUpdate: currentQueue.lastBaseUpdate, shared: currentQueue.shared, - effects: currentQueue.effects, + callbacks: null, }; workInProgress.updateQueue = clone; } @@ -326,7 +333,9 @@ export function enqueueCapturedUpdate( tag: update.tag, payload: update.payload, - callback: update.callback, + // When this update is rebased, we should not fire its + // callback again. + callback: null, next: null, }; @@ -355,7 +364,7 @@ export function enqueueCapturedUpdate( firstBaseUpdate: newFirst, lastBaseUpdate: newLast, shared: currentQueue.shared, - effects: currentQueue.effects, + callbacks: currentQueue.callbacks, }; workInProgress.updateQueue = queue; return; @@ -577,7 +586,10 @@ export function processUpdateQueue( tag: update.tag, payload: update.payload, - callback: update.callback, + + // When this update is rebased, we should not fire its + // callback again. + callback: null, next: null, }; @@ -594,18 +606,16 @@ export function processUpdateQueue( instance, ); const callback = update.callback; - if ( - callback !== null && - // If the update was already committed, we should not queue its - // callback again. - update.lane !== NoLane - ) { + if (callback !== null) { workInProgress.flags |= Callback; - const effects = queue.effects; - if (effects === null) { - queue.effects = [update]; + if (isHiddenUpdate) { + workInProgress.flags |= Visibility; + } + const callbacks = queue.callbacks; + if (callbacks === null) { + queue.callbacks = [callback]; } else { - effects.push(update); + callbacks.push(callback); } } } @@ -679,22 +689,51 @@ export function checkHasForceUpdateAfterProcessing(): boolean { return hasForceUpdate; } -export function commitUpdateQueue( - finishedWork: Fiber, - finishedQueue: UpdateQueue, - instance: any, +export function deferHiddenCallbacks( + updateQueue: UpdateQueue, ): void { - // Commit the effects - const effects = finishedQueue.effects; - finishedQueue.effects = null; - if (effects !== null) { - for (let i = 0; i < effects.length; i++) { - const effect = effects[i]; - const callback = effect.callback; - if (callback !== null) { - effect.callback = null; - callCallback(callback, instance); - } + // When an update finishes on a hidden component, its callback should not + // be fired until/unless the component is made visible again. Stash the + // callback on the shared queue object so it can be fired later. + const newHiddenCallbacks = updateQueue.callbacks; + if (newHiddenCallbacks !== null) { + const existingHiddenCallbacks = updateQueue.shared.hiddenCallbacks; + if (existingHiddenCallbacks === null) { + updateQueue.shared.hiddenCallbacks = newHiddenCallbacks; + } else { + updateQueue.shared.hiddenCallbacks = existingHiddenCallbacks.concat( + newHiddenCallbacks, + ); + } + } +} + +export function commitHiddenCallbacks( + updateQueue: UpdateQueue, + context: any, +): void { + // This component is switching from hidden -> visible. Commit any callbacks + // that were previously deferred. + const hiddenCallbacks = updateQueue.shared.hiddenCallbacks; + if (hiddenCallbacks !== null) { + updateQueue.shared.hiddenCallbacks = null; + for (let i = 0; i < hiddenCallbacks.length; i++) { + const callback = hiddenCallbacks[i]; + callCallback(callback, context); + } + } +} + +export function commitCallbacks( + updateQueue: UpdateQueue, + context: any, +): void { + const callbacks = updateQueue.callbacks; + if (callbacks !== null) { + updateQueue.callbacks = null; + for (let i = 0; i < callbacks.length; i++) { + const callback = callbacks[i]; + callCallback(callback, context); } } } diff --git a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.old.js b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.old.js index 3b39da9ae4181..38b43941fc1bf 100644 --- a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.old.js +++ b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.old.js @@ -102,7 +102,12 @@ import { enterDisallowedContextReadInDEV, exitDisallowedContextReadInDEV, } from './ReactFiberNewContext.old'; -import {Callback, ShouldCapture, DidCapture} from './ReactFiberFlags'; +import { + Callback, + Visibility, + ShouldCapture, + DidCapture, +} from './ReactFiberFlags'; import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags'; @@ -136,6 +141,7 @@ export type Update = {| export type SharedQueue = {| pending: Update | null, lanes: Lanes, + hiddenCallbacks: Array<() => mixed> | null, |}; export type UpdateQueue = {| @@ -143,7 +149,7 @@ export type UpdateQueue = {| firstBaseUpdate: Update | null, lastBaseUpdate: Update | null, shared: SharedQueue, - effects: Array> | null, + callbacks: Array<() => mixed> | null, |}; export const UpdateState = 0; @@ -175,8 +181,9 @@ export function initializeUpdateQueue(fiber: Fiber): void { shared: { pending: null, lanes: NoLanes, + hiddenCallbacks: null, }, - effects: null, + callbacks: null, }; fiber.updateQueue = queue; } @@ -194,7 +201,7 @@ export function cloneUpdateQueue( firstBaseUpdate: currentQueue.firstBaseUpdate, lastBaseUpdate: currentQueue.lastBaseUpdate, shared: currentQueue.shared, - effects: currentQueue.effects, + callbacks: null, }; workInProgress.updateQueue = clone; } @@ -326,7 +333,9 @@ export function enqueueCapturedUpdate( tag: update.tag, payload: update.payload, - callback: update.callback, + // When this update is rebased, we should not fire its + // callback again. + callback: null, next: null, }; @@ -355,7 +364,7 @@ export function enqueueCapturedUpdate( firstBaseUpdate: newFirst, lastBaseUpdate: newLast, shared: currentQueue.shared, - effects: currentQueue.effects, + callbacks: currentQueue.callbacks, }; workInProgress.updateQueue = queue; return; @@ -577,7 +586,10 @@ export function processUpdateQueue( tag: update.tag, payload: update.payload, - callback: update.callback, + + // When this update is rebased, we should not fire its + // callback again. + callback: null, next: null, }; @@ -594,18 +606,16 @@ export function processUpdateQueue( instance, ); const callback = update.callback; - if ( - callback !== null && - // If the update was already committed, we should not queue its - // callback again. - update.lane !== NoLane - ) { + if (callback !== null) { workInProgress.flags |= Callback; - const effects = queue.effects; - if (effects === null) { - queue.effects = [update]; + if (isHiddenUpdate) { + workInProgress.flags |= Visibility; + } + const callbacks = queue.callbacks; + if (callbacks === null) { + queue.callbacks = [callback]; } else { - effects.push(update); + callbacks.push(callback); } } } @@ -679,22 +689,51 @@ export function checkHasForceUpdateAfterProcessing(): boolean { return hasForceUpdate; } -export function commitUpdateQueue( - finishedWork: Fiber, - finishedQueue: UpdateQueue, - instance: any, +export function deferHiddenCallbacks( + updateQueue: UpdateQueue, ): void { - // Commit the effects - const effects = finishedQueue.effects; - finishedQueue.effects = null; - if (effects !== null) { - for (let i = 0; i < effects.length; i++) { - const effect = effects[i]; - const callback = effect.callback; - if (callback !== null) { - effect.callback = null; - callCallback(callback, instance); - } + // When an update finishes on a hidden component, its callback should not + // be fired until/unless the component is made visible again. Stash the + // callback on the shared queue object so it can be fired later. + const newHiddenCallbacks = updateQueue.callbacks; + if (newHiddenCallbacks !== null) { + const existingHiddenCallbacks = updateQueue.shared.hiddenCallbacks; + if (existingHiddenCallbacks === null) { + updateQueue.shared.hiddenCallbacks = newHiddenCallbacks; + } else { + updateQueue.shared.hiddenCallbacks = existingHiddenCallbacks.concat( + newHiddenCallbacks, + ); + } + } +} + +export function commitHiddenCallbacks( + updateQueue: UpdateQueue, + context: any, +): void { + // This component is switching from hidden -> visible. Commit any callbacks + // that were previously deferred. + const hiddenCallbacks = updateQueue.shared.hiddenCallbacks; + if (hiddenCallbacks !== null) { + updateQueue.shared.hiddenCallbacks = null; + for (let i = 0; i < hiddenCallbacks.length; i++) { + const callback = hiddenCallbacks[i]; + callCallback(callback, context); + } + } +} + +export function commitCallbacks( + updateQueue: UpdateQueue, + context: any, +): void { + const callbacks = updateQueue.callbacks; + if (callbacks !== null) { + updateQueue.callbacks = null; + for (let i = 0; i < callbacks.length; i++) { + const callback = callbacks[i]; + callCallback(callback, context); } } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index e5cfebea5f88c..5c46585431b99 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -75,6 +75,7 @@ import { ChildDeletion, Snapshot, Update, + Callback, Ref, Hydrating, Passive, @@ -100,7 +101,11 @@ import { startPassiveEffectTimer, } from './ReactProfilerTimer.new'; import {ConcurrentMode, NoMode, ProfileMode} from './ReactTypeOfMode'; -import {commitUpdateQueue} from './ReactFiberClassUpdateQueue.new'; +import { + deferHiddenCallbacks, + commitHiddenCallbacks, + commitCallbacks, +} from './ReactFiberClassUpdateQueue.new'; import { getPublicInstance, supportsMutation, @@ -854,7 +859,7 @@ function commitLayoutEffectOnFiber( const updateQueue: UpdateQueue< *, > | null = (finishedWork.updateQueue: any); - if (updateQueue !== null) { + if (finishedWork.flags & Callback && updateQueue !== null) { if (__DEV__) { if ( finishedWork.type === finishedWork.elementType && @@ -885,7 +890,7 @@ function commitLayoutEffectOnFiber( // We could update instance props and state here, // but instead we rely on them being set during last render. // TODO: revisit this when we implement resuming. - commitUpdateQueue(finishedWork, updateQueue, instance); + commitCallbacks(updateQueue, instance); } break; } @@ -895,7 +900,7 @@ function commitLayoutEffectOnFiber( const updateQueue: UpdateQueue< *, > | null = (finishedWork.updateQueue: any); - if (updateQueue !== null) { + if (finishedWork.flags & Callback && updateQueue !== null) { let instance = null; if (finishedWork.child !== null) { switch (finishedWork.child.tag) { @@ -907,7 +912,7 @@ function commitLayoutEffectOnFiber( break; } } - commitUpdateQueue(finishedWork, updateQueue, instance); + commitCallbacks(updateQueue, instance); } break; } @@ -1059,6 +1064,10 @@ function reappearLayoutEffectsOnFiber(node: Fiber) { safelyCallComponentDidMount(node, node.return, instance); } safelyAttachRef(node, node.return); + const updateQueue: UpdateQueue<*> | null = (node.updateQueue: any); + if (updateQueue !== null) { + commitHiddenCallbacks(updateQueue, instance); + } break; } case HostComponent: { @@ -2155,6 +2164,15 @@ function commitMutationEffectsOnFiber( safelyDetachRef(current, current.return); } } + + if (flags & Callback && offscreenSubtreeIsHidden) { + const updateQueue: UpdateQueue< + *, + > | null = (finishedWork.updateQueue: any); + if (updateQueue !== null) { + deferHiddenCallbacks(updateQueue); + } + } return; } case HostComponent: { @@ -2341,16 +2359,21 @@ function commitMutationEffectsOnFiber( return; } case OffscreenComponent: { + const newState: OffscreenState | null = finishedWork.memoizedState; + const isHidden = newState !== null; const wasHidden = current !== null && current.memoizedState !== null; if (finishedWork.mode & ConcurrentMode) { // Before committing the children, track on the stack whether this // offscreen subtree was already hidden, so that we don't unmount the // effects again. + const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden; offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden; recursivelyTraverseMutationEffects(root, finishedWork, lanes); offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; } else { recursivelyTraverseMutationEffects(root, finishedWork, lanes); } @@ -2359,8 +2382,6 @@ function commitMutationEffectsOnFiber( if (flags & Visibility) { const offscreenInstance: OffscreenInstance = finishedWork.stateNode; - const newState: OffscreenState | null = finishedWork.memoizedState; - const isHidden = newState !== null; const offscreenBoundary: Fiber = finishedWork; // Track the current state on the Offscreen instance so we can diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index eae4bc52e3585..ab279b6b05e3e 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -75,6 +75,7 @@ import { ChildDeletion, Snapshot, Update, + Callback, Ref, Hydrating, Passive, @@ -100,7 +101,11 @@ import { startPassiveEffectTimer, } from './ReactProfilerTimer.old'; import {ConcurrentMode, NoMode, ProfileMode} from './ReactTypeOfMode'; -import {commitUpdateQueue} from './ReactFiberClassUpdateQueue.old'; +import { + deferHiddenCallbacks, + commitHiddenCallbacks, + commitCallbacks, +} from './ReactFiberClassUpdateQueue.old'; import { getPublicInstance, supportsMutation, @@ -854,7 +859,7 @@ function commitLayoutEffectOnFiber( const updateQueue: UpdateQueue< *, > | null = (finishedWork.updateQueue: any); - if (updateQueue !== null) { + if (finishedWork.flags & Callback && updateQueue !== null) { if (__DEV__) { if ( finishedWork.type === finishedWork.elementType && @@ -885,7 +890,7 @@ function commitLayoutEffectOnFiber( // We could update instance props and state here, // but instead we rely on them being set during last render. // TODO: revisit this when we implement resuming. - commitUpdateQueue(finishedWork, updateQueue, instance); + commitCallbacks(updateQueue, instance); } break; } @@ -895,7 +900,7 @@ function commitLayoutEffectOnFiber( const updateQueue: UpdateQueue< *, > | null = (finishedWork.updateQueue: any); - if (updateQueue !== null) { + if (finishedWork.flags & Callback && updateQueue !== null) { let instance = null; if (finishedWork.child !== null) { switch (finishedWork.child.tag) { @@ -907,7 +912,7 @@ function commitLayoutEffectOnFiber( break; } } - commitUpdateQueue(finishedWork, updateQueue, instance); + commitCallbacks(updateQueue, instance); } break; } @@ -1059,6 +1064,10 @@ function reappearLayoutEffectsOnFiber(node: Fiber) { safelyCallComponentDidMount(node, node.return, instance); } safelyAttachRef(node, node.return); + const updateQueue: UpdateQueue<*> | null = (node.updateQueue: any); + if (updateQueue !== null) { + commitHiddenCallbacks(updateQueue, instance); + } break; } case HostComponent: { @@ -2130,6 +2139,15 @@ function commitMutationEffectsOnFiber( safelyDetachRef(current, current.return); } } + + if (flags & Callback && offscreenSubtreeIsHidden) { + const updateQueue: UpdateQueue< + *, + > | null = (finishedWork.updateQueue: any); + if (updateQueue !== null) { + deferHiddenCallbacks(updateQueue); + } + } return; } case HostComponent: { @@ -2312,16 +2330,21 @@ function commitMutationEffectsOnFiber( return; } case OffscreenComponent: { + const newState: OffscreenState | null = finishedWork.memoizedState; + const isHidden = newState !== null; const wasHidden = current !== null && current.memoizedState !== null; if (finishedWork.mode & ConcurrentMode) { // Before committing the children, track on the stack whether this // offscreen subtree was already hidden, so that we don't unmount the // effects again. + const prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden; const prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden; + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden; offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden; recursivelyTraverseMutationEffects(root, finishedWork, lanes); offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden; + offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden; } else { recursivelyTraverseMutationEffects(root, finishedWork, lanes); } @@ -2330,8 +2353,6 @@ function commitMutationEffectsOnFiber( if (flags & Visibility) { const offscreenInstance: OffscreenInstance = finishedWork.stateNode; - const newState: OffscreenState | null = finishedWork.memoizedState; - const isHidden = newState !== null; const offscreenBoundary: Fiber = finishedWork; // Track the current state on the Offscreen instance so we can diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index 589e647646753..6032d495949ee 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -640,4 +640,59 @@ describe('ReactOffscreen', () => { }); expect(root).toMatchRenderedOutput(null); }); + + it('class component setState callbacks do not fire until tree is visible', async () => { + const root = ReactNoop.createRoot(); + + let child; + class Child extends React.Component { + state = {text: 'A'}; + render() { + child = this; + return ; + } + } + + // Initial render + await act(async () => { + root.render( + + + , + ); + }); + expect(Scheduler).toHaveYielded(['A']); + expect(root).toMatchRenderedOutput(