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

Use EventPriority to track update priority #21082

Merged
merged 1 commit into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ let ReactFeatureFlags;
let Suspense;
let SuspenseList;
let act;

// Copied from ReactFiberLanes. Don't do this!
// This is hard coded directly to avoid needing to import, and
// we'll remove this as we replace runWithPriority with React APIs.
export const IdleLanePriority = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💖

let IdleEventPriority;

function dispatchMouseEvent(to, from) {
if (!to) {
Expand Down Expand Up @@ -89,6 +85,8 @@ describe('ReactDOMServerPartialHydration', () => {
Scheduler = require('scheduler');
Suspense = React.Suspense;
SuspenseList = React.SuspenseList;

IdleEventPriority = require('react-reconciler/constants').IdleEventPriority;
});

// Note: This is based on a similar component we use in www. We can delete
Expand Down Expand Up @@ -628,7 +626,7 @@ describe('ReactDOMServerPartialHydration', () => {
expect(span.textContent).toBe('Hello');

// Schedule an update at idle priority
ReactDOM.unstable_runWithPriority(IdleLanePriority, () => {
ReactDOM.unstable_runWithPriority(IdleEventPriority, () => {
root.render(<App text="Hi" className="hi" />);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ let Scheduler;
let Suspense;
let act;

// Copied from ReactFiberLanes. Don't do this!
// This is hard coded directly to avoid needing to import, and
// we'll remove this as we replace runWithPriority with React APIs.
export const IdleLanePriority = 2;
let IdleEventPriority;

function dispatchMouseHoverEvent(to, from) {
if (!to) {
Expand Down Expand Up @@ -101,7 +98,7 @@ function dispatchClickEvent(target) {
// and there's no native DOM event that maps to idle priority, so this is a
// temporary workaround. Need something like ReactDOM.unstable_IdleUpdates.
function TODO_scheduleIdleDOMSchedulerTask(fn) {
ReactDOM.unstable_runWithPriority(IdleLanePriority, () => {
ReactDOM.unstable_runWithPriority(IdleEventPriority, () => {
const prevEvent = window.event;
window.event = {type: 'message'};
try {
Expand All @@ -125,6 +122,8 @@ describe('ReactDOMServerSelectiveHydration', () => {
act = ReactTestUtils.unstable_concurrentAct;
Scheduler = require('scheduler');
Suspense = React.Suspense;

IdleEventPriority = require('react-reconciler/constants').IdleEventPriority;
});

// @gate experimental
Expand Down
8 changes: 5 additions & 3 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ import {
attemptDiscreteHydration,
attemptContinuousHydration,
attemptHydrationAtCurrentPriority,
runWithPriority,
getCurrentUpdateLanePriority,
} from 'react-reconciler/src/ReactFiberReconciler';
import {
runWithPriority,
getCurrentUpdatePriority,
} from 'react-reconciler/src/ReactEventPriorities';
import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
import {canUseDOM} from 'shared/ExecutionEnvironment';
import ReactVersion from 'shared/ReactVersion';
Expand Down Expand Up @@ -74,7 +76,7 @@ setAttemptSynchronousHydration(attemptSynchronousHydration);
setAttemptDiscreteHydration(attemptDiscreteHydration);
setAttemptContinuousHydration(attemptContinuousHydration);
setAttemptHydrationAtCurrentPriority(attemptHydrationAtCurrentPriority);
setGetCurrentUpdatePriority(getCurrentUpdateLanePriority);
setGetCurrentUpdatePriority(getCurrentUpdatePriority);
setAttemptHydrationAtPriority(runWithPriority);

let didWarnAboutUnstableCreatePortal = false;
Expand Down
29 changes: 5 additions & 24 deletions packages/react-dom/src/events/ReactDOMEventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,6 @@ import {
discreteUpdates,
} from './ReactDOMUpdateBatching';

import {
InputContinuousLanePriority as InputContinuousLanePriority_old,
getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_old,
setCurrentUpdateLanePriority as setCurrentUpdateLanePriority_old,
} from 'react-reconciler/src/ReactFiberLane.old';
import {
InputContinuousLanePriority as InputContinuousLanePriority_new,
getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_new,
setCurrentUpdateLanePriority as setCurrentUpdateLanePriority_new,
} from 'react-reconciler/src/ReactFiberLane.new';
import {getCurrentPriorityLevel as getCurrentPriorityLevel_old} from 'react-reconciler/src/SchedulerWithReactIntegration.old';
import {
getCurrentPriorityLevel as getCurrentPriorityLevel_new,
Expand All @@ -68,19 +58,10 @@ import {
ContinuousEventPriority,
DefaultEventPriority,
IdleEventPriority,
getCurrentUpdatePriority,
setCurrentUpdatePriority,
} from 'react-reconciler/src/ReactEventPriorities';

// TODO: These should use the opaque EventPriority type instead of LanePriority.
// Then internally we can use a Lane.
const InputContinuousLanePriority = enableNewReconciler
? InputContinuousLanePriority_new
: InputContinuousLanePriority_old;
const getCurrentUpdateLanePriority = enableNewReconciler
? getCurrentUpdateLanePriority_new
: getCurrentUpdateLanePriority_old;
const setCurrentUpdateLanePriority = enableNewReconciler
? setCurrentUpdateLanePriority_new
: setCurrentUpdateLanePriority_old;
const getCurrentPriorityLevel = enableNewReconciler
? getCurrentPriorityLevel_new
: getCurrentPriorityLevel_old;
Expand Down Expand Up @@ -167,12 +148,12 @@ function dispatchContinuousEvent(
container,
nativeEvent,
) {
const previousPriority = getCurrentUpdateLanePriority();
const previousPriority = getCurrentUpdatePriority();
try {
setCurrentUpdateLanePriority(InputContinuousLanePriority);
setCurrentUpdatePriority(ContinuousEventPriority);
dispatchEvent(domEventName, eventSystemFlags, container, nativeEvent);
} finally {
setCurrentUpdateLanePriority(previousPriority);
setCurrentUpdatePriority(previousPriority);
}
}

Expand Down
28 changes: 18 additions & 10 deletions packages/react-dom/src/events/ReactDOMEventReplaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type {Container, SuspenseInstance} from '../client/ReactDOMHostConfig';
import type {DOMEventName} from '../events/DOMEventNames';
import type {EventSystemFlags} from './EventSystemFlags';
import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
import type {LanePriority} from 'react-reconciler/src/ReactFiberLane.old';
import type {EventPriority} from 'react-reconciler/src/ReactEventPriorities';

import {enableSelectiveHydration} from 'shared/ReactFeatureFlags';
import {
Expand All @@ -30,6 +30,7 @@ import {
getClosestInstanceFromNode,
} from '../client/ReactDOMComponentTree';
import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags';
import {isHigherEventPriority} from 'react-reconciler/src/ReactEventPriorities';

let attemptSynchronousHydration: (fiber: Object) => void;

Expand Down Expand Up @@ -57,16 +58,16 @@ export function setAttemptHydrationAtCurrentPriority(
attemptHydrationAtCurrentPriority = fn;
}

let getCurrentUpdatePriority: () => LanePriority;
let getCurrentUpdatePriority: () => EventPriority;

export function setGetCurrentUpdatePriority(fn: () => LanePriority) {
export function setGetCurrentUpdatePriority(fn: () => EventPriority) {
getCurrentUpdatePriority = fn;
}

let attemptHydrationAtPriority: <T>(priority: LanePriority, fn: () => T) => T;
let attemptHydrationAtPriority: <T>(priority: EventPriority, fn: () => T) => T;

export function setAttemptHydrationAtPriority(
fn: <T>(priority: LanePriority, fn: () => T) => T,
fn: <T>(priority: EventPriority, fn: () => T) => T,
) {
attemptHydrationAtPriority = fn;
}
Expand Down Expand Up @@ -109,7 +110,7 @@ const queuedPointerCaptures: Map<number, QueuedReplayableEvent> = new Map();
type QueuedHydrationTarget = {|
blockedOn: null | Container | SuspenseInstance,
target: Node,
lanePriority: LanePriority,
priority: EventPriority,
|};
const queuedExplicitHydrationTargets: Array<QueuedHydrationTarget> = [];

Expand Down Expand Up @@ -390,7 +391,7 @@ function attemptExplicitHydrationTarget(
// We're blocked on hydrating this boundary.
// Increase its priority.
queuedTarget.blockedOn = instance;
attemptHydrationAtPriority(queuedTarget.lanePriority, () => {
attemptHydrationAtPriority(queuedTarget.priority, () => {
attemptHydrationAtCurrentPriority(nearestMounted);
});

Expand All @@ -412,16 +413,23 @@ function attemptExplicitHydrationTarget(

export function queueExplicitHydrationTarget(target: Node): void {
if (enableSelectiveHydration) {
const updateLanePriority = getCurrentUpdatePriority();
// TODO: This will read the priority if it's dispatched by the React
// event system but not native events. Should read window.event.type, like
// we do for updates (getCurrentEventPriority).
const updatePriority = getCurrentUpdatePriority();
const queuedTarget: QueuedHydrationTarget = {
blockedOn: null,
target: target,
lanePriority: updateLanePriority,
priority: updatePriority,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It comes full circle - this was the original name of the field when it was the scheduler priority, which we had to keep alongside lanePriority.

};
let i = 0;
for (; i < queuedExplicitHydrationTargets.length; i++) {
// Stop once we hit the first target with lower priority than
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lower than what?

if (
updateLanePriority <= queuedExplicitHydrationTargets[i].lanePriority
!isHigherEventPriority(
updatePriority,
queuedExplicitHydrationTargets[i].priority,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

) {
break;
}
Expand Down
61 changes: 49 additions & 12 deletions packages/react-reconciler/src/ReactEventPriorities.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,61 @@ import {
ContinuousEventPriority as ContinuousEventPriority_old,
DefaultEventPriority as DefaultEventPriority_old,
IdleEventPriority as IdleEventPriority_old,
getCurrentUpdatePriority as getCurrentUpdatePriority_old,
setCurrentUpdatePriority as setCurrentUpdatePriority_old,
runWithPriority as runWithPriority_old,
isHigherEventPriority as isHigherEventPriority_old,
} from './ReactEventPriorities.old';

import {
DiscreteEventPriority as DiscreteEventPriority_new,
ContinuousEventPriority as ContinuousEventPriority_new,
DefaultEventPriority as DefaultEventPriority_new,
IdleEventPriority as IdleEventPriority_new,
getCurrentUpdatePriority as getCurrentUpdatePriority_new,
setCurrentUpdatePriority as setCurrentUpdatePriority_new,
runWithPriority as runWithPriority_new,
isHigherEventPriority as isHigherEventPriority_new,
} from './ReactEventPriorities.new';

export const DiscreteEventPriority = enableNewReconciler
? DiscreteEventPriority_new
: DiscreteEventPriority_old;
export const ContinuousEventPriority = enableNewReconciler
? ContinuousEventPriority_new
: ContinuousEventPriority_old;
export const DefaultEventPriority = enableNewReconciler
? DefaultEventPriority_new
: DefaultEventPriority_old;
export const IdleEventPriority = enableNewReconciler
? IdleEventPriority_new
: IdleEventPriority_old;
export opaque type EventPriority = number;

export const DiscreteEventPriority: EventPriority = enableNewReconciler
? (DiscreteEventPriority_new: any)
: (DiscreteEventPriority_old: any);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why any type them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host config isn't forked, but it handles types from both reconciler forks. Like Fiber is technically a cross-fork type, because it gets passed between the host config and reconciler. That's why we have the ReactInternalTypes module.

Other than forking all the renderers, there's no way (at least that I've that I've found) to express that the type passed is the correct one for the given fork. So we cheat.

export const ContinuousEventPriority: EventPriority = enableNewReconciler
? (ContinuousEventPriority_new: any)
: (ContinuousEventPriority_old: any);
export const DefaultEventPriority: EventPriority = enableNewReconciler
? (DefaultEventPriority_new: any)
: (DefaultEventPriority_old: any);
export const IdleEventPriority: EventPriority = enableNewReconciler
? (IdleEventPriority_new: any)
: (IdleEventPriority_old: any);

export function runWithPriority<T>(priority: EventPriority, fn: () => T): T {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is only for tests now - how long until we can remove this? We'll need Offscreen for the low priority, but can we replace the Continuous with something that's actually continuous instead?

return enableNewReconciler
? runWithPriority_new((priority: any), fn)
: runWithPriority_old((priority: any), fn);
}

export function getCurrentUpdatePriority(): EventPriority {
return enableNewReconciler
? (getCurrentUpdatePriority_new(): any)
: (getCurrentUpdatePriority_old(): any);
}

export function setCurrentUpdatePriority(priority: EventPriority) {
return enableNewReconciler
? setCurrentUpdatePriority_new((priority: any))
: setCurrentUpdatePriority_old((priority: any));
}

export function isHigherEventPriority(
a: EventPriority,
b: EventPriority,
): boolean {
return enableNewReconciler
? isHigherEventPriority_new((a: any), (b: any))
: isHigherEventPriority_old((a: any), (b: any));
}
70 changes: 65 additions & 5 deletions packages/react-reconciler/src/ReactEventPriorities.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,69 @@
* @flow
*/

export {
SyncLane as DiscreteEventPriority,
InputContinuousLane as ContinuousEventPriority,
DefaultLane as DefaultEventPriority,
IdleLane as IdleEventPriority,
import type {Lane, Lanes} from './ReactFiberLane.new';

import {
NoLane,
SyncLane,
InputContinuousLane,
DefaultLane,
IdleLane,
getHighestPriorityLane,
includesNonIdleWork,
} from './ReactFiberLane.new';

export opaque type EventPriority = Lane;

export const DiscreteEventPriority: EventPriority = SyncLane;
export const ContinuousEventPriority: EventPriority = InputContinuousLane;
export const DefaultEventPriority: EventPriority = DefaultLane;
export const IdleEventPriority: EventPriority = IdleLane;

let currentUpdatePriority: EventPriority = NoLane;

export function getCurrentUpdatePriority(): EventPriority {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: naming, I was thinking I would rename this to getCurrentEventPriority, and then rename the host config one to something like getCurrentHostEventPriority. Gonna leave as-is in this PR, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bother to rename more than this if we're going to remove it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is staying. It tracks a Lane now instead of a LanePriority.

return currentUpdatePriority;
}

export function setCurrentUpdatePriority(newPriority: EventPriority) {
currentUpdatePriority = newPriority;
}

export function runWithPriority<T>(priority: EventPriority, fn: () => T): T {
const previousPriority = currentUpdatePriority;
try {
currentUpdatePriority = priority;
return fn();
} finally {
currentUpdatePriority = previousPriority;
}
}

export function higherEventPriority(
a: EventPriority,
b: EventPriority,
): EventPriority {
return a !== 0 && a < b ? a : b;
}

export function isHigherEventPriority(
a: EventPriority,
b: EventPriority,
): boolean {
return a !== 0 && a < b;
}

export function lanesToEventPriority(lanes: Lanes): EventPriority {
const lane = getHighestPriorityLane(lanes);
if (!isHigherEventPriority(DiscreteEventPriority, lane)) {
return DiscreteEventPriority;
}
if (!isHigherEventPriority(ContinuousEventPriority, lane)) {
return ContinuousEventPriority;
}
if (includesNonIdleWork(lane)) {
return DefaultEventPriority;
}
return IdleEventPriority;
}
Loading