Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronously flush the transition lane scheduled in a popstate event #26025

Merged
merged 7 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/react-art/src/ReactFiberConfigART.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

beforeEach(() => {
jest.resetModules();
container = document.createElement('div');
React = require('react');
ReactDOM = require('react-dom');
Expand Down Expand Up @@ -566,4 +565,139 @@ 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);
tyao1 marked this conversation as resolved.
Show resolved Hide resolved
queueMicrotask(() => {
window.event = undefined;
});
});
assertLog(['Suspend!']);

await act(async () => {
_setText('1');
});
assertLog(['Child:false/1', 'Suspend!']);

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

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

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

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

shouldAttemptEagerTransition(): boolean {
return false;
},

now: Scheduler.unstable_now,

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

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -849,8 +849,6 @@ 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,6 +70,9 @@ 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,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;
Expand Down
3 changes: 3 additions & 0 deletions packages/react-test-renderer/src/ReactFiberConfigTestHost.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down