From f567431a8fc9fbd3943dadb7c512559cd0170740 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Mon, 17 Apr 2023 16:24:16 -0400 Subject: [PATCH] Revert "Synchronously flush the transition lane scheduled in a popstate event (#26025)" This reverts commit d121c67004a2e6b0bb5d341843663ef213f64863. --- packages/react-art/src/ReactFiberConfigART.js | 4 - .../src/client/ReactFiberConfigDOM.js | 4 - .../src/__tests__/ReactDOMFiberAsync-test.js | 137 +----------------- .../src/ReactFiberConfigFabric.js | 4 - .../src/ReactFiberConfigNative.js | 4 - .../src/createReactNoop.js | 4 - .../src/ReactFiberRootScheduler.js | 28 +--- .../src/ReactFiberWorkLoop.js | 12 +- .../ReactFiberHostContext-test.internal.js | 3 - .../src/forks/ReactFiberConfig.custom.js | 2 - .../src/ReactFiberConfigTestHost.js | 3 - 11 files changed, 9 insertions(+), 196 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index bcf52149fd661..591072cec84e8 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -346,10 +346,6 @@ 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 2315c00547953..93c8950b05575 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -527,10 +527,6 @@ 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 79bf341605342..61bb7a318d617 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -27,6 +27,7 @@ describe('ReactDOMFiberAsync', () => { let container; beforeEach(() => { + jest.resetModules(); container = document.createElement('div'); React = require('react'); ReactDOM = require('react-dom'); @@ -39,7 +40,6 @@ describe('ReactDOMFiberAsync', () => { assertLog = InternalTestUtils.assertLog; document.body.appendChild(container); - window.event = undefined; }); afterEach(() => { @@ -566,139 +566,4 @@ describe('ReactDOMFiberAsync', () => { expect(container.textContent).toBe('new'); }); - - 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); - function onPopstate() { - Scheduler.log(`popState`); - React.startTransition(() => { - setHasNavigated(true); - }); - setSyncState(true); - } - 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'); - // 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']); - await act(() => { - root.unmount(); - }); - }); - - 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; - function onPopstate() { - setSyncState(true); - } - - 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']); - - React.startTransition(() => { - 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(); - }); - }); - - 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(); - }); }); diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 28bdfcb55dc53..5dd36c20baafb 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -334,10 +334,6 @@ 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 5467ecf72d376..b218a1f77c234 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -265,10 +265,6 @@ 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 fe2e382f353ef..f32ec38806b65 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -526,10 +526,6 @@ 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/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 86f882a025726..7206579745871 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -20,8 +20,6 @@ import { getNextLanes, includesSyncLane, markStarvedLanesAsExpired, - markRootEntangled, - mergeLanes, } from './ReactFiberLane'; import { CommitContext, @@ -51,11 +49,7 @@ import { IdleEventPriority, lanesToEventPriority, } from './ReactEventPriorities'; -import { - supportsMicrotasks, - scheduleMicrotask, - shouldAttemptEagerTransition, -} from './ReactFiberConfig'; +import {supportsMicrotasks, scheduleMicrotask} from './ReactFiberConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; const {ReactCurrentActQueue} = ReactSharedInternals; @@ -78,8 +72,6 @@ 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,14 +238,6 @@ function processRootScheduleInMicrotask() { let root = firstScheduledRoot; while (root !== null) { const next = root.next; - - if ( - currentEventTransitionLane !== NoLane && - shouldAttemptEagerTransition() - ) { - markRootEntangled(root, mergeLanes(currentEventTransitionLane, SyncLane)); - } - const nextLanes = scheduleTaskForRootDuringMicrotask(root, currentTime); if (nextLanes === NoLane) { // This root has no more pending work. Remove it from the schedule. To @@ -283,8 +267,6 @@ 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(); @@ -490,11 +472,3 @@ 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 89781a4cfad26..b6ec683ee65fd 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -278,8 +278,6 @@ import { flushSyncWorkOnAllRoots, flushSyncWorkOnLegacyRootsOnly, getContinuationForRoot, - getCurrentEventTransitionLane, - setCurrentEventTransitionLane, } from './ReactFiberRootScheduler'; import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext'; @@ -584,6 +582,8 @@ 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 { @@ -640,11 +640,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 (getCurrentEventTransitionLane() === NoLane) { + if (currentEventTransitionLane === NoLane) { // All transitions within the same event are assigned the same lane. - setCurrentEventTransitionLane(claimNextTransitionLane()); + currentEventTransitionLane = claimNextTransitionLane(); } - return getCurrentEventTransitionLane(); + return currentEventTransitionLane; } // Updates originating inside certain React methods, like flushSync, have @@ -848,6 +848,8 @@ export function performConcurrentWorkOnRoot( resetNestedUpdateFlag(); } + currentEventTransitionLane = NoLanes; + if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { throw new Error('Should not already be working.'); } diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index bfbcddfcb7373..a4c32090bfd8a 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -70,9 +70,6 @@ describe('ReactFiberHostContext', () => { getCurrentEventPriority: function () { return DefaultEventPriority; }, - shouldAttemptEagerTransition() { - return false; - }, requestPostPaintCallback: function () {}, maySuspendCommit(type, props) { return false; diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index 9e579dca47e7e..e791f63d7ec4a 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -66,8 +66,6 @@ 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; diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 3b478b01276db..1b7e9adcee86b 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -213,9 +213,6 @@ export function createTextInstance( export function getCurrentEventPriority(): * { return DefaultEventPriority; } -export function shouldAttemptEagerTransition(): boolean { - return false; -} export const isPrimaryRenderer = false; export const warnsIfNotActing = true;