diff --git a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js index 0311920ba2994..ae9254add8d8d 100644 --- a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js +++ b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.new.js @@ -90,8 +90,10 @@ import type {Lanes, Lane} from './ReactFiberLane.new'; import { NoLane, NoLanes, + OffscreenLane, isSubsetOfLanes, mergeLanes, + removeLanes, isTransitionLane, intersectLanes, markRootEntangled, @@ -108,6 +110,7 @@ import {StrictLegacyMode} from './ReactTypeOfMode'; import { markSkippedUpdateLanes, isUnsafeClassRenderPhaseUpdate, + getWorkInProgressRootRenderLanes, } from './ReactFiberWorkLoop.new'; import { enqueueConcurrentClassUpdate, @@ -523,9 +526,23 @@ export function processUpdateQueue( let update = firstBaseUpdate; do { - const updateLane = update.lane; + // TODO: Don't need this field anymore const updateEventTime = update.eventTime; - if (!isSubsetOfLanes(renderLanes, updateLane)) { + + // An extra OffscreenLane bit is added to updates that were made to + // a hidden tree, so that we can distinguish them from updates that were + // already there when the tree was hidden. + const updateLane = removeLanes(update.lane, OffscreenLane); + const isHiddenUpdate = updateLane !== update.lane; + + // Check if this update was made while the tree was hidden. If so, then + // it's not a "base" update and we should disregard the extra base lanes + // that were added to renderLanes when we entered the Offscreen tree. + const shouldSkipUpdate = isHiddenUpdate + ? !isSubsetOfLanes(getWorkInProgressRootRenderLanes(), updateLane) + : !isSubsetOfLanes(renderLanes, updateLane); + + if (shouldSkipUpdate) { // Priority is insufficient. Skip this update. If this is the first // skipped update, the previous update/state is the new base // update/state. diff --git a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js index ed5a02cfea9e2..ee35a11640e2f 100644 --- a/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js +++ b/packages/react-reconciler/src/ReactFiberConcurrentUpdates.new.js @@ -17,14 +17,21 @@ import type { Update as ClassUpdate, } from './ReactFiberClassUpdateQueue.new'; import type {Lane, Lanes} from './ReactFiberLane.new'; +import type {OffscreenInstance} from './ReactFiberOffscreenComponent'; import {warnAboutUpdateOnNotYetMountedFiberInDEV} from './ReactFiberWorkLoop.new'; -import {NoLane, NoLanes, mergeLanes} from './ReactFiberLane.new'; +import { + NoLane, + NoLanes, + mergeLanes, + markHiddenUpdate, +} from './ReactFiberLane.new'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; -import {HostRoot} from './ReactWorkTags'; +import {HostRoot, OffscreenComponent} from './ReactWorkTags'; -type ConcurrentUpdate = { +export type ConcurrentUpdate = { next: ConcurrentUpdate, + lane: Lane, }; type ConcurrentQueue = { @@ -38,11 +45,13 @@ type ConcurrentQueue = { const concurrentQueues: Array = []; let concurrentQueuesIndex = 0; -export function finishQueueingConcurrentUpdates(): Lanes { +let concurrentlyUpdatedLanes: Lanes = NoLanes; + +export function finishQueueingConcurrentUpdates(): void { const endIndex = concurrentQueuesIndex; concurrentQueuesIndex = 0; - let lanes = NoLanes; + concurrentlyUpdatedLanes = NoLanes; let i = 0; while (i < endIndex) { @@ -68,12 +77,13 @@ export function finishQueueingConcurrentUpdates(): Lanes { } if (lane !== NoLane) { - lanes = mergeLanes(lanes, lane); - markUpdateLaneFromFiberToRoot(fiber, lane); + markUpdateLaneFromFiberToRoot(fiber, update, lane); } } +} - return lanes; +export function getConcurrentlyUpdatedLanes(): Lanes { + return concurrentlyUpdatedLanes; } function enqueueUpdate( @@ -89,6 +99,8 @@ function enqueueUpdate( concurrentQueues[concurrentQueuesIndex++] = update; concurrentQueues[concurrentQueuesIndex++] = lane; + concurrentlyUpdatedLanes = mergeLanes(concurrentlyUpdatedLanes, lane); + // The fiber's `lane` field is used in some places to check if any work is // scheduled, to perform an eager bailout, so we need to update it immediately. // TODO: We should probably move this to the "shared" queue instead. @@ -151,11 +163,15 @@ export function unsafe_markUpdateLaneFromFiberToRoot( sourceFiber: Fiber, lane: Lane, ): FiberRoot | null { - markUpdateLaneFromFiberToRoot(sourceFiber, lane); + markUpdateLaneFromFiberToRoot(sourceFiber, null, lane); return getRootForUpdatedFiber(sourceFiber); } -function markUpdateLaneFromFiberToRoot(sourceFiber: Fiber, lane: Lane): void { +function markUpdateLaneFromFiberToRoot( + sourceFiber: Fiber, + update: ConcurrentUpdate | null, + lane: Lane, +): void { // Update the source fiber's lanes sourceFiber.lanes = mergeLanes(sourceFiber.lanes, lane); let alternate = sourceFiber.alternate; @@ -163,15 +179,31 @@ function markUpdateLaneFromFiberToRoot(sourceFiber: Fiber, lane: Lane): void { alternate.lanes = mergeLanes(alternate.lanes, lane); } // Walk the parent path to the root and update the child lanes. + let isHidden = false; let parent = sourceFiber.return; + let node = sourceFiber; while (parent !== null) { parent.childLanes = mergeLanes(parent.childLanes, lane); alternate = parent.alternate; if (alternate !== null) { alternate.childLanes = mergeLanes(alternate.childLanes, lane); } + + if (parent.tag === OffscreenComponent) { + const offscreenInstance: OffscreenInstance = parent.stateNode; + if (offscreenInstance.isHidden) { + isHidden = true; + } + } + + node = parent; parent = parent.return; } + + if (isHidden && update !== null && node.tag === HostRoot) { + const root: FiberRoot = node.stateNode; + markHiddenUpdate(root, update, lane); + } } function getRootForUpdatedFiber(sourceFiber: Fiber): FiberRoot | null { diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 697e87dd01379..c3ff781fb5f79 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -44,6 +44,7 @@ import { import { NoLane, SyncLane, + OffscreenLane, NoLanes, isSubsetOfLanes, includesBlockingLane, @@ -83,6 +84,7 @@ import { } from './ReactHookEffectTags'; import { getWorkInProgressRoot, + getWorkInProgressRootRenderLanes, scheduleUpdateOnFiber, requestUpdateLane, requestEventTime, @@ -811,8 +813,20 @@ function updateReducer( let newBaseQueueLast = null; let update = first; do { - const updateLane = update.lane; - if (!isSubsetOfLanes(renderLanes, updateLane)) { + // An extra OffscreenLane bit is added to updates that were made to + // a hidden tree, so that we can distinguish them from updates that were + // already there when the tree was hidden. + const updateLane = removeLanes(update.lane, OffscreenLane); + const isHiddenUpdate = updateLane !== update.lane; + + // Check if this update was made while the tree was hidden. If so, then + // it's not a "base" update and we should disregard the extra base lanes + // that were added to renderLanes when we entered the Offscreen tree. + const shouldSkipUpdate = isHiddenUpdate + ? !isSubsetOfLanes(getWorkInProgressRootRenderLanes(), updateLane) + : !isSubsetOfLanes(renderLanes, updateLane); + + if (shouldSkipUpdate) { // Priority is insufficient. Skip this update. If this is the first // skipped update, the previous update/state is the new base // update/state. diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 708c9318b3a98..bb76950eb9735 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -9,6 +9,7 @@ import type {FiberRoot} from './ReactInternalTypes'; import type {Transition} from './ReactFiberTracingMarkerComponent.new'; +import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates.new'; // TODO: Ideally these types would be opaque but that doesn't work well with // our reconciler fork infra, since these leak into non-reconciler packages. @@ -648,6 +649,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { const entanglements = root.entanglements; const eventTimes = root.eventTimes; const expirationTimes = root.expirationTimes; + const hiddenUpdates = root.hiddenUpdates; // Clear the lanes that no longer have pending work let lanes = noLongerPendingLanes; @@ -659,6 +661,21 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { eventTimes[index] = NoTimestamp; expirationTimes[index] = NoTimestamp; + const hiddenUpdatesForLane = hiddenUpdates[index]; + if (hiddenUpdatesForLane !== null) { + hiddenUpdates[index] = null; + // "Hidden" updates are updates that were made to a hidden component. They + // have special logic associated with them because they may be entangled + // with updates that occur outside that tree. But once the outer tree + // commits, they behave like regular updates. + for (let i = 0; i < hiddenUpdatesForLane.length; i++) { + const update = hiddenUpdatesForLane[i]; + if (update !== null) { + update.lane &= ~OffscreenLane; + } + } + } + lanes &= ~lane; } } @@ -694,6 +711,22 @@ export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) { } } +export function markHiddenUpdate( + root: FiberRoot, + update: ConcurrentUpdate, + lane: Lane, +) { + const index = laneToIndex(lane); + const hiddenUpdates = root.hiddenUpdates; + const hiddenUpdatesForLane = hiddenUpdates[index]; + if (hiddenUpdatesForLane === null) { + hiddenUpdates[index] = [update]; + } else { + hiddenUpdatesForLane.push(update); + } + update.lane = lane | OffscreenLane; +} + export function getBumpedLaneForHydration( root: FiberRoot, renderLanes: Lanes, diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index ca9edcc3ded7f..d8d2752787177 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -80,6 +80,8 @@ function FiberRootNode( this.entangledLanes = NoLanes; this.entanglements = createLaneMap(NoLanes); + this.hiddenUpdates = createLaneMap(null); + this.identifierPrefix = identifierPrefix; this.onRecoverableError = onRecoverableError; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 82963ea0fa9d1..e7e95b2694e07 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -199,6 +199,7 @@ import { import { enqueueConcurrentRenderForLane, finishQueueingConcurrentUpdates, + getConcurrentlyUpdatedLanes, } from './ReactFiberConcurrentUpdates.new'; import { @@ -425,6 +426,10 @@ export function getWorkInProgressRoot(): FiberRoot | null { return workInProgressRoot; } +export function getWorkInProgressRootRenderLanes(): Lanes { + return workInProgressRootRenderLanes; +} + export function requestEventTime() { if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { // We're inside React, so it's fine to read the actual time. @@ -2059,7 +2064,7 @@ function commitRootImpl( // Make sure to account for lanes that were updated by a concurrent event // during the render phase; don't mark them as finished. - const concurrentlyUpdatedLanes = finishQueueingConcurrentUpdates(); + const concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes(); remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes); markRootFinished(root, remainingLanes); diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 618260b47c209..c6a7ed58b0b08 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -27,6 +27,7 @@ import type {RootTag} from './ReactRootTags'; import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig'; import type {Cache} from './ReactFiberCacheComponent.old'; import type {Transition} from './ReactFiberTracingMarkerComponent.new'; +import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates.new'; // Unwind Circular: moved from ReactFiberHooks.old export type HookType = @@ -225,6 +226,7 @@ type BaseFiberRootProperties = {| callbackPriority: Lane, eventTimes: LaneMap, expirationTimes: LaneMap, + hiddenUpdates: LaneMap | null>, pendingLanes: Lanes, suspendedLanes: Lanes, diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index d1e836c252ec9..42b462100f316 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -6,6 +6,7 @@ let LegacyHidden; let Offscreen; let useState; let useLayoutEffect; +let useEffect; describe('ReactOffscreen', () => { beforeEach(() => { @@ -19,6 +20,7 @@ describe('ReactOffscreen', () => { Offscreen = React.unstable_Offscreen; useState = React.useState; useLayoutEffect = React.useLayoutEffect; + useEffect = React.useEffect; }); function Text(props) { @@ -473,4 +475,124 @@ describe('ReactOffscreen', () => { // Now it's visible expect(root).toMatchRenderedOutput(Hi); }); + + // Only works in new reconciler + // @gate variant + it('revealing a hidden tree at high priority does not cause tearing', async () => { + // When revealing an offscreen tree, we need to include updates that were + // previously deferred because the tree was hidden, even if they are lower + // priority than the current render. However, we should *not* include low + // priority updates that are entangled with updates outside of the hidden + // tree, because that can cause tearing. + // + // This test covers a scenario where an update multiple updates inside a + // hidden tree share the same lane, but are processed at different times + // because of the timing of when they were scheduled. + + let setInner; + function Child({outer}) { + 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]); + + return ; + } + + let setOuter; + function App({show}) { + const [outer, _setOuter] = useState(0); + setOuter = _setOuter; + return ( + <> + + + + + + ); + } + + // Render a hidden tree + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([ + 'Outer: 0', + 'Inner: 0', + 'Inner and outer are consistent', + ]); + expect(root).toMatchRenderedOutput( + <> + +