Fix: Synchronous popstate transitions (#30759)
This is a refactor of the fix in #27505. When a transition update is scheduled by a popstate event, (i.e. a back/ forward navigation) we attempt to render it synchronously even though it's a transition, since it's likely the previous page's data is cached. In #27505, I changed the implementation so that it only "upgrades" the priority of the transition for a single attempt. If the attempt suspends, say because the data is not cached after all, from then on it should be treated as a normal transition. But it turns out #27505 did not work as intended, because it relied on marking the root with pending synchronous work (root.pendingLanes), which was never cleared until the popstate update completed. The test scenarios I wrote accidentally worked for a different reason related to suspending the work loop, which I'm currently in the middle of refactoring.
Andrew Clark committed
Aug 23, 2024 at 12:30 UTC
ee7f6757c446c4e79ecc7e2bc22b8c9b712834b7
3 files changed
+116
-43
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
+1
-1
@@ -744,7 +744,7 @@ describe('ReactDOMFiberAsync', () => {
744
// Because it suspended, it remains on the current path
745
expect(div.textContent).toBe('/path/a');
746
});
747
- assertLog(['Suspend! [/path/b]']);
747
+ assertLog([]);
748
749
await act(async () => {
750
resolvePromise();
packages/react-reconciler/src/ReactFiberLane.js
+61
-11
@@ -92,6 +92,14 @@ export const DeferredLane: Lane = /* */ 0b1000000000000000000
92
export const UpdateLanes: Lanes =
93
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes;
94
95
+export const HydrationLanes =
96
+ SyncHydrationLane |
97
+ InputContinuousHydrationLane |
98
+ DefaultHydrationLane |
99
+ TransitionHydrationLane |
100
+ SelectiveHydrationLane |
101
+ IdleHydrationLane;
102
+
103
// This function is used for the experimental timeline (react-devtools-timeline)
104
// It should be kept in sync with the Lanes values above.
105
export function getLabelForLane(lane: Lane): string | void {
@@ -282,6 +290,51 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
290
return nextLanes;
291
}
292
293
+export function getNextLanesToFlushSync(
294
+ root: FiberRoot,
295
+ extraLanesToForceSync: Lane | Lanes,
296
+): Lanes {
297
+ // Similar to getNextLanes, except instead of choosing the next lanes to work
298
+ // on based on their priority, it selects all the lanes that have equal or
299
+ // higher priority than those are given. That way they can be synchronously
300
+ // rendered in a single batch.
301
+ //
302
+ // The main use case is updates scheduled by popstate events, which are
303
+ // flushed synchronously even though they are transitions.
304
+ const lanesToFlush = SyncUpdateLanes | extraLanesToForceSync;
305
+
306
+ // Early bailout if there's no pending work left.
307
+ const pendingLanes = root.pendingLanes;
308
+ if (pendingLanes === NoLanes) {
309
+ return NoLanes;
310
+ }
311
+
312
+ const suspendedLanes = root.suspendedLanes;
313
+ const pingedLanes = root.pingedLanes;
314
+
315
+ // Remove lanes that are suspended (but not pinged)
316
+ const unblockedLanes = pendingLanes & ~(suspendedLanes & ~pingedLanes);
317
+ const unblockedLanesWithMatchingPriority =
318
+ unblockedLanes & getLanesOfEqualOrHigherPriority(lanesToFlush);
319
+
320
+ // If there are matching hydration lanes, we should do those by themselves.
321
+ // Hydration lanes must never include updates.
322
+ if (unblockedLanesWithMatchingPriority & HydrationLanes) {
323
+ return (
324
+ (unblockedLanesWithMatchingPriority & HydrationLanes) | SyncHydrationLane
325
+ );
326
+ }
327
+
328
+ if (unblockedLanesWithMatchingPriority) {
329
+ // Always include the SyncLane as part of the result, even if there's no
330
+ // pending sync work, to indicate the priority of the entire batch of work
331
+ // is considered Sync.
332
+ return unblockedLanesWithMatchingPriority | SyncLane;
333
+ }
334
+
335
+ return NoLanes;
336
+}
337
+
338
export function getEntangledLanes(root: FiberRoot, renderLanes: Lanes): Lanes {
339
let entangledLanes = renderLanes;
340
@@ -534,6 +587,14 @@ export function getHighestPriorityLane(lanes: Lanes): Lane {
587
return lanes & -lanes;
588
}
589
590
+function getLanesOfEqualOrHigherPriority(lanes: Lane | Lanes): Lanes {
591
+ // Create a mask with all bits to the right or same as the highest bit.
592
+ // So if lanes is 0b100, the result would be 0b111.
593
+ // If lanes is 0b101, the result would be 0b111.
594
+ const lowestPriorityLaneIndex = 31 - clz32(lanes);
595
+ return (1 << (lowestPriorityLaneIndex + 1)) - 1;
596
+}
597
+
598
export function pickArbitraryLane(lanes: Lanes): Lane {
599
// This wrapper function gets inlined. Only exists so to communicate that it
600
// doesn't matter which bit is selected; you can pick any bit without
@@ -757,17 +818,6 @@ export function markRootEntangled(root: FiberRoot, entangledLanes: Lanes) {
818
}
819
}
820
760
-export function upgradePendingLaneToSync(root: FiberRoot, lane: Lane) {
761
- // Since we're upgrading the priority of the given lane, there is now pending
762
- // sync work.
763
- root.pendingLanes |= SyncLane;
764
-
765
- // Entangle the sync lane with the lane we're upgrading. This means SyncLane
766
- // will not be allowed to finish without also finishing the given lane.
767
- root.entangledLanes |= SyncLane;
768
- root.entanglements[SyncLaneIndex] |= lane;
769
-}
770
-
821
export function upgradePendingLanesToSync(
822
root: FiberRoot,
823
lanesToUpgrade: Lanes,
packages/react-reconciler/src/ReactFiberRootScheduler.js
+54
-31
@@ -8,7 +8,7 @@
8
*/
9
10
import type {FiberRoot} from './ReactInternalTypes';
11
-import type {Lane} from './ReactFiberLane';
11
+import type {Lane, Lanes} from './ReactFiberLane';
12
import type {PriorityLevel} from 'scheduler/src/SchedulerPriorities';
13
import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent';
14
@@ -24,8 +24,8 @@ import {
24
getNextLanes,
25
includesSyncLane,
26
markStarvedLanesAsExpired,
27
- upgradePendingLaneToSync,
27
claimNextTransitionLane,
28
+ getNextLanesToFlushSync,
29
} from './ReactFiberLane';
30
import {
31
CommitContext,
@@ -145,18 +145,21 @@ export function ensureRootIsScheduled(root: FiberRoot): void {
145
export function flushSyncWorkOnAllRoots() {
146
// This is allowed to be called synchronously, but the caller should check
147
// the execution context first.
148
- flushSyncWorkAcrossRoots_impl(false);
148
+ flushSyncWorkAcrossRoots_impl(NoLanes, false);
149
}
150
151
export function flushSyncWorkOnLegacyRootsOnly() {
152
// This is allowed to be called synchronously, but the caller should check
153
// the execution context first.
154
if (!disableLegacyMode) {
155
- flushSyncWorkAcrossRoots_impl(true);
155
+ flushSyncWorkAcrossRoots_impl(NoLanes, true);
156
}
157
}
158
159
-function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
159
+function flushSyncWorkAcrossRoots_impl(
160
+ syncTransitionLanes: Lanes | Lane,
161
+ onlyLegacy: boolean,
162
+) {
163
if (isFlushingWork) {
164
// Prevent reentrancy.
165
// TODO: Is this overly defensive? The callers must check the execution
@@ -179,17 +182,28 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
182
if (onlyLegacy && (disableLegacyMode || root.tag !== LegacyRoot)) {
183
// Skip non-legacy roots.
184
} else {
182
- const workInProgressRoot = getWorkInProgressRoot();
183
- const workInProgressRootRenderLanes =
184
- getWorkInProgressRootRenderLanes();
185
- const nextLanes = getNextLanes(
186
- root,
187
- root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,
188
- );
189
- if (includesSyncLane(nextLanes)) {
190
- // This root has pending sync work. Flush it now.
191
- didPerformSomeWork = true;
192
- performSyncWorkOnRoot(root, nextLanes);
185
+ if (syncTransitionLanes !== NoLanes) {
186
+ const nextLanes = getNextLanesToFlushSync(root, syncTransitionLanes);
187
+ if (nextLanes !== NoLanes) {
188
+ // This root has pending sync work. Flush it now.
189
+ didPerformSomeWork = true;
190
+ performSyncWorkOnRoot(root, nextLanes);
191
+ }
192
+ } else {
193
+ const workInProgressRoot = getWorkInProgressRoot();
194
+ const workInProgressRootRenderLanes =
195
+ getWorkInProgressRootRenderLanes();
196
+ const nextLanes = getNextLanes(
197
+ root,
198
+ root === workInProgressRoot
199
+ ? workInProgressRootRenderLanes
200
+ : NoLanes,
201
+ );
202
+ if (includesSyncLane(nextLanes)) {
203
+ // This root has pending sync work. Flush it now.
204
+ didPerformSomeWork = true;
205
+ performSyncWorkOnRoot(root, nextLanes);
206
+ }
207
}
208
}
209
root = root.next;
@@ -209,23 +223,23 @@ function processRootScheduleInMicrotask() {
223
// We'll recompute this as we iterate through all the roots and schedule them.
224
mightHavePendingSyncWork = false;
225
226
+ let syncTransitionLanes = NoLanes;
227
+ if (currentEventTransitionLane !== NoLane) {
228
+ if (shouldAttemptEagerTransition()) {
229
+ // A transition was scheduled during an event, but we're going to try to
230
+ // render it synchronously anyway. We do this during a popstate event to
231
+ // preserve the scroll position of the previous page.
232
+ syncTransitionLanes = currentEventTransitionLane;
233
+ }
234
+ currentEventTransitionLane = NoLane;
235
+ }
236
+
237
const currentTime = now();
238
239
let prev = null;
240
let root = firstScheduledRoot;
241
while (root !== null) {
242
const next = root.next;
218
-
219
- if (
220
- currentEventTransitionLane !== NoLane &&
221
- shouldAttemptEagerTransition()
222
- ) {
223
- // A transition was scheduled during an event, but we're going to try to
224
- // render it synchronously anyway. We do this during a popstate event to
225
- // preserve the scroll position of the previous page.
226
- upgradePendingLaneToSync(root, currentEventTransitionLane);
227
- }
228
-
243
const nextLanes = scheduleTaskForRootDuringMicrotask(root, currentTime);
244
if (nextLanes === NoLane) {
245
// This root has no more pending work. Remove it from the schedule. To
@@ -248,18 +262,27 @@ function processRootScheduleInMicrotask() {
262
} else {
263
// This root still has work. Keep it in the list.
264
prev = root;
251
- if (includesSyncLane(nextLanes)) {
265
+
266
+ // This is a fast-path optimization to early exit from
267
+ // flushSyncWorkOnAllRoots if we can be certain that there is no remaining
268
+ // synchronous work to perform. Set this to true if there might be sync
269
+ // work left.
270
+ if (
271
+ // Skip the optimization if syncTransitionLanes is set
272
+ syncTransitionLanes !== NoLanes ||
273
+ // Common case: we're not treating any extra lanes as synchronous, so we
274
+ // can just check if the next lanes are sync.
275
+ includesSyncLane(nextLanes)
276
+ ) {
277
mightHavePendingSyncWork = true;
278
}
279
}
280
root = next;
281
}
282
258
- currentEventTransitionLane = NoLane;
259
-
283
// At the end of the microtask, flush any pending synchronous work. This has
284
// to come at the end, because it does actual rendering work that might throw.
262
- flushSyncWorkOnAllRoots();
285
+ flushSyncWorkAcrossRoots_impl(syncTransitionLanes, false);
286
}
287
288
function scheduleTaskForRootDuringMicrotask(