Skip to content

Commit

Permalink
Synchronously render the transition lane on popstate
Browse files Browse the repository at this point in the history
  • Loading branch information
tyao1 committed Jan 20, 2023
1 parent ee85098 commit ffb7566
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 9 deletions.
4 changes: 4 additions & 0 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@ export function getCurrentEventPriority() {
return DefaultEventPriority;
}

export function getIsCurrentEventPopState() {
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/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,10 @@ export function getCurrentEventPriority(): EventPriority {
return getEventPriority(currentEvent.type);
}

export function getIsCurrentEventPopState(): 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
80 changes: 80 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,4 +658,84 @@ 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.unstable_yieldValue(`popState`);
setSyncState(true);
React.startTransition(() => {
setHasNavigated(true);
});
}
React.useEffect(() => {
window.addEventListener('popstate', onPopstate);
return () => window.removeEventListener('popstate', onPopstate);
}, []);
Scheduler.unstable_yieldValue(`render:${hasNavigated}/${syncState}`);
return null;
}
const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['render:false/false']);

await act(async () => {
const popStateEvent = new Event('popstate');
window.dispatchEvent(popStateEvent);
});
expect(Scheduler).toHaveYielded(['popState', 'render:true/true']);

root.unmount();
});

it('transition lane in popState should still 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.unstable_yieldValue('Suspend!');
throw never;
}
function onPopstate() {
Scheduler.unstable_yieldValue(`popState`);
React.startTransition(() => {
setShouldSuspend(val => !val);
});
}
React.useEffect(() => {
window.addEventListener('popstate', onPopstate);
return () => window.removeEventListener('popstate', onPopstate);
}, []);
Scheduler.unstable_yieldValue(`Child:${shouldSuspend}/${text}`);
return null;
}

const root = ReactDOMClient.createRoot(container);
await act(async () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['Child:false/0']);

await act(() => {
window.dispatchEvent(new Event('popstate'));
});
expect(Scheduler).toHaveYielded(['popState', 'Suspend!']);

await act(async () => {
_setText(true);
});

// Transition lane yields
expect(Scheduler).toHaveYielded(['Child:false/true', 'Suspend!']);

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

export function getIsCurrentEventPopState(): 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/ReactNativeHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ export function getCurrentEventPriority(): * {
return DefaultEventPriority;
}

export function getIsCurrentEventPopState(): 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 @@ -442,6 +442,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return currentEventPriority;
},

getIsCurrentEventPopState(): boolean {
return false;
},

now: Scheduler.unstable_now,

isPrimaryRenderer: true,
Expand Down
15 changes: 12 additions & 3 deletions packages/react-reconciler/src/ReactFiberLane.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
nextLanes = getHighestPriorityLanes(nonIdlePingedLanes);
}
}
// Batch transitions in `popstate` with the sync lane
if (root.forceSync && isTransitionLane(nonIdleUnblockedLanes)) {
nextLanes |= nonIdleUnblockedLanes & TransitionLanes;
}
} else {
// The only remaining work is Idle.
const unblockedLanes = pendingLanes & ~suspendedLanes;
Expand Down Expand Up @@ -315,7 +319,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
lanes &= ~lane;
}
}

return nextLanes;
}

Expand Down Expand Up @@ -471,8 +474,11 @@ export function getLanesToRetrySynchronouslyOnError(
return NoLanes;
}

export function includesSyncLane(lanes: Lanes): boolean {
return (lanes & (SyncLane | SyncHydrationLane)) !== NoLanes;
export function includesSyncLaneOrForceSync(
lanes: Lanes,
root: FiberRoot,
): boolean {
return (lanes & (SyncLane | SyncHydrationLane)) !== NoLanes || root.forceSync;
}

export function includesNonIdleWork(lanes: Lanes): boolean {
Expand Down Expand Up @@ -633,6 +639,7 @@ export function markRootUpdated(
export function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
root.suspendedLanes |= suspendedLanes;
root.pingedLanes &= ~suspendedLanes;
root.forceSync = false;

// The suspended lanes are no longer CPU-bound. Clear their expiration times.
const expirationTimes = root.expirationTimes;
Expand Down Expand Up @@ -671,6 +678,8 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {

root.errorRecoveryDisabledLanes &= remainingLanes;

root.forceSync = false;

const entanglements = root.entanglements;
const eventTimes = root.eventTimes;
const expirationTimes = root.expirationTimes;
Expand Down
24 changes: 18 additions & 6 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import {
scheduleMicrotask,
prepareRendererToRender,
resetRendererAfterRender,
getIsCurrentEventPopState,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -137,7 +138,7 @@ import {
NoTimestamp,
claimNextTransitionLane,
claimNextRetryLane,
includesSyncLane,
includesSyncLaneOrForceSync,
isSubsetOfLanes,
mergeLanes,
removeLanes,
Expand All @@ -147,6 +148,7 @@ import {
includesOnlyTransitions,
includesBlockingLane,
includesExpiredLane,
isTransitionLane,
getNextLanes,
markStarvedLanesAsExpired,
getLanesToRetrySynchronouslyOnError,
Expand Down Expand Up @@ -727,6 +729,7 @@ export function scheduleUpdateOnFiber(
}

// Mark that the root has a pending update.
markRootWithPopState(root, lane);
markRootUpdated(root, lane, eventTime);

if (
Expand Down Expand Up @@ -914,7 +917,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
// TODO: Temporary until we confirm this warning is not fired.
if (
existingCallbackNode == null &&
!includesSyncLane(existingCallbackPriority)
!includesSyncLaneOrForceSync(existingCallbackPriority, root)
) {
console.error(
'Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue.',
Expand All @@ -932,7 +935,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {

// Schedule a new callback.
let newCallbackNode;
if (includesSyncLane(newCallbackPriority)) {
if (includesSyncLaneOrForceSync(newCallbackPriority, root)) {
// Special case: Sync React callbacks are scheduled on a special
// internal queue
if (root.tag === LegacyRoot) {
Expand Down Expand Up @@ -1455,6 +1458,12 @@ function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
markRootSuspended_dontCallThisOneDirectly(root, suspendedLanes);
}

function markRootWithPopState(root: FiberRoot, updateLane: Lane) {
if (isTransitionLane(updateLane) && getIsCurrentEventPopState()) {
root.forceSync = true;
}
}

// This is the entry point for synchronous tasks that don't go
// through Scheduler
function performSyncWorkOnRoot(root: FiberRoot) {
Expand All @@ -1469,7 +1478,7 @@ function performSyncWorkOnRoot(root: FiberRoot) {
flushPassiveEffects();

let lanes = getNextLanes(root, NoLanes);
if (!includesSyncLane(lanes)) {
if (!includesSyncLaneOrForceSync(lanes, root)) {
// There's no remaining sync work left.
ensureRootIsScheduled(root, now());
return null;
Expand Down Expand Up @@ -2920,13 +2929,16 @@ function commitRootImpl(
// TODO: We can optimize this by not scheduling the callback earlier. Since we
// currently schedule the callback in multiple places, will wait until those
// are consolidated.
if (includesSyncLane(pendingPassiveEffectsLanes) && root.tag !== LegacyRoot) {
if (
includesSyncLaneOrForceSync(pendingPassiveEffectsLanes, root) &&
root.tag !== LegacyRoot
) {
flushPassiveEffects();
}

// Read this again, since a passive effect might have updated it
remainingLanes = root.pendingLanes;
if (includesSyncLane(remainingLanes)) {
if (includesSyncLaneOrForceSync(remainingLanes, root)) {
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
markNestedUpdateScheduled();
}
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactInternalTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ type BaseFiberRootProperties = {
pooledCache: Cache | null,
pooledCacheLanes: Lanes,

forceSync: boolean,

// TODO: In Fizz, id generation is specific to each server config. Maybe we
// should do this in Fiber, too? Deferring this decision for now because
// there's no other place to store the prefix except for an internal field on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ describe('ReactFiberHostContext', () => {
getCurrentEventPriority: function() {
return DefaultEventPriority;
},
getIsCurrentEventPopState() {
return false;
},
requestPostPaintCallback: function() {},
prepareRendererToRender: function() {},
resetRendererAfterRender: function() {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ export const preparePortalMount = $$$hostConfig.preparePortalMount;
export const prepareScopeUpdate = $$$hostConfig.prepareScopeUpdate;
export const getInstanceFromScope = $$$hostConfig.getInstanceFromScope;
export const getCurrentEventPriority = $$$hostConfig.getCurrentEventPriority;
export const getIsCurrentEventPopState =
$$$hostConfig.getIsCurrentEventPopState;
export const detachDeletedInstance = $$$hostConfig.detachDeletedInstance;
export const requestPostPaintCallback = $$$hostConfig.requestPostPaintCallback;
export const prepareRendererToRender = $$$hostConfig.prepareRendererToRender;
Expand Down
3 changes: 3 additions & 0 deletions packages/react-test-renderer/src/ReactTestHostConfig.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 getIsCurrentEventPopState(): boolean {
return false;
}

export const isPrimaryRenderer = false;
export const warnsIfNotActing = true;
Expand Down

0 comments on commit ffb7566

Please sign in to comment.