Skip to content

Commit

Permalink
Revert "Synchronously flush the transition lane scheduled in a popsta…
Browse files Browse the repository at this point in the history
…te event (facebook#26025)"

This reverts commit d121c67.
  • Loading branch information
kassens committed Apr 17, 2023
1 parent 0fe1bfa commit f567431
Show file tree
Hide file tree
Showing 11 changed files with 9 additions and 196 deletions.
4 changes: 0 additions & 4 deletions packages/react-art/src/ReactFiberConfigART.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 0 additions & 4 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
137 changes: 1 addition & 136 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('ReactDOMFiberAsync', () => {
let container;

beforeEach(() => {
jest.resetModules();
container = document.createElement('div');
React = require('react');
ReactDOM = require('react-dom');
Expand All @@ -39,7 +40,6 @@ describe('ReactDOMFiberAsync', () => {
assertLog = InternalTestUtils.assertLog;

document.body.appendChild(container);
window.event = undefined;
});

afterEach(() => {
Expand Down Expand Up @@ -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(<App />);
});
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(<App />);
});
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(<App />);
});
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();
});
});
4 changes: 0 additions & 4 deletions packages/react-native-renderer/src/ReactFiberConfigFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 0 additions & 4 deletions packages/react-native-renderer/src/ReactFiberConfigNative.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,6 @@ export function getCurrentEventPriority(): * {
return DefaultEventPriority;
}

export function shouldAttemptEagerTransition(): boolean {
return false;
}

// -------------------
// Mutation
// -------------------
Expand Down
4 changes: 0 additions & 4 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return currentEventPriority;
},

shouldAttemptEagerTransition(): boolean {
return false;
},

now: Scheduler.unstable_now,

isPrimaryRenderer: true,
Expand Down
28 changes: 1 addition & 27 deletions packages/react-reconciler/src/ReactFiberRootScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import {
getNextLanes,
includesSyncLane,
markStarvedLanesAsExpired,
markRootEntangled,
mergeLanes,
} from './ReactFiberLane';
import {
CommitContext,
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
12 changes: 7 additions & 5 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,6 @@ import {
flushSyncWorkOnAllRoots,
flushSyncWorkOnLegacyRootsOnly,
getContinuationForRoot,
getCurrentEventTransitionLane,
setCurrentEventTransitionLane,
} from './ReactFiberRootScheduler';
import {getMaskedContext, getUnmaskedContext} from './ReactFiberContext';

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -848,6 +848,8 @@ export function performConcurrentWorkOnRoot(
resetNestedUpdateFlag();
}

currentEventTransitionLane = NoLanes;

if ((executionContext & (RenderContext | CommitContext)) !== NoContext) {
throw new Error('Should not already be working.');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ describe('ReactFiberHostContext', () => {
getCurrentEventPriority: function () {
return DefaultEventPriority;
},
shouldAttemptEagerTransition() {
return false;
},
requestPostPaintCallback: function () {},
maySuspendCommit(type, props) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions packages/react-test-renderer/src/ReactFiberConfigTestHost.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit f567431

Please sign in to comment.