diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 88c4d1946cb6d..afa54b7b7b117 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -36,10 +36,7 @@ export type Lane = number; export type LaneMap = Array; import invariant from 'shared/invariant'; -import { - enableCache, - enableTransitionEntanglement, -} from 'shared/ReactFeatureFlags'; +import {enableCache} from 'shared/ReactFeatureFlags'; import { ImmediatePriority as ImmediateSchedulerPriority, @@ -95,6 +92,7 @@ export const DefaultLanes: Lanes = /* */ 0b0000000000000000000 const TransitionHydrationLane: Lane = /* */ 0b0000000000000000001000000000000; const TransitionLanes: Lanes = /* */ 0b0000000001111111110000000000000; +const SomeTransitionLane: Lane = /* */ 0b0000000000000000010000000000000; const RetryLanes: Lanes = /* */ 0b0000011110000000000000000000000; @@ -113,6 +111,9 @@ export const NoTimestamp = -1; let currentUpdateLanePriority: LanePriority = NoLanePriority; +let nextTransitionLane: Lane = SomeTransitionLane; +let nextRetryLane: Lane = SomeRetryLane; + export function getCurrentUpdateLanePriority(): LanePriority { return currentUpdateLanePriority; } @@ -309,15 +310,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { return NoLanes; } - if (enableTransitionEntanglement) { - // We don't need to do anything extra here, because we apply per-lane - // transition entanglement in the entanglement loop below. - } else { - // If there are higher priority lanes, we'll include them even if they - // are suspended. - nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes); - } - // If we're already in the middle of a render, switching lanes will interrupt // it and we'll lose our progress. We should only do this if the new lanes are // higher priority. @@ -350,6 +342,11 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { // entanglement is usually "best effort": we'll try our best to render the // lanes in the same batch, but it's not worth throwing out partially // completed work in order to do it. + // TODO: Reconsider this. The counter-argument is that the partial work + // represents an intermediate state, which we don't want to show to the user. + // And by spending extra time finishing it, we're increasing the amount of + // time it takes to show the final state, which is what they are actually + // waiting for. // // For those exceptions where entanglement is semantically important, like // useMutableSource, we should ensure that there is no partial work at the @@ -559,34 +556,23 @@ export function findUpdateLane( ); } -// To ensure consistency across multiple updates in the same event, this should -// be pure function, so that it always returns the same lane for given inputs. -export function findTransitionLane(wipLanes: Lanes, pendingLanes: Lanes): Lane { - // First look for lanes that are completely unclaimed, i.e. have no - // pending work. - let lane = pickArbitraryLane(TransitionLanes & ~pendingLanes); - if (lane === NoLane) { - // If all lanes have pending work, look for a lane that isn't currently - // being worked on. - lane = pickArbitraryLane(TransitionLanes & ~wipLanes); - if (lane === NoLane) { - // If everything is being worked on, pick any lane. This has the - // effect of interrupting the current work-in-progress. - lane = pickArbitraryLane(TransitionLanes); - } +export function claimNextTransitionLane(): Lane { + // Cycle through the lanes, assigning each new transition to the next lane. + // In most cases, this means every transition gets its own lane, until we + // run out of lanes and cycle back to the beginning. + const lane = nextTransitionLane; + nextTransitionLane <<= 1; + if ((nextTransitionLane & TransitionLanes) === 0) { + nextTransitionLane = SomeTransitionLane; } return lane; } -// To ensure consistency across multiple updates in the same event, this should -// be pure function, so that it always returns the same lane for given inputs. -export function findRetryLane(wipLanes: Lanes): Lane { - // This is a fork of `findUpdateLane` designed specifically for Suspense - // "retries" — a special update that attempts to flip a Suspense boundary - // from its placeholder state to its primary/resolved state. - let lane = pickArbitraryLane(RetryLanes & ~wipLanes); - if (lane === NoLane) { - lane = pickArbitraryLane(RetryLanes); +export function claimNextRetryLane(): Lane { + const lane = nextRetryLane; + nextRetryLane <<= 1; + if ((nextRetryLane & RetryLanes) === 0) { + nextRetryLane = SomeRetryLane; } return lane; } @@ -595,16 +581,6 @@ function getHighestPriorityLane(lanes: Lanes) { return lanes & -lanes; } -function getLowestPriorityLane(lanes: Lanes): Lane { - // This finds the most significant non-zero bit. - const index = 31 - clz32(lanes); - return index < 0 ? NoLanes : 1 << index; -} - -function getEqualOrHigherPriorityLanes(lanes: Lanes | Lane): Lanes { - return (getLowestPriorityLane(lanes) << 1) - 1; -} - export function pickArbitraryLane(lanes: Lanes): Lane { // This wrapper function gets inlined. Only exists so to communicate that it // doesn't matter which bit is selected; you can pick any bit without @@ -676,39 +652,21 @@ export function markRootUpdated( ) { root.pendingLanes |= updateLane; - // TODO: Theoretically, any update to any lane can unblock any other lane. But - // it's not practical to try every single possible combination. We need a - // heuristic to decide which lanes to attempt to render, and in which batches. - // For now, we use the same heuristic as in the old ExpirationTimes model: - // retry any lane at equal or lower priority, but don't try updates at higher - // priority without also including the lower priority updates. This works well - // when considering updates across different priority levels, but isn't - // sufficient for updates within the same priority, since we want to treat - // those updates as parallel. - - // Unsuspend any update at equal or lower priority. - const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111 - - if (enableTransitionEntanglement) { - // If there are any suspended transitions, it's possible this new update - // could unblock them. Clear the suspended lanes so that we can try rendering - // them again. - // - // TODO: We really only need to unsuspend only lanes that are in the - // `subtreeLanes` of the updated fiber, or the update lanes of the return - // path. This would exclude suspended updates in an unrelated sibling tree, - // since there's no way for this update to unblock it. - // - // We don't do this if the incoming update is idle, because we never process - // idle updates until after all the regular updates have finished; there's no - // way it could unblock a transition. - if ((updateLane & IdleLanes) === NoLanes) { - root.suspendedLanes = NoLanes; - root.pingedLanes = NoLanes; - } - } else { - root.suspendedLanes &= higherPriorityLanes; - root.pingedLanes &= higherPriorityLanes; + // If there are any suspended transitions, it's possible this new update + // could unblock them. Clear the suspended lanes so that we can try rendering + // them again. + // + // TODO: We really only need to unsuspend only lanes that are in the + // `subtreeLanes` of the updated fiber, or the update lanes of the return + // path. This would exclude suspended updates in an unrelated sibling tree, + // since there's no way for this update to unblock it. + // + // We don't do this if the incoming update is idle, because we never process + // idle updates until after all the regular updates have finished; there's no + // way it could unblock a transition. + if ((updateLane & IdleLanes) === NoLanes) { + root.suspendedLanes = NoLanes; + root.pingedLanes = NoLanes; } const eventTimes = root.eventTimes; @@ -801,16 +759,32 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { } export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) { - root.entangledLanes |= entangledLanes; + // In addition to entangling each of the given lanes with each other, we also + // have to consider _transitive_ entanglements. For each lane that is already + // entangled with *any* of the given lanes, that lane is now transitively + // entangled with *all* the given lanes. + // + // Translated: If C is entangled with A, then entangling A with B also + // entangles C with B. + // + // If this is hard to grasp, it might help to intentionally break this + // function and look at the tests that fail in ReactTransition-test.js. Try + // commenting out one of the conditions below. + const rootEntangledLanes = (root.entangledLanes |= entangledLanes); const entanglements = root.entanglements; - let lanes = entangledLanes; - while (lanes > 0) { + let lanes = rootEntangledLanes; + while (lanes) { const index = pickArbitraryLaneIndex(lanes); const lane = 1 << index; - - entanglements[index] |= entangledLanes; - + if ( + // Is this one of the newly entangled lanes? + (lane & entangledLanes) | + // Is this lane transitively entangled with the newly entangled lanes? + (entanglements[index] & entangledLanes) + ) { + entanglements[index] |= entangledLanes; + } lanes &= ~lane; } } diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index a083ae68d8a8e..3a0cdc3af2b6b 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -36,10 +36,7 @@ export type Lane = number; export type LaneMap = Array; import invariant from 'shared/invariant'; -import { - enableCache, - enableTransitionEntanglement, -} from 'shared/ReactFeatureFlags'; +import {enableCache} from 'shared/ReactFeatureFlags'; import { ImmediatePriority as ImmediateSchedulerPriority, @@ -95,6 +92,7 @@ export const DefaultLanes: Lanes = /* */ 0b0000000000000000000 const TransitionHydrationLane: Lane = /* */ 0b0000000000000000001000000000000; const TransitionLanes: Lanes = /* */ 0b0000000001111111110000000000000; +const SomeTransitionLane: Lane = /* */ 0b0000000000000000010000000000000; const RetryLanes: Lanes = /* */ 0b0000011110000000000000000000000; @@ -113,6 +111,9 @@ export const NoTimestamp = -1; let currentUpdateLanePriority: LanePriority = NoLanePriority; +let nextTransitionLane: Lane = SomeTransitionLane; +let nextRetryLane: Lane = SomeRetryLane; + export function getCurrentUpdateLanePriority(): LanePriority { return currentUpdateLanePriority; } @@ -309,15 +310,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { return NoLanes; } - if (enableTransitionEntanglement) { - // We don't need to do anything extra here, because we apply per-lane - // transition entanglement in the entanglement loop below. - } else { - // If there are higher priority lanes, we'll include them even if they - // are suspended. - nextLanes = pendingLanes & getEqualOrHigherPriorityLanes(nextLanes); - } - // If we're already in the middle of a render, switching lanes will interrupt // it and we'll lose our progress. We should only do this if the new lanes are // higher priority. @@ -350,6 +342,11 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { // entanglement is usually "best effort": we'll try our best to render the // lanes in the same batch, but it's not worth throwing out partially // completed work in order to do it. + // TODO: Reconsider this. The counter-argument is that the partial work + // represents an intermediate state, which we don't want to show to the user. + // And by spending extra time finishing it, we're increasing the amount of + // time it takes to show the final state, which is what they are actually + // waiting for. // // For those exceptions where entanglement is semantically important, like // useMutableSource, we should ensure that there is no partial work at the @@ -559,34 +556,23 @@ export function findUpdateLane( ); } -// To ensure consistency across multiple updates in the same event, this should -// be pure function, so that it always returns the same lane for given inputs. -export function findTransitionLane(wipLanes: Lanes, pendingLanes: Lanes): Lane { - // First look for lanes that are completely unclaimed, i.e. have no - // pending work. - let lane = pickArbitraryLane(TransitionLanes & ~pendingLanes); - if (lane === NoLane) { - // If all lanes have pending work, look for a lane that isn't currently - // being worked on. - lane = pickArbitraryLane(TransitionLanes & ~wipLanes); - if (lane === NoLane) { - // If everything is being worked on, pick any lane. This has the - // effect of interrupting the current work-in-progress. - lane = pickArbitraryLane(TransitionLanes); - } +export function claimNextTransitionLane(): Lane { + // Cycle through the lanes, assigning each new transition to the next lane. + // In most cases, this means every transition gets its own lane, until we + // run out of lanes and cycle back to the beginning. + const lane = nextTransitionLane; + nextTransitionLane <<= 1; + if ((nextTransitionLane & TransitionLanes) === 0) { + nextTransitionLane = SomeTransitionLane; } return lane; } -// To ensure consistency across multiple updates in the same event, this should -// be pure function, so that it always returns the same lane for given inputs. -export function findRetryLane(wipLanes: Lanes): Lane { - // This is a fork of `findUpdateLane` designed specifically for Suspense - // "retries" — a special update that attempts to flip a Suspense boundary - // from its placeholder state to its primary/resolved state. - let lane = pickArbitraryLane(RetryLanes & ~wipLanes); - if (lane === NoLane) { - lane = pickArbitraryLane(RetryLanes); +export function claimNextRetryLane(): Lane { + const lane = nextRetryLane; + nextRetryLane <<= 1; + if ((nextRetryLane & RetryLanes) === 0) { + nextRetryLane = SomeRetryLane; } return lane; } @@ -595,16 +581,6 @@ function getHighestPriorityLane(lanes: Lanes) { return lanes & -lanes; } -function getLowestPriorityLane(lanes: Lanes): Lane { - // This finds the most significant non-zero bit. - const index = 31 - clz32(lanes); - return index < 0 ? NoLanes : 1 << index; -} - -function getEqualOrHigherPriorityLanes(lanes: Lanes | Lane): Lanes { - return (getLowestPriorityLane(lanes) << 1) - 1; -} - export function pickArbitraryLane(lanes: Lanes): Lane { // This wrapper function gets inlined. Only exists so to communicate that it // doesn't matter which bit is selected; you can pick any bit without @@ -676,39 +652,21 @@ export function markRootUpdated( ) { root.pendingLanes |= updateLane; - // TODO: Theoretically, any update to any lane can unblock any other lane. But - // it's not practical to try every single possible combination. We need a - // heuristic to decide which lanes to attempt to render, and in which batches. - // For now, we use the same heuristic as in the old ExpirationTimes model: - // retry any lane at equal or lower priority, but don't try updates at higher - // priority without also including the lower priority updates. This works well - // when considering updates across different priority levels, but isn't - // sufficient for updates within the same priority, since we want to treat - // those updates as parallel. - - // Unsuspend any update at equal or lower priority. - const higherPriorityLanes = updateLane - 1; // Turns 0b1000 into 0b0111 - - if (enableTransitionEntanglement) { - // If there are any suspended transitions, it's possible this new update - // could unblock them. Clear the suspended lanes so that we can try rendering - // them again. - // - // TODO: We really only need to unsuspend only lanes that are in the - // `subtreeLanes` of the updated fiber, or the update lanes of the return - // path. This would exclude suspended updates in an unrelated sibling tree, - // since there's no way for this update to unblock it. - // - // We don't do this if the incoming update is idle, because we never process - // idle updates until after all the regular updates have finished; there's no - // way it could unblock a transition. - if ((updateLane & IdleLanes) === NoLanes) { - root.suspendedLanes = NoLanes; - root.pingedLanes = NoLanes; - } - } else { - root.suspendedLanes &= higherPriorityLanes; - root.pingedLanes &= higherPriorityLanes; + // If there are any suspended transitions, it's possible this new update + // could unblock them. Clear the suspended lanes so that we can try rendering + // them again. + // + // TODO: We really only need to unsuspend only lanes that are in the + // `subtreeLanes` of the updated fiber, or the update lanes of the return + // path. This would exclude suspended updates in an unrelated sibling tree, + // since there's no way for this update to unblock it. + // + // We don't do this if the incoming update is idle, because we never process + // idle updates until after all the regular updates have finished; there's no + // way it could unblock a transition. + if ((updateLane & IdleLanes) === NoLanes) { + root.suspendedLanes = NoLanes; + root.pingedLanes = NoLanes; } const eventTimes = root.eventTimes; @@ -801,16 +759,32 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) { } export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) { - root.entangledLanes |= entangledLanes; + // In addition to entangling each of the given lanes with each other, we also + // have to consider _transitive_ entanglements. For each lane that is already + // entangled with *any* of the given lanes, that lane is now transitively + // entangled with *all* the given lanes. + // + // Translated: If C is entangled with A, then entangling A with B also + // entangles C with B. + // + // If this is hard to grasp, it might help to intentionally break this + // function and look at the tests that fail in ReactTransition-test.js. Try + // commenting out one of the conditions below. + const rootEntangledLanes = (root.entangledLanes |= entangledLanes); const entanglements = root.entanglements; - let lanes = entangledLanes; - while (lanes > 0) { + let lanes = rootEntangledLanes; + while (lanes) { const index = pickArbitraryLaneIndex(lanes); const lane = 1 << index; - - entanglements[index] |= entangledLanes; - + if ( + // Is this one of the newly entangled lanes? + (lane & entangledLanes) | + // Is this lane transitively entangled with the newly entangled lanes? + (entanglements[index] & entangledLanes) + ) { + entanglements[index] |= entangledLanes; + } lanes &= ~lane; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 7513d21e27b1a..f148e2bd46b9d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -145,8 +145,8 @@ import { SyncBatchedLane, NoTimestamp, findUpdateLane, - findTransitionLane, - findRetryLane, + claimNextTransitionLane, + claimNextRetryLane, includesSomeLane, isSubsetOfLanes, mergeLanes, @@ -302,8 +302,6 @@ let workInProgressRootUpdatedLanes: Lanes = NoLanes; // Lanes that were pinged (in an interleaved event) during this render. let workInProgressRootPingedLanes: Lanes = NoLanes; -let mostRecentlyUpdatedRoot: FiberRoot | null = null; - // The most recent time we committed a fallback. This lets us ensure a train // model where we don't commit new loading states in too quick succession. let globalMostRecentFallbackTime: number = 0; @@ -360,7 +358,7 @@ let spawnedWorkDuringRender: null | Array = null; // between the first and second call. let currentEventTime: number = NoTimestamp; let currentEventWipLanes: Lanes = NoLanes; -let currentEventPendingLanes: Lanes = NoLanes; +let currentEventTransitionLane: Lanes = NoLanes; // Dev only flag that tracks if passive effects are currently being flushed. // We warn about state updates for unmounted components differently in this case. @@ -428,20 +426,17 @@ export function requestUpdateLane(fiber: Fiber): Lane { // event. Then reset the cached values once we can be sure the event is over. // Our heuristic for that is whenever we enter a concurrent work loop. // - // We'll do the same for `currentEventPendingLanes` below. + // We'll do the same for `currentEventTransitionLane` below. if (currentEventWipLanes === NoLanes) { currentEventWipLanes = workInProgressRootIncludedLanes; } const isTransition = requestCurrentTransition() !== NoTransition; if (isTransition) { - if (currentEventPendingLanes !== NoLanes) { - currentEventPendingLanes = - mostRecentlyUpdatedRoot !== null - ? mostRecentlyUpdatedRoot.pendingLanes - : NoLanes; + if (currentEventTransitionLane === NoLane) { + currentEventTransitionLane = claimNextTransitionLane(); } - return findTransitionLane(currentEventWipLanes, currentEventPendingLanes); + return currentEventTransitionLane; } // TODO: Remove this dependency on the Scheduler priority. @@ -494,7 +489,8 @@ function requestRetryLane(fiber: Fiber) { if (currentEventWipLanes === NoLanes) { currentEventWipLanes = workInProgressRootIncludedLanes; } - return findRetryLane(currentEventWipLanes); + + return claimNextRetryLane(); } export function scheduleUpdateOnFiber( @@ -618,13 +614,6 @@ export function scheduleUpdateOnFiber( schedulePendingInteractions(root, lane); } - // We use this when assigning a lane for a transition inside - // `requestUpdateLane`. We assume it's the same as the root being updated, - // since in the common case of a single root app it probably is. If it's not - // the same root, then it's not a huge deal, we just might batch more stuff - // together more than necessary. - mostRecentlyUpdatedRoot = root; - return root; } @@ -793,7 +782,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // event time. The next update will compute a new event time. currentEventTime = NoTimestamp; currentEventWipLanes = NoLanes; - currentEventPendingLanes = NoLanes; + currentEventTransitionLane = NoLanes; invariant( (executionContext & (RenderContext | CommitContext)) === NoContext, @@ -2461,6 +2450,8 @@ function retryTimedOutBoundary(boundaryFiber: Fiber, retryLane: Lane) { // suspended it has resolved, which means at least part of the tree was // likely unblocked. Try rendering again, at a new expiration time. if (retryLane === NoLane) { + // TODO: Assign this to `suspenseState.retryLane`? to avoid + // unnecessary entanglement? retryLane = requestRetryLane(boundaryFiber); } // TODO: Special case idle priority? diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 5f372831d741a..3589bff2c6be3 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -145,8 +145,8 @@ import { SyncBatchedLane, NoTimestamp, findUpdateLane, - findTransitionLane, - findRetryLane, + claimNextTransitionLane, + claimNextRetryLane, includesSomeLane, isSubsetOfLanes, mergeLanes, @@ -302,8 +302,6 @@ let workInProgressRootUpdatedLanes: Lanes = NoLanes; // Lanes that were pinged (in an interleaved event) during this render. let workInProgressRootPingedLanes: Lanes = NoLanes; -let mostRecentlyUpdatedRoot: FiberRoot | null = null; - // The most recent time we committed a fallback. This lets us ensure a train // model where we don't commit new loading states in too quick succession. let globalMostRecentFallbackTime: number = 0; @@ -360,7 +358,7 @@ let spawnedWorkDuringRender: null | Array = null; // between the first and second call. let currentEventTime: number = NoTimestamp; let currentEventWipLanes: Lanes = NoLanes; -let currentEventPendingLanes: Lanes = NoLanes; +let currentEventTransitionLane: Lanes = NoLanes; // Dev only flag that tracks if passive effects are currently being flushed. // We warn about state updates for unmounted components differently in this case. @@ -428,20 +426,17 @@ export function requestUpdateLane(fiber: Fiber): Lane { // event. Then reset the cached values once we can be sure the event is over. // Our heuristic for that is whenever we enter a concurrent work loop. // - // We'll do the same for `currentEventPendingLanes` below. + // We'll do the same for `currentEventTransitionLane` below. if (currentEventWipLanes === NoLanes) { currentEventWipLanes = workInProgressRootIncludedLanes; } const isTransition = requestCurrentTransition() !== NoTransition; if (isTransition) { - if (currentEventPendingLanes !== NoLanes) { - currentEventPendingLanes = - mostRecentlyUpdatedRoot !== null - ? mostRecentlyUpdatedRoot.pendingLanes - : NoLanes; + if (currentEventTransitionLane === NoLane) { + currentEventTransitionLane = claimNextTransitionLane(); } - return findTransitionLane(currentEventWipLanes, currentEventPendingLanes); + return currentEventTransitionLane; } // TODO: Remove this dependency on the Scheduler priority. @@ -494,7 +489,8 @@ function requestRetryLane(fiber: Fiber) { if (currentEventWipLanes === NoLanes) { currentEventWipLanes = workInProgressRootIncludedLanes; } - return findRetryLane(currentEventWipLanes); + + return claimNextRetryLane(); } export function scheduleUpdateOnFiber( @@ -618,13 +614,6 @@ export function scheduleUpdateOnFiber( schedulePendingInteractions(root, lane); } - // We use this when assigning a lane for a transition inside - // `requestUpdateLane`. We assume it's the same as the root being updated, - // since in the common case of a single root app it probably is. If it's not - // the same root, then it's not a huge deal, we just might batch more stuff - // together more than necessary. - mostRecentlyUpdatedRoot = root; - return root; } @@ -793,7 +782,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // event time. The next update will compute a new event time. currentEventTime = NoTimestamp; currentEventWipLanes = NoLanes; - currentEventPendingLanes = NoLanes; + currentEventTransitionLane = NoLanes; invariant( (executionContext & (RenderContext | CommitContext)) === NoContext, @@ -2461,6 +2450,8 @@ function retryTimedOutBoundary(boundaryFiber: Fiber, retryLane: Lane) { // suspended it has resolved, which means at least part of the tree was // likely unblocked. Try rendering again, at a new expiration time. if (retryLane === NoLane) { + // TODO: Assign this to `suspenseState.retryLane`? to avoid + // unnecessary entanglement? retryLane = requestRetryLane(boundaryFiber); } // TODO: Special case idle priority? diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index fd224b2a1f813..c416d2404856c 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -733,19 +733,10 @@ describe('ReactExpiration', () => { // Both normal pri updates should have expired. expect(Scheduler).toFlushExpired([ 'Sibling', - gate(flags => flags.enableTransitionEntanglement) - ? // Notice that the high pri update didn't flush yet. Expiring one lane - // doesn't affect other lanes. (Unless they are intentionally - // entangled, like we do for overlapping transitions that affect the - // same state.) - 'High pri: 0' - : // In the current implementation, once we pick the next lanes to work - // on, we entangle it with all pending at equal or higher priority. - // We could feasibly change this heuristic so that the high pri - // update doesn't render until after the expired updates have - // finished. But the important thing in this test is that the normal - // updates expired. - 'High pri: 1', + // Notice that the high pri update didn't flush yet. Expiring one lane + // doesn't affect other lanes. (Unless they are intentionally entangled, + // like we do for overlapping transitions that affect the same state.) + 'High pri: 0', 'Normal pri: 2', 'Sibling', ]); diff --git a/packages/react-reconciler/src/__tests__/ReactTransition-test.js b/packages/react-reconciler/src/__tests__/ReactTransition-test.js index 1b25cf7dd1879..0b9084356c710 100644 --- a/packages/react-reconciler/src/__tests__/ReactTransition-test.js +++ b/packages/react-reconciler/src/__tests__/ReactTransition-test.js @@ -16,7 +16,12 @@ let Scheduler; let Suspense; let useState; let useTransition; +let startTransition; let act; +let getCacheForType; + +let caches; +let seededCache; describe('ReactTransition', () => { beforeEach(() => { @@ -27,44 +32,142 @@ describe('ReactTransition', () => { useState = React.useState; useTransition = React.unstable_useTransition; Suspense = React.Suspense; + startTransition = React.unstable_startTransition; + getCacheForType = React.unstable_getCacheForType; act = ReactNoop.act; + + caches = []; + seededCache = null; }); - function Text(props) { - Scheduler.unstable_yieldValue(props.text); - return props.text; + function createTextCache() { + if (seededCache !== null) { + // Trick to seed a cache before it exists. + // TODO: Need a built-in API to seed data before the initial render (i.e. + // not a refresh because nothing has mounted yet). + const cache = seededCache; + seededCache = null; + return cache; + } + + const data = new Map(); + const version = caches.length + 1; + const cache = { + version, + data, + resolve(text) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'resolved', + value: text, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'resolved'; + record.value = text; + thenable.pings.forEach(t => t()); + } + }, + reject(text, error) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'rejected', + value: error, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'rejected'; + record.value = error; + thenable.pings.forEach(t => t()); + } + }, + }; + caches.push(cache); + return cache; } - function createAsyncText(text) { - let resolved = false; - const Component = function() { - if (!resolved) { - Scheduler.unstable_yieldValue('Suspend! [' + text + ']'); - throw promise; + function readText(text) { + const textCache = getCacheForType(createTextCache); + const record = textCache.data.get(text); + if (record !== undefined) { + switch (record.status) { + case 'pending': + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + throw record.value; + case 'rejected': + Scheduler.unstable_yieldValue(`Error! [${text}]`); + throw record.value; + case 'resolved': + return textCache.version; } - return ; - }; - const promise = new Promise(resolve => { - Component.resolve = function() { - resolved = true; - return resolve(); + } else { + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + + const thenable = { + pings: [], + then(resolve) { + if (newRecord.status === 'pending') { + thenable.pings.push(resolve); + } else { + Promise.resolve().then(() => resolve(newRecord.value)); + } + }, }; - }); - return Component; + + const newRecord = { + status: 'pending', + value: thenable, + }; + textCache.data.set(text, newRecord); + + throw thenable; + } + } + + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return text; + } + + function AsyncText({text}) { + readText(text); + Scheduler.unstable_yieldValue(text); + return text; + } + + function seedNextTextCache(text) { + if (seededCache === null) { + seededCache = createTextCache(); + } + seededCache.resolve(text); + } + + function resolveText(text) { + if (caches.length === 0) { + throw Error('Cache does not exist.'); + } else { + // Resolve the most recently created cache. An older cache can by + // resolved with `caches[index].resolve(text)`. + caches[caches.length - 1].resolve(text); + } } // @gate experimental - it('isPending works even if called from outside an input event', async () => { - const Async = createAsyncText('Async'); + // @gate enableCache + test('isPending works even if called from outside an input event', async () => { let start; function App() { const [show, setShow] = useState(false); - const [startTransition, isPending] = useTransition(); - start = () => startTransition(() => setShow(true)); + const [_start, isPending] = useTransition(); + start = () => _start(() => setShow(true)); return ( }> {isPending ? : null} - {show ? : } + {show ? : } ); } @@ -89,9 +192,360 @@ describe('ReactTransition', () => { expect(root).toMatchRenderedOutput('Pending...(empty)'); - await Async.resolve(); + await resolveText('Async'); }); expect(Scheduler).toHaveYielded(['Async']); expect(root).toMatchRenderedOutput('Async'); }); + + // @gate experimental + // @gate enableCache + test( + 'when multiple transitions update the same queue, only the most recent ' + + 'one is allowed to finish (no intermediate states)', + async () => { + let update; + function App() { + const [startContentChange, isContentPending] = useTransition(); + const [label, setLabel] = useState('A'); + const [contents, setContents] = useState('A'); + update = value => { + ReactNoop.discreteUpdates(() => { + setLabel(value); + startContentChange(() => { + setContents(value); + }); + }); + }; + return ( + <> + +
+ }> + + +
+ + ); + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + seedNextTextCache('A content'); + root.render(); + }); + expect(Scheduler).toHaveYielded(['A label', 'A content']); + expect(root).toMatchRenderedOutput( + <> + A label
A content
+ , + ); + + // Switch to B + await act(async () => { + update('B'); + }); + expect(Scheduler).toHaveYielded([ + // Commit pending state + 'B label (loading...)', + 'A content', + + // Attempt to render B, but it suspends + 'B label', + 'Suspend! [B content]', + 'Loading...', + ]); + // This is a refresh transition so it shouldn't show a fallback + expect(root).toMatchRenderedOutput( + <> + B label (loading...)
A content
+ , + ); + + // Before B finishes loading, switch to C + await act(async () => { + update('C'); + }); + expect(Scheduler).toHaveYielded([ + // Commit pending state + 'C label (loading...)', + 'A content', + + // Attempt to render C, but it suspends + 'C label', + 'Suspend! [C content]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> + C label (loading...)
A content
+ , + ); + + // Finish loading B. But we're not allowed to render B because it's + // entangled with C. So we're still pending. + await act(async () => { + resolveText('B content'); + }); + expect(Scheduler).toHaveYielded([ + // Attempt to render C, but it suspends + 'C label', + 'Suspend! [C content]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> + C label (loading...)
A content
+ , + ); + + // Now finish loading C. This is the terminal update, so it can finish. + await act(async () => { + resolveText('C content'); + }); + expect(Scheduler).toHaveYielded(['C label', 'C content']); + expect(root).toMatchRenderedOutput( + <> + C label
C content
+ , + ); + }, + ); + + // Same as previous test, but for class update queue. + // @gate experimental + // @gate enableCache + test( + 'when multiple transitions update the same queue, only the most recent ' + + 'one is allowed to finish (no intermediate states) (classes)', + async () => { + let update; + class App extends React.Component { + state = { + label: 'A', + contents: 'A', + }; + render() { + update = value => { + ReactNoop.discreteUpdates(() => { + this.setState({label: value}); + startTransition(() => { + this.setState({contents: value}); + }); + }); + }; + const label = this.state.label; + const contents = this.state.contents; + const isContentPending = label !== contents; + return ( + <> + +
+ }> + + +
+ + ); + } + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + seedNextTextCache('A content'); + root.render(); + }); + expect(Scheduler).toHaveYielded(['A label', 'A content']); + expect(root).toMatchRenderedOutput( + <> + A label
A content
+ , + ); + + // Switch to B + await act(async () => { + update('B'); + }); + expect(Scheduler).toHaveYielded([ + // Commit pending state + 'B label (loading...)', + 'A content', + + // Attempt to render B, but it suspends + 'B label', + 'Suspend! [B content]', + 'Loading...', + ]); + // This is a refresh transition so it shouldn't show a fallback + expect(root).toMatchRenderedOutput( + <> + B label (loading...)
A content
+ , + ); + + // Before B finishes loading, switch to C + await act(async () => { + update('C'); + }); + expect(Scheduler).toHaveYielded([ + // Commit pending state + 'C label (loading...)', + 'A content', + + // Attempt to render C, but it suspends + 'C label', + 'Suspend! [C content]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> + C label (loading...)
A content
+ , + ); + + // Finish loading B. But we're not allowed to render B because it's + // entangled with C. So we're still pending. + await act(async () => { + resolveText('B content'); + }); + expect(Scheduler).toHaveYielded([ + // Attempt to render C, but it suspends + 'C label', + 'Suspend! [C content]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput( + <> + C label (loading...)
A content
+ , + ); + + // Now finish loading C. This is the terminal update, so it can finish. + await act(async () => { + resolveText('C content'); + }); + expect(Scheduler).toHaveYielded(['C label', 'C content']); + expect(root).toMatchRenderedOutput( + <> + C label
C content
+ , + ); + }, + ); + + // @gate experimental + // @gate enableCache + test( + 'when multiple transitions update overlapping queues, all the transitions ' + + 'across all the queues are entangled', + async () => { + let setShowA; + let setShowB; + let setShowC; + function App() { + const [showA, _setShowA] = useState(false); + const [showB, _setShowB] = useState(false); + const [showC, _setShowC] = useState(false); + setShowA = _setShowA; + setShowB = _setShowB; + setShowC = _setShowC; + + // Only one of these children should be visible at a time. Except + // instead of being modeled as a single state, it's three separate + // states that are updated simultaneously. This may seem a bit + // contrived, but it's more common than you might think. Usually via + // a framework or indirection. For example, consider a tooltip manager + // that only shows a single tooltip at a time. Or a router that + // highlights links to the active route. + return ( + <> + }> + {showA ? : null} + {showB ? : null} + {showC ? : null} + + + ); + } + + // Initial render. Start with all children hidden. + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([]); + expect(root).toMatchRenderedOutput(null); + + // Switch to A. + await act(async () => { + startTransition(() => { + setShowA(true); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [A]', 'Loading...']); + expect(root).toMatchRenderedOutput(null); + + // Before A loads, switch to B. This should entangle A with B. + await act(async () => { + startTransition(() => { + setShowA(false); + setShowB(true); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [B]', 'Loading...']); + expect(root).toMatchRenderedOutput(null); + + // Before A or B loads, switch to C. This should entangle C with B, and + // transitively entangle C with A. + await act(async () => { + startTransition(() => { + setShowB(false); + setShowC(true); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [C]', 'Loading...']); + expect(root).toMatchRenderedOutput(null); + + // Now the data starts resolving out of order. + + // First resolve B. This will attempt to render C, since everything is + // entangled. + await act(async () => { + startTransition(() => { + resolveText('B'); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [C]', 'Loading...']); + expect(root).toMatchRenderedOutput(null); + + // Now resolve A. Again, this will attempt to render C, since everything + // is entangled. + await act(async () => { + startTransition(() => { + resolveText('A'); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [C]', 'Loading...']); + expect(root).toMatchRenderedOutput(null); + + // Finally, resolve C. This time we can finish. + await act(async () => { + startTransition(() => { + resolveText('C'); + }); + }); + expect(Scheduler).toHaveYielded(['C']); + expect(root).toMatchRenderedOutput('C'); + }, + ); });