diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 843582cd9d177..8e2617cfa64d7 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -26,7 +26,8 @@ export opaque type LanePriority = | 13 | 14 | 15 - | 16; + | 16 + | 17; export opaque type Lanes = number; export opaque type Lane = number; export opaque type LaneMap = Array; @@ -42,23 +43,25 @@ import { NoPriority as NoSchedulerPriority, } from './SchedulerWithReactIntegration.new'; -export const SyncLanePriority: LanePriority = 16; -const SyncBatchedLanePriority: LanePriority = 15; +export const SyncLanePriority: LanePriority = 17; +const SyncBatchedLanePriority: LanePriority = 16; -const InputDiscreteHydrationLanePriority: LanePriority = 14; -export const InputDiscreteLanePriority: LanePriority = 13; +const InputDiscreteHydrationLanePriority: LanePriority = 15; +export const InputDiscreteLanePriority: LanePriority = 14; -const InputContinuousHydrationLanePriority: LanePriority = 12; -export const InputContinuousLanePriority: LanePriority = 11; +const InputContinuousHydrationLanePriority: LanePriority = 13; +export const InputContinuousLanePriority: LanePriority = 12; -const DefaultHydrationLanePriority: LanePriority = 10; -export const DefaultLanePriority: LanePriority = 9; +const DefaultHydrationLanePriority: LanePriority = 11; +export const DefaultLanePriority: LanePriority = 10; -const TransitionShortHydrationLanePriority: LanePriority = 8; -export const TransitionShortLanePriority: LanePriority = 7; +const TransitionShortHydrationLanePriority: LanePriority = 9; +export const TransitionShortLanePriority: LanePriority = 8; -const TransitionLongHydrationLanePriority: LanePriority = 6; -export const TransitionLongLanePriority: LanePriority = 5; +const TransitionLongHydrationLanePriority: LanePriority = 7; +export const TransitionLongLanePriority: LanePriority = 6; + +const RetryLanePriority: LanePriority = 5; const SelectiveHydrationLanePriority: LanePriority = 4; @@ -90,25 +93,30 @@ const InputContinuousUpdateRangeStart = 6; const InputContinuousUpdateRangeEnd = 8; export const DefaultHydrationLane: Lane = /* */ 0b0000000000000000000000100000000; -const DefaultLanes: Lanes = /* */ 0b0000000000000000011111100000000; +export const DefaultLanes: Lanes = /* */ 0b0000000000000000000111100000000; const DefaultUpdateRangeStart = 9; -const DefaultUpdateRangeEnd = 14; +const DefaultUpdateRangeEnd = 12; + +const TransitionShortHydrationLane: Lane = /* */ 0b0000000000000000001000000000000; +const TransitionShortLanes: Lanes = /* */ 0b0000000000000011111000000000000; +const TransitionShortUpdateRangeStart = 13; +const TransitionShortUpdateRangeEnd = 17; -const TransitionShortHydrationLane: Lane = /* */ 0b0000000000000000100000000000000; -const TransitionShortLanes: Lanes = /* */ 0b0000000000011111100000000000000; -const TransitionShortUpdateRangeStart = 15; -const TransitionShortUpdateRangeEnd = 20; +const TransitionLongHydrationLane: Lane = /* */ 0b0000000000000100000000000000000; +const TransitionLongLanes: Lanes = /* */ 0b0000000001111100000000000000000; +const TransitionLongUpdateRangeStart = 18; +const TransitionLongUpdateRangeEnd = 22; -const TransitionLongHydrationLane: Lane = /* */ 0b0000000000100000000000000000000; -const TransitionLongLanes: Lanes = /* */ 0b0000011111100000000000000000000; -const TransitionLongUpdateRangeStart = 21; -const TransitionLongUpdateRangeEnd = 26; +// Includes all updates. Except Idle updates, which have special semantics. +const UpdateRangeEnd = TransitionLongUpdateRangeEnd; -export const SelectiveHydrationLane: Lane = /* */ 0b0000110000000000000000000000000; +const RetryLanes: Lanes = /* */ 0b0000011110000000000000000000000; +const RetryRangeStart = 22; +const RetryRangeEnd = 26; + +export const SelectiveHydrationLane: Lane = /* */ 0b0000100000000000000000000000000; const SelectiveHydrationRangeEnd = 27; -// Includes all non-Idle updates -const UpdateRangeEnd = 27; const NonIdleLanes = /* */ 0b0000111111111111111111111111111; export const IdleHydrationLane: Lane = /* */ 0b0001000000000000000000000000000; @@ -206,6 +214,12 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes { return transitionLongLanes; } } + const retryLanes = RetryLanes & lanes; + if (retryLanes !== NoLanes) { + return_highestLanePriority = RetryLanePriority; + return_updateRangeEnd = RetryRangeEnd; + return retryLanes; + } if (lanes & SelectiveHydrationLane) { return_highestLanePriority = SelectiveHydrationLanePriority; return_updateRangeEnd = SelectiveHydrationRangeEnd; @@ -275,6 +289,7 @@ export function lanePriorityToSchedulerPriority( case TransitionLongHydrationLanePriority: case TransitionLongLanePriority: case SelectiveHydrationLanePriority: + case RetryLanePriority: return NormalSchedulerPriority; case IdleHydrationLanePriority: case IdleLanePriority: @@ -537,6 +552,9 @@ export function findUpdateLane( case TransitionLongLanePriority: // Should be handled by findTransitionLane instead break; + case RetryLanePriority: + // Should be handled by findRetryLane instead + break; case IdleLanePriority: let lane = findLane(IdleUpdateRangeStart, IdleUpdateRangeEnd, wipLanes); if (lane === NoLane) { @@ -604,6 +622,19 @@ export function findTransitionLane( ); } +// 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 = findLane(RetryRangeStart, RetryRangeEnd, wipLanes); + if (lane === NoLane) { + lane = pickArbitraryLane(RetryLanes); + } + return lane; +} + function findLane(start, end, skipLanes) { // This finds the first bit between the `start` and `end` positions that isn't // in `skipLanes`. @@ -808,6 +839,11 @@ export function getBumpedLaneForHydration( case TransitionLongLanePriority: lane = TransitionLongHydrationLane; break; + case RetryLanePriority: + // Shouldn't be reachable under normal circumstances, so there's no + // dedicated lane for retry priority. Use the one for long transitions. + lane = TransitionLongHydrationLane; + break; case SelectiveHydrationLanePriority: lane = SelectiveHydrationLane; break; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 9ca87567bcd9c..84dc05502ea81 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -123,6 +123,7 @@ import { NoTimestamp, findUpdateLane, findTransitionLane, + findRetryLane, includesSomeLane, isSubsetOfLanes, mergeLanes, @@ -470,6 +471,28 @@ export function requestUpdateLane( return lane; } +function requestRetryLane(fiber: Fiber) { + // This is a fork of `requestUpdateLane` designed specifically for Suspense + // "retries" — a special update that attempts to flip a Suspense boundary + // from its placeholder state to its primary/resolved state. + + // Special cases + const mode = fiber.mode; + if ((mode & BlockingMode) === NoMode) { + return (SyncLane: Lane); + } else if ((mode & ConcurrentMode) === NoMode) { + return getCurrentPriorityLevel() === ImmediateSchedulerPriority + ? (SyncLane: Lane) + : (SyncBatchedLane: Lane); + } + + // See `requestUpdateLane` for explanation of `currentEventWipLanes` + if (currentEventWipLanes === NoLanes) { + currentEventWipLanes = workInProgressRootIncludedLanes; + } + return findRetryLane(currentEventWipLanes); +} + export function scheduleUpdateOnFiber( fiber: Fiber, lane: Lane, @@ -2691,10 +2714,7 @@ 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) { - const suspenseConfig = null; // Retries don't carry over the already committed update. - // TODO: Should retries get their own lane? Maybe it could share with - // transitions. - retryLane = requestUpdateLane(boundaryFiber, suspenseConfig); + retryLane = requestRetryLane(boundaryFiber); } // TODO: Special case idle priority? const eventTime = requestEventTime(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index ebc5063303b7f..a520a1eb65bfa 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -146,6 +146,7 @@ import { NoTimestamp, findUpdateLane, findTransitionLane, + findRetryLane, includesSomeLane, isSubsetOfLanes, mergeLanes, @@ -493,6 +494,28 @@ export function requestUpdateLane( return lane; } +function requestRetryLane(fiber: Fiber) { + // This is a fork of `requestUpdateLane` designed specifically for Suspense + // "retries" — a special update that attempts to flip a Suspense boundary + // from its placeholder state to its primary/resolved state. + + // Special cases + const mode = fiber.mode; + if ((mode & BlockingMode) === NoMode) { + return (SyncLane: Lane); + } else if ((mode & ConcurrentMode) === NoMode) { + return getCurrentPriorityLevel() === ImmediateSchedulerPriority + ? (SyncLane: Lane) + : (SyncBatchedLane: Lane); + } + + // See `requestUpdateLane` for explanation of `currentEventWipLanes` + if (currentEventWipLanes === NoLanes) { + currentEventWipLanes = workInProgressRootIncludedLanes; + } + return findRetryLane(currentEventWipLanes); +} + export function scheduleUpdateOnFiber( fiber: Fiber, lane: Lane, @@ -2838,10 +2861,7 @@ 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) { - const suspenseConfig = null; // Retries don't carry over the already committed update. - // TODO: Should retries get their own lane? Maybe it could share with - // transitions. - retryLane = requestUpdateLane(boundaryFiber, suspenseConfig); + retryLane = requestRetryLane(boundaryFiber); } // TODO: Special case idle priority? const eventTime = requestEventTime(); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js index 2db8f7ecbf734..2314ef0c47ada 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js @@ -219,6 +219,11 @@ describe('ReactSuspense', () => { await resolve1(); ReactNoop.render(element); expect(Scheduler).toFlushWithoutYielding(); + + // Force fallback to commit. + // TODO: Should be able to use `act` here. + jest.runAllTimers(); + expect(ReactNoop.getChildren()).toEqual([ text('Waiting Tier 2'), text('Done'), diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index a8ffa822960a4..a0f2b13e670a9 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -623,6 +623,17 @@ describe('ReactSuspenseWithNoopRenderer', () => { // The new reconciler batches everything together, so it finishes without // suspending again. 'Sibling', + + // NOTE: The final of the update got pushed into a lower priority range of + // lanes, leading to the extra intermediate render. This is because when + // we schedule the fourth update, we're already in the middle of rendering + // the three others. Since there are only three lanes in the default + // range, the fourth lane is shifted to slightly lower priority. This + // could easily change when we tweak our batching heuristics. Ideally, + // they'd all have default priority and render in a single batch. + 'Suspend! [Step 3]', + 'Sibling', + 'Step 4', ]); }); @@ -3896,4 +3907,59 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(Scheduler).toHaveYielded(['Promise resolved [B]', 'B']); expect(root).toMatchRenderedOutput(); }); + + it('retries have lower priority than normal updates', async () => { + const {useState} = React; + + let setText; + function UpdatingText() { + const [text, _setText] = useState('A'); + setText = _setText; + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render( + <> + + }> + + + , + ); + }); + expect(Scheduler).toHaveYielded(['A', 'Suspend! [Async]', 'Loading...']); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + + await ReactNoop.act(async () => { + // Resolve the promise. This will trigger a retry. + await resolveText('Async'); + expect(Scheduler).toHaveYielded(['Promise resolved [Async]']); + // Before the retry happens, schedule a new update. + setText('B'); + + // The update should be allowed to finish before the retry is attempted. + expect(Scheduler).toFlushUntilNextPaint(['B']); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + }); + // Then do the retry. + expect(Scheduler).toHaveYielded(['Async']); + expect(root).toMatchRenderedOutput( + <> + + + , + ); + }); }); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 722d02d815e80..78672895584fd 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -3923,7 +3923,6 @@ describe('Profiler', () => { expect(onPostCommit).toHaveBeenCalledTimes(1); expect(onPostCommit.mock.calls[0][4]).toMatchInteractions([ - initialRenderInteraction, highPriUpdateInteraction, ]);