Skip to content

Commit 8c680a0

Browse files
tyao1AndyPengc12
authored andcommitted
Synchronously flush the transition lane scheduled in a popstate event (facebook#26025)
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Browsers restore state like forms and scroll position right after the popstate event. To make sure the page work as expected on back or forward button, we need to flush transitions scheduled in a popstate synchronously, and only yields if it suspends. This PR adds a new HostConfig method to check if `window.event === 'popstate'`, and `scheduleMicrotask` if a transition is scheduled in a `PopStateEvent`. ## How did you test this change? yarn test
1 parent 1b1c9b2 commit 8c680a0

File tree

11 files changed

+196
-9
lines changed

11 files changed

+196
-9
lines changed

packages/react-art/src/ReactFiberConfigART.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ export function getCurrentEventPriority() {
346346
return DefaultEventPriority;
347347
}
348348

349+
export function shouldAttemptEagerTransition() {
350+
return false;
351+
}
352+
349353
// The ART renderer is secondary to the React DOM renderer.
350354
export const isPrimaryRenderer = false;
351355

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,10 @@ export function getCurrentEventPriority(): EventPriority {
527527
return getEventPriority(currentEvent.type);
528528
}
529529

530+
export function shouldAttemptEagerTransition(): boolean {
531+
return window.event && window.event.type === 'popstate';
532+
}
533+
530534
export const isPrimaryRenderer = true;
531535
export const warnsIfNotActing = true;
532536
// This initialization code may run even on server environments

packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ describe('ReactDOMFiberAsync', () => {
2727
let container;
2828

2929
beforeEach(() => {
30-
jest.resetModules();
3130
container = document.createElement('div');
3231
React = require('react');
3332
ReactDOM = require('react-dom');
@@ -40,6 +39,7 @@ describe('ReactDOMFiberAsync', () => {
4039
assertLog = InternalTestUtils.assertLog;
4140

4241
document.body.appendChild(container);
42+
window.event = undefined;
4343
});
4444

4545
afterEach(() => {
@@ -566,4 +566,139 @@ describe('ReactDOMFiberAsync', () => {
566566

567567
expect(container.textContent).toBe('new');
568568
});
569+
570+
it('should synchronously render the transition lane scheduled in a popState', async () => {
571+
function App() {
572+
const [syncState, setSyncState] = React.useState(false);
573+
const [hasNavigated, setHasNavigated] = React.useState(false);
574+
function onPopstate() {
575+
Scheduler.log(`popState`);
576+
React.startTransition(() => {
577+
setHasNavigated(true);
578+
});
579+
setSyncState(true);
580+
}
581+
React.useEffect(() => {
582+
window.addEventListener('popstate', onPopstate);
583+
return () => {
584+
window.removeEventListener('popstate', onPopstate);
585+
};
586+
}, []);
587+
Scheduler.log(`render:${hasNavigated}/${syncState}`);
588+
return null;
589+
}
590+
const root = ReactDOMClient.createRoot(container);
591+
await act(async () => {
592+
root.render(<App />);
593+
});
594+
assertLog(['render:false/false']);
595+
596+
await act(async () => {
597+
const popStateEvent = new Event('popstate');
598+
// Jest is not emulating window.event correctly in the microtask
599+
window.event = popStateEvent;
600+
window.dispatchEvent(popStateEvent);
601+
queueMicrotask(() => {
602+
window.event = undefined;
603+
});
604+
});
605+
606+
assertLog(['popState', 'render:true/true']);
607+
await act(() => {
608+
root.unmount();
609+
});
610+
});
611+
612+
it('Should not flush transition lanes if there is no transition scheduled in popState', async () => {
613+
let setHasNavigated;
614+
function App() {
615+
const [syncState, setSyncState] = React.useState(false);
616+
const [hasNavigated, _setHasNavigated] = React.useState(false);
617+
setHasNavigated = _setHasNavigated;
618+
function onPopstate() {
619+
setSyncState(true);
620+
}
621+
622+
React.useEffect(() => {
623+
window.addEventListener('popstate', onPopstate);
624+
return () => {
625+
window.removeEventListener('popstate', onPopstate);
626+
};
627+
}, []);
628+
629+
Scheduler.log(`render:${hasNavigated}/${syncState}`);
630+
return null;
631+
}
632+
const root = ReactDOMClient.createRoot(container);
633+
await act(async () => {
634+
root.render(<App />);
635+
});
636+
assertLog(['render:false/false']);
637+
638+
React.startTransition(() => {
639+
setHasNavigated(true);
640+
});
641+
await act(async () => {
642+
const popStateEvent = new Event('popstate');
643+
// Jest is not emulating window.event correctly in the microtask
644+
window.event = popStateEvent;
645+
window.dispatchEvent(popStateEvent);
646+
queueMicrotask(() => {
647+
window.event = undefined;
648+
});
649+
});
650+
assertLog(['render:false/true', 'render:true/true']);
651+
await act(() => {
652+
root.unmount();
653+
});
654+
});
655+
656+
it('transition lane in popState should yield if it suspends', async () => {
657+
const never = {then() {}};
658+
let _setText;
659+
660+
function App() {
661+
const [shouldSuspend, setShouldSuspend] = React.useState(false);
662+
const [text, setText] = React.useState('0');
663+
_setText = setText;
664+
if (shouldSuspend) {
665+
Scheduler.log('Suspend!');
666+
throw never;
667+
}
668+
function onPopstate() {
669+
React.startTransition(() => {
670+
setShouldSuspend(val => !val);
671+
});
672+
}
673+
React.useEffect(() => {
674+
window.addEventListener('popstate', onPopstate);
675+
return () => window.removeEventListener('popstate', onPopstate);
676+
}, []);
677+
Scheduler.log(`Child:${shouldSuspend}/${text}`);
678+
return text;
679+
}
680+
681+
const root = ReactDOMClient.createRoot(container);
682+
await act(async () => {
683+
root.render(<App />);
684+
});
685+
assertLog(['Child:false/0']);
686+
687+
await act(() => {
688+
const popStateEvent = new Event('popstate');
689+
window.event = popStateEvent;
690+
window.dispatchEvent(popStateEvent);
691+
queueMicrotask(() => {
692+
window.event = undefined;
693+
});
694+
});
695+
assertLog(['Suspend!']);
696+
697+
await act(async () => {
698+
_setText('1');
699+
});
700+
assertLog(['Child:false/1', 'Suspend!']);
701+
702+
root.unmount();
703+
});
569704
});

packages/react-native-renderer/src/ReactFiberConfigFabric.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,10 @@ export function getCurrentEventPriority(): * {
334334
return DefaultEventPriority;
335335
}
336336

337+
export function shouldAttemptEagerTransition(): boolean {
338+
return false;
339+
}
340+
337341
// The Fabric renderer is secondary to the existing React Native renderer.
338342
export const isPrimaryRenderer = false;
339343

packages/react-native-renderer/src/ReactFiberConfigNative.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,10 @@ export function getCurrentEventPriority(): * {
265265
return DefaultEventPriority;
266266
}
267267

268+
export function shouldAttemptEagerTransition(): boolean {
269+
return false;
270+
}
271+
268272
// -------------------
269273
// Mutation
270274
// -------------------

packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
526526
return currentEventPriority;
527527
},
528528

529+
shouldAttemptEagerTransition(): boolean {
530+
return false;
531+
},
532+
529533
now: Scheduler.unstable_now,
530534

531535
isPrimaryRenderer: true,

packages/react-reconciler/src/ReactFiberRootScheduler.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import {
2020
getNextLanes,
2121
includesSyncLane,
2222
markStarvedLanesAsExpired,
23+
markRootEntangled,
24+
mergeLanes,
2325
} from './ReactFiberLane';
2426
import {
2527
CommitContext,
@@ -49,7 +51,11 @@ import {
4951
IdleEventPriority,
5052
lanesToEventPriority,
5153
} from './ReactEventPriorities';
52-
import {supportsMicrotasks, scheduleMicrotask} from './ReactFiberConfig';
54+
import {
55+
supportsMicrotasks,
56+
scheduleMicrotask,
57+
shouldAttemptEagerTransition,
58+
} from './ReactFiberConfig';
5359

5460
import ReactSharedInternals from 'shared/ReactSharedInternals';
5561
const {ReactCurrentActQueue} = ReactSharedInternals;
@@ -72,6 +78,8 @@ let mightHavePendingSyncWork: boolean = false;
7278

7379
let isFlushingWork: boolean = false;
7480

81+
let currentEventTransitionLane: Lane = NoLanes;
82+
7583
export function ensureRootIsScheduled(root: FiberRoot): void {
7684
// This function is called whenever a root receives an update. It does two
7785
// things 1) it ensures the root is in the root schedule, and 2) it ensures
@@ -238,6 +246,14 @@ function processRootScheduleInMicrotask() {
238246
let root = firstScheduledRoot;
239247
while (root !== null) {
240248
const next = root.next;
249+
250+
if (
251+
currentEventTransitionLane !== NoLane &&
252+
shouldAttemptEagerTransition()
253+
) {
254+
markRootEntangled(root, mergeLanes(currentEventTransitionLane, SyncLane));
255+
}
256+
241257
const nextLanes = scheduleTaskForRootDuringMicrotask(root, currentTime);
242258
if (nextLanes === NoLane) {
243259
// This root has no more pending work. Remove it from the schedule. To
@@ -267,6 +283,8 @@ function processRootScheduleInMicrotask() {
267283
root = next;
268284
}
269285

286+
currentEventTransitionLane = NoLane;
287+
270288
// At the end of the microtask, flush any pending synchronous work. This has
271289
// to come at the end, because it does actual rendering work that might throw.
272290
flushSyncWorkOnAllRoots();
@@ -472,3 +490,11 @@ function scheduleImmediateTask(cb: () => mixed) {
472490
Scheduler_scheduleCallback(ImmediateSchedulerPriority, cb);
473491
}
474492
}
493+
494+
export function getCurrentEventTransitionLane(): Lane {
495+
return currentEventTransitionLane;
496+
}
497+
498+
export function setCurrentEventTransitionLane(lane: Lane): void {
499+
currentEventTransitionLane = lane;
500+
}

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,8 @@ import {
278278
flushSyncWorkOnAllRoots,
279279
flushSyncWorkOnLegacyRootsOnly,
280280
getContinuationForRoot,
281+
getCurrentEventTransitionLane,
282+
setCurrentEventTransitionLane,
281283
} from './ReactFiberRootScheduler';
282284
import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext';
283285

@@ -582,8 +584,6 @@ const NESTED_PASSIVE_UPDATE_LIMIT = 50;
582584
let nestedPassiveUpdateCount: number = 0;
583585
let rootWithPassiveNestedUpdates: FiberRoot | null = null;
584586

585-
let currentEventTransitionLane: Lanes = NoLanes;
586-
587587
let isRunningInsertionEffect = false;
588588

589589
export function getWorkInProgressRoot(): FiberRoot | null {
@@ -640,11 +640,11 @@ export function requestUpdateLane(fiber: Fiber): Lane {
640640
// The trick we use is to cache the first of each of these inputs within an
641641
// event. Then reset the cached values once we can be sure the event is
642642
// over. Our heuristic for that is whenever we enter a concurrent work loop.
643-
if (currentEventTransitionLane === NoLane) {
643+
if (getCurrentEventTransitionLane() === NoLane) {
644644
// All transitions within the same event are assigned the same lane.
645-
currentEventTransitionLane = claimNextTransitionLane();
645+
setCurrentEventTransitionLane(claimNextTransitionLane());
646646
}
647-
return currentEventTransitionLane;
647+
return getCurrentEventTransitionLane();
648648
}
649649

650650
// Updates originating inside certain React methods, like flushSync, have
@@ -848,8 +848,6 @@ export function performConcurrentWorkOnRoot(
848848
resetNestedUpdateFlag();
849849
}
850850

851-
currentEventTransitionLane = NoLanes;
852-
853851
if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
854852
throw new Error('Should not already be working.');
855853
}

packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ describe('ReactFiberHostContext', () => {
7070
getCurrentEventPriority: function () {
7171
return DefaultEventPriority;
7272
},
73+
shouldAttemptEagerTransition() {
74+
return false;
75+
},
7376
requestPostPaintCallback: function () {},
7477
maySuspendCommit(type, props) {
7578
return false;

packages/react-reconciler/src/forks/ReactFiberConfig.custom.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ export const preparePortalMount = $$$config.preparePortalMount;
6666
export const prepareScopeUpdate = $$$config.prepareScopeUpdate;
6767
export const getInstanceFromScope = $$$config.getInstanceFromScope;
6868
export const getCurrentEventPriority = $$$config.getCurrentEventPriority;
69+
export const shouldAttemptEagerTransition =
70+
$$$config.shouldAttemptEagerTransition;
6971
export const detachDeletedInstance = $$$config.detachDeletedInstance;
7072
export const requestPostPaintCallback = $$$config.requestPostPaintCallback;
7173
export const maySuspendCommit = $$$config.maySuspendCommit;

0 commit comments

Comments
 (0)