From 79294e914b889b7d65bc6ffe86dd7d69502a7f4a Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 18 Jan 2023 00:00:29 -0800 Subject: [PATCH 1/7] Synchronously render the transition lane on popstate --- packages/react-art/src/ReactFiberConfigART.js | 4 ++ .../src/client/ReactFiberConfigDOM.js | 4 ++ .../src/__tests__/ReactDOMFiberAsync-test.js | 41 +++++++++++++++++++ .../src/ReactFiberConfigFabric.js | 4 ++ .../src/ReactFiberConfigNative.js | 4 ++ .../src/createReactNoop.js | 4 ++ .../src/ReactFiberClassUpdateQueue.js | 4 +- .../react-reconciler/src/ReactFiberHooks.js | 4 +- .../react-reconciler/src/ReactFiberLane.js | 6 ++- .../src/ReactFiberRootScheduler.js | 16 +++++++- .../ReactFiberHostContext-test.internal.js | 3 ++ .../src/ReactFiberConfigTestHost.js | 3 ++ 12 files changed, 91 insertions(+), 6 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 591072cec84e8..bcf52149fd661 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -346,6 +346,10 @@ export function getCurrentEventPriority() { return DefaultEventPriority; } +export function shouldAttemptEagerTransition() { + return false; +} + // The ART renderer is secondary to the React DOM renderer. export const isPrimaryRenderer = false; diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 93c8950b05575..2315c00547953 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -527,6 +527,10 @@ export function getCurrentEventPriority(): EventPriority { return getEventPriority(currentEvent.type); } +export function shouldAttemptEagerTransition(): boolean { + return window.event && window.event.type === 'popstate'; +} + export const isPrimaryRenderer = true; export const warnsIfNotActing = true; // This initialization code may run even on server environments diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index 61bb7a318d617..36ab33642bdab 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -566,4 +566,45 @@ describe('ReactDOMFiberAsync', () => { expect(container.textContent).toBe('new'); }); + + it('should synchronously render the transition lane in a popState', async () => { + function App() { + const [syncState, setSyncState] = React.useState(false); + const [hasNavigated, setHasNavigated] = React.useState(false); + function onPopstate() { + Scheduler.log(`popState`); + React.startTransition(() => { + setHasNavigated(true); + }); + setSyncState(true); + + // Jest is not emulating window.event correctly in the microtask + const currentEvent = window.event; + window.event = currentEvent; + queueMicrotask(() => { + window.event = null; + }); + } + React.useEffect(() => { + window.addEventListener('popstate', onPopstate); + return () => window.removeEventListener('popstate', onPopstate); + }, []); + Scheduler.log(`render:${hasNavigated}/${syncState}`); + return null; + } + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + assertLog(['render:false/false']); + + await act(async () => { + const popStateEvent = new Event('popstate'); + window.dispatchEvent(popStateEvent); + }); + + assertLog(['popState', 'render:true/true']); + + root.unmount(); + }); }); diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 5dd36c20baafb..28bdfcb55dc53 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -334,6 +334,10 @@ export function getCurrentEventPriority(): * { return DefaultEventPriority; } +export function shouldAttemptEagerTransition(): boolean { + return false; +} + // The Fabric renderer is secondary to the existing React Native renderer. export const isPrimaryRenderer = false; diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index b218a1f77c234..5467ecf72d376 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -265,6 +265,10 @@ export function getCurrentEventPriority(): * { return DefaultEventPriority; } +export function shouldAttemptEagerTransition(): boolean { + return false; +} + // ------------------- // Mutation // ------------------- diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index f32ec38806b65..fe2e382f353ef 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -526,6 +526,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return currentEventPriority; }, + shouldAttemptEagerTransition(): boolean { + return false; + }, + now: Scheduler.unstable_now, isPrimaryRenderer: true, diff --git a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js index 2e88c982022d3..0e67d67e63a28 100644 --- a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js @@ -94,7 +94,7 @@ import { isSubsetOfLanes, mergeLanes, removeLanes, - isTransitionLane, + includesTransitionLane, intersectLanes, markRootEntangled, } from './ReactFiberLane'; @@ -279,7 +279,7 @@ export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) { } const sharedQueue: SharedQueue = (updateQueue: any).shared; - if (isTransitionLane(lane)) { + if (includesTransitionLane(lane)) { let queueLanes = sharedQueue.lanes; // If any entangled lanes are no longer pending on the root, then they must diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 4be1994114a32..cc3decbafad32 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -67,7 +67,7 @@ import { mergeLanes, removeLanes, intersectLanes, - isTransitionLane, + includesTransitionLane, markRootEntangled, markRootMutableRead, } from './ReactFiberLane'; @@ -2744,7 +2744,7 @@ function entangleTransitionUpdate( queue: UpdateQueue, lane: Lane, ): void { - if (isTransitionLane(lane)) { + if (includesTransitionLane(lane)) { let queueLanes = queue.lanes; // If any entangled lanes are no longer pending on the root, then they diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 835bf01193423..95b125ff1ad1a 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -452,6 +452,10 @@ export function getLanesToRetrySynchronouslyOnError( return NoLanes; } +export function getTransitionLanes(lanes: Lanes): Lanes { + return lanes & TransitionLanes; +} + export function includesSyncLane(lanes: Lanes): boolean { return (lanes & (SyncLane | SyncHydrationLane)) !== NoLanes; } @@ -494,7 +498,7 @@ export function includesExpiredLane(root: FiberRoot, lanes: Lanes): boolean { return (lanes & root.expiredLanes) !== NoLanes; } -export function isTransitionLane(lane: Lane): boolean { +export function includesTransitionLane(lane: Lane): boolean { return (lane & TransitionLanes) !== NoLanes; } diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 7206579745871..be2aa614b526f 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -19,7 +19,11 @@ import { getHighestPriorityLane, getNextLanes, includesSyncLane, + includesTransitionLane, markStarvedLanesAsExpired, + markRootEntangled, + mergeLanes, + getTransitionLanes, } from './ReactFiberLane'; import { CommitContext, @@ -49,7 +53,11 @@ import { IdleEventPriority, lanesToEventPriority, } from './ReactEventPriorities'; -import {supportsMicrotasks, scheduleMicrotask} from './ReactFiberConfig'; +import { + supportsMicrotasks, + scheduleMicrotask, + shouldAttemptEagerTransition, +} from './ReactFiberConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; @@ -238,6 +246,12 @@ function processRootScheduleInMicrotask() { let root = firstScheduledRoot; while (root !== null) { const next = root.next; + if (includesTransitionLane(root.pendingLanes) && shouldAttemptEagerTransition()) { + markRootEntangled( + root, + mergeLanes(getTransitionLanes(root.pendingLanes), SyncLane), + ); + } const nextLanes = scheduleTaskForRootDuringMicrotask(root, currentTime); if (nextLanes === NoLane) { // This root has no more pending work. Remove it from the schedule. To diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index a4c32090bfd8a..bfbcddfcb7373 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -70,6 +70,9 @@ describe('ReactFiberHostContext', () => { getCurrentEventPriority: function () { return DefaultEventPriority; }, + shouldAttemptEagerTransition() { + return false; + }, requestPostPaintCallback: function () {}, maySuspendCommit(type, props) { return false; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 1b7e9adcee86b..3b478b01276db 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -213,6 +213,9 @@ export function createTextInstance( export function getCurrentEventPriority(): * { return DefaultEventPriority; } +export function shouldAttemptEagerTransition(): boolean { + return false; +} export const isPrimaryRenderer = false; export const warnsIfNotActing = true; From 6aaa397d8678d7b90718267d048a0443c049040d Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Tue, 4 Apr 2023 16:03:09 -0700 Subject: [PATCH 2/7] Use currentEventTransitionLane --- .../src/__tests__/ReactDOMFiberAsync-test.js | 52 ++++++++++++++++--- .../react-reconciler/src/ReactFiberLane.js | 4 -- .../src/ReactFiberRootScheduler.js | 20 +++++-- .../src/ReactFiberWorkLoop.js | 12 ++--- 4 files changed, 67 insertions(+), 21 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index 36ab33642bdab..a7333ff036f9f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -16,6 +16,7 @@ let ReactDOMClient; let Scheduler; let act; let waitForAll; +let waitFor; let assertLog; const setUntrackedInputValue = Object.getOwnPropertyDescriptor( @@ -37,6 +38,7 @@ describe('ReactDOMFiberAsync', () => { const InternalTestUtils = require('internal-test-utils'); waitForAll = InternalTestUtils.waitForAll; + waitFor = InternalTestUtils.waitFor; assertLog = InternalTestUtils.assertLog; document.body.appendChild(container); @@ -567,7 +569,7 @@ describe('ReactDOMFiberAsync', () => { expect(container.textContent).toBe('new'); }); - it('should synchronously render the transition lane in a popState', async () => { + it('should synchronously render the transition lane scheduled in a popState', async () => { function App() { const [syncState, setSyncState] = React.useState(false); const [hasNavigated, setHasNavigated] = React.useState(false); @@ -579,11 +581,8 @@ describe('ReactDOMFiberAsync', () => { setSyncState(true); // Jest is not emulating window.event correctly in the microtask - const currentEvent = window.event; - window.event = currentEvent; - queueMicrotask(() => { - window.event = null; - }); + // TODO: this causes `window.event` to always be popState in this test file + window.event = window.event; } React.useEffect(() => { window.addEventListener('popstate', onPopstate); @@ -607,4 +606,45 @@ describe('ReactDOMFiberAsync', () => { root.unmount(); }); + + it('should not flush transition lanes if there is no transition scheduled in popState', async () => { + let _setHasNavigated; + function App({shouldTransition}) { + const [syncState, setSyncState] = React.useState(false); + const [hasNavigated, setHasNavigated] = React.useState(false); + _setHasNavigated = setHasNavigated; + function onPopstate() { + setSyncState(true); + + // Jest is not emulating window.event correctly in the microtask + window.event = window.event; + } + + React.useEffect(() => { + window.addEventListener('popstate', onPopstate); + return () => window.removeEventListener('popstate', onPopstate); + }, []); + + Scheduler.log(`render:${hasNavigated}/${syncState}`); + + return null; + } + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + assertLog(['render:false/false']); + + // Trigger a transition update to be scheduled + React.startTransition(() => { + _setHasNavigated(true); + }) + + // The popState should not flush the transition update + const popStateEvent = new Event('popstate'); + window.dispatchEvent(popStateEvent); + await waitFor(['render:false/true']); + + root.unmount(); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 95b125ff1ad1a..1508d03f90589 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -452,10 +452,6 @@ export function getLanesToRetrySynchronouslyOnError( return NoLanes; } -export function getTransitionLanes(lanes: Lanes): Lanes { - return lanes & TransitionLanes; -} - export function includesSyncLane(lanes: Lanes): boolean { return (lanes & (SyncLane | SyncHydrationLane)) !== NoLanes; } diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index be2aa614b526f..d26df7f87d323 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -19,11 +19,9 @@ import { getHighestPriorityLane, getNextLanes, includesSyncLane, - includesTransitionLane, markStarvedLanesAsExpired, markRootEntangled, mergeLanes, - getTransitionLanes, } from './ReactFiberLane'; import { CommitContext, @@ -80,6 +78,8 @@ let mightHavePendingSyncWork: boolean = false; let isFlushingWork: boolean = false; +let currentEventTransitionLane: Lane = NoLanes; + export function ensureRootIsScheduled(root: FiberRoot): void { // This function is called whenever a root receives an update. It does two // things 1) it ensures the root is in the root schedule, and 2) it ensures @@ -246,12 +246,14 @@ function processRootScheduleInMicrotask() { let root = firstScheduledRoot; while (root !== null) { const next = root.next; - if (includesTransitionLane(root.pendingLanes) && shouldAttemptEagerTransition()) { + + if (currentEventTransitionLane !== NoLane && shouldAttemptEagerTransition()) { markRootEntangled( root, - mergeLanes(getTransitionLanes(root.pendingLanes), SyncLane), + mergeLanes(currentEventTransitionLane, SyncLane), ); } + const nextLanes = scheduleTaskForRootDuringMicrotask(root, currentTime); if (nextLanes === NoLane) { // This root has no more pending work. Remove it from the schedule. To @@ -281,6 +283,8 @@ function processRootScheduleInMicrotask() { root = next; } + currentEventTransitionLane = NoLane; + // At the end of the microtask, flush any pending synchronous work. This has // to come at the end, because it does actual rendering work that might throw. flushSyncWorkOnAllRoots(); @@ -486,3 +490,11 @@ function scheduleImmediateTask(cb: () => mixed) { Scheduler_scheduleCallback(ImmediateSchedulerPriority, cb); } } + +export function getCurrentEventTransitionLane(): Lane { + return currentEventTransitionLane; +} + +export function setCurrentEventTransitionLane(lane: Lane): void { + currentEventTransitionLane = lane; +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6f00533d3004d..a25cf0934a7a4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -279,6 +279,8 @@ import { flushSyncWorkOnAllRoots, flushSyncWorkOnLegacyRootsOnly, getContinuationForRoot, + getCurrentEventTransitionLane, + setCurrentEventTransitionLane, } from './ReactFiberRootScheduler'; import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext'; @@ -583,8 +585,6 @@ const NESTED_PASSIVE_UPDATE_LIMIT = 50; let nestedPassiveUpdateCount: number = 0; let rootWithPassiveNestedUpdates: FiberRoot | null = null; -let currentEventTransitionLane: Lanes = NoLanes; - let isRunningInsertionEffect = false; export function getWorkInProgressRoot(): FiberRoot | null { @@ -641,11 +641,11 @@ export function requestUpdateLane(fiber: Fiber): Lane { // The trick we use is to cache the first of each of these inputs within an // 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. - if (currentEventTransitionLane === NoLane) { + if (getCurrentEventTransitionLane() === NoLane) { // All transitions within the same event are assigned the same lane. - currentEventTransitionLane = claimNextTransitionLane(); + setCurrentEventTransitionLane(claimNextTransitionLane()); } - return currentEventTransitionLane; + return getCurrentEventTransitionLane(); } // Updates originating inside certain React methods, like flushSync, have @@ -849,8 +849,6 @@ export function performConcurrentWorkOnRoot( resetNestedUpdateFlag(); } - currentEventTransitionLane = NoLanes; - if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { throw new Error('Should not already be working.'); } From bfaa019b20e651b89b60835538c8ab56a1e54bcc Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Tue, 4 Apr 2023 16:22:37 -0700 Subject: [PATCH 3/7] Fix the test --- .../src/__tests__/ReactDOMFiberAsync-test.js | 64 ++++++++++--------- .../src/ReactFiberRootScheduler.js | 14 ++-- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index a7333ff036f9f..75031232adee4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -16,7 +16,6 @@ let ReactDOMClient; let Scheduler; let act; let waitForAll; -let waitFor; let assertLog; const setUntrackedInputValue = Object.getOwnPropertyDescriptor( @@ -28,7 +27,6 @@ describe('ReactDOMFiberAsync', () => { let container; beforeEach(() => { - jest.resetModules(); container = document.createElement('div'); React = require('react'); ReactDOM = require('react-dom'); @@ -38,7 +36,6 @@ describe('ReactDOMFiberAsync', () => { const InternalTestUtils = require('internal-test-utils'); waitForAll = InternalTestUtils.waitForAll; - waitFor = InternalTestUtils.waitFor; assertLog = InternalTestUtils.assertLog; document.body.appendChild(container); @@ -579,14 +576,12 @@ describe('ReactDOMFiberAsync', () => { setHasNavigated(true); }); setSyncState(true); - - // Jest is not emulating window.event correctly in the microtask - // TODO: this causes `window.event` to always be popState in this test file - window.event = window.event; } React.useEffect(() => { window.addEventListener('popstate', onPopstate); - return () => window.removeEventListener('popstate', onPopstate); + return () => { + window.removeEventListener('popstate', onPopstate); + }; }, []); Scheduler.log(`render:${hasNavigated}/${syncState}`); return null; @@ -599,34 +594,38 @@ describe('ReactDOMFiberAsync', () => { await act(async () => { const popStateEvent = new Event('popstate'); + // Jest is not emulating window.event correctly in the microtask + window.event = popStateEvent; window.dispatchEvent(popStateEvent); + queueMicrotask(() => { + window.event = undefined; + }); }); assertLog(['popState', 'render:true/true']); - - root.unmount(); + await act(() => { + root.unmount(); + }); }); - it('should not flush transition lanes if there is no transition scheduled in popState', async () => { - let _setHasNavigated; - function App({shouldTransition}) { + it('Should not flush transition lanes if there is no transition scheduled in popState', async () => { + let setHasNavigated; + function App() { const [syncState, setSyncState] = React.useState(false); - const [hasNavigated, setHasNavigated] = React.useState(false); - _setHasNavigated = setHasNavigated; + const [hasNavigated, _setHasNavigated] = React.useState(false); + setHasNavigated = _setHasNavigated; function onPopstate() { setSyncState(true); - - // Jest is not emulating window.event correctly in the microtask - window.event = window.event; } React.useEffect(() => { window.addEventListener('popstate', onPopstate); - return () => window.removeEventListener('popstate', onPopstate); + return () => { + window.removeEventListener('popstate', onPopstate); + }; }, []); Scheduler.log(`render:${hasNavigated}/${syncState}`); - return null; } const root = ReactDOMClient.createRoot(container); @@ -635,16 +634,21 @@ describe('ReactDOMFiberAsync', () => { }); assertLog(['render:false/false']); - // Trigger a transition update to be scheduled React.startTransition(() => { - _setHasNavigated(true); - }) - - // The popState should not flush the transition update - const popStateEvent = new Event('popstate'); - window.dispatchEvent(popStateEvent); - await waitFor(['render:false/true']); - - root.unmount(); + setHasNavigated(true); + }); + await act(async () => { + const popStateEvent = new Event('popstate'); + // Jest is not emulating window.event correctly in the microtask + window.event = popStateEvent; + window.dispatchEvent(popStateEvent); + queueMicrotask(() => { + window.event = undefined; + }); + }); + assertLog(['render:false/true', 'render:true/true']); + await act(() => { + root.unmount(); + }); }); }); diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index d26df7f87d323..86f882a025726 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -246,12 +246,12 @@ function processRootScheduleInMicrotask() { let root = firstScheduledRoot; while (root !== null) { const next = root.next; - - if (currentEventTransitionLane !== NoLane && shouldAttemptEagerTransition()) { - markRootEntangled( - root, - mergeLanes(currentEventTransitionLane, SyncLane), - ); + + if ( + currentEventTransitionLane !== NoLane && + shouldAttemptEagerTransition() + ) { + markRootEntangled(root, mergeLanes(currentEventTransitionLane, SyncLane)); } const nextLanes = scheduleTaskForRootDuringMicrotask(root, currentTime); @@ -284,7 +284,7 @@ function processRootScheduleInMicrotask() { } currentEventTransitionLane = NoLane; - + // At the end of the microtask, flush any pending synchronous work. This has // to come at the end, because it does actual rendering work that might throw. flushSyncWorkOnAllRoots(); From f642605a4536ea076da2820a419c5d71ac6b3b34 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 12 Apr 2023 12:34:54 -0700 Subject: [PATCH 4/7] feedback --- packages/react-reconciler/src/ReactFiberClassUpdateQueue.js | 4 ++-- packages/react-reconciler/src/ReactFiberHooks.js | 4 ++-- packages/react-reconciler/src/ReactFiberLane.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js index 0e67d67e63a28..2e88c982022d3 100644 --- a/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberClassUpdateQueue.js @@ -94,7 +94,7 @@ import { isSubsetOfLanes, mergeLanes, removeLanes, - includesTransitionLane, + isTransitionLane, intersectLanes, markRootEntangled, } from './ReactFiberLane'; @@ -279,7 +279,7 @@ export function entangleTransitions(root: FiberRoot, fiber: Fiber, lane: Lane) { } const sharedQueue: SharedQueue = (updateQueue: any).shared; - if (includesTransitionLane(lane)) { + if (isTransitionLane(lane)) { let queueLanes = sharedQueue.lanes; // If any entangled lanes are no longer pending on the root, then they must diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index cc3decbafad32..4be1994114a32 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -67,7 +67,7 @@ import { mergeLanes, removeLanes, intersectLanes, - includesTransitionLane, + isTransitionLane, markRootEntangled, markRootMutableRead, } from './ReactFiberLane'; @@ -2744,7 +2744,7 @@ function entangleTransitionUpdate( queue: UpdateQueue, lane: Lane, ): void { - if (includesTransitionLane(lane)) { + if (isTransitionLane(lane)) { let queueLanes = queue.lanes; // If any entangled lanes are no longer pending on the root, then they diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 1508d03f90589..835bf01193423 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -494,7 +494,7 @@ export function includesExpiredLane(root: FiberRoot, lanes: Lanes): boolean { return (lanes & root.expiredLanes) !== NoLanes; } -export function includesTransitionLane(lane: Lane): boolean { +export function isTransitionLane(lane: Lane): boolean { return (lane & TransitionLanes) !== NoLanes; } From 7fb616330fddec03d66d8eb3658e6293b18be7a4 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 12 Apr 2023 15:02:45 -0700 Subject: [PATCH 5/7] flow --- packages/react-reconciler/src/forks/ReactFiberConfig.custom.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index e791f63d7ec4a..9e579dca47e7e 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -66,6 +66,8 @@ export const preparePortalMount = $$$config.preparePortalMount; export const prepareScopeUpdate = $$$config.prepareScopeUpdate; export const getInstanceFromScope = $$$config.getInstanceFromScope; export const getCurrentEventPriority = $$$config.getCurrentEventPriority; +export const shouldAttemptEagerTransition = + $$$config.shouldAttemptEagerTransition; export const detachDeletedInstance = $$$config.detachDeletedInstance; export const requestPostPaintCallback = $$$config.requestPostPaintCallback; export const maySuspendCommit = $$$config.maySuspendCommit; From 7a34b06cbdfad58922fd681a3047f38efaeafd6e Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 12 Apr 2023 15:55:12 -0700 Subject: [PATCH 6/7] add test --- .../src/__tests__/ReactDOMFiberAsync-test.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index 75031232adee4..a5da7e45c468a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -651,4 +651,53 @@ describe('ReactDOMFiberAsync', () => { root.unmount(); }); }); + + it('transition lane in popState should yield if it suspends', async () => { + const never = {then() {}}; + let _setText; + + function App() { + const [shouldSuspend, setShouldSuspend] = React.useState(false); + const [text, setText] = React.useState('0'); + _setText = setText; + if (shouldSuspend) { + Scheduler.log('Suspend!'); + throw never; + } + function onPopstate() { + React.startTransition(() => { + setShouldSuspend(val => !val); + }); + } + React.useEffect(() => { + window.addEventListener('popstate', onPopstate); + return () => window.removeEventListener('popstate', onPopstate); + }, []); + Scheduler.log(`Child:${shouldSuspend}/${text}`); + return text; + } + + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + assertLog(['Child:false/0']); + + await act(() => { + const popStateEvent = new Event('popstate'); + window.event = popStateEvent; + window.dispatchEvent(popStateEvent); + queueMicrotask(() => { + window.event = undefined; + }); + }); + assertLog(['Suspend!']); + + await act(async () => { + _setText('1'); + }); + assertLog(['Child:false/1', 'Suspend!']); + + root.unmount(); + }); }); From b340309c386e47f3b8f51cc84ee266f040c51f8d Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 12 Apr 2023 17:18:09 -0700 Subject: [PATCH 7/7] Cleanup window.event --- packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index a5da7e45c468a..79bf341605342 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -39,6 +39,7 @@ describe('ReactDOMFiberAsync', () => { assertLog = InternalTestUtils.assertLog; document.body.appendChild(container); + window.event = undefined; }); afterEach(() => {