Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
refactors the currentUpdatePriority apis to more clearly communicate …
…when you are reading a raw value and when you are getting a resolved value. This allows requestUpdateLane to do a single call into the renderer to get the update priority while still making a reasonable stack based updating of update priorities ergonomic. I had to expose a `peekCurrentUpdatePriority` to satisfy startTransition which needs to set a new priority only if the current one is too low. I tried to stay away from get/set naming to avoid potential issues with whether you are reading the raw value or the derived one that considers window.event
  • Loading branch information
gnoff committed Apr 5, 2024
commit fcbb36982d6853c672b12eac6ed851949e22e1d8
8 changes: 4 additions & 4 deletions packages/react-art/src/ReactFiberConfigART.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,16 +343,16 @@ export function shouldSetTextContent(type, props) {

let currentUpdatePriority: EventPriority = NoEventPriority;

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

export function getCurrentUpdatePriority() {
export function getCurrentUpdatePriority(): EventPriority {
return currentUpdatePriority;
}

export function getCurrentEventPriority() {
return DefaultEventPriority;
export function resolveUpdatePriority(): EventPriority {
return currentUpdatePriority || DefaultEventPriority;
}

export function shouldAttemptEagerTransition() {
Expand Down
18 changes: 18 additions & 0 deletions packages/react-dom-bindings/src/client/ReactDOMUpdatePriority.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@

import type {EventPriority} from 'react-reconciler/src/ReactEventPriorities';

import {getEventPriority} from '../events/ReactDOMEventListener';
import {
NoEventPriority,
DefaultEventPriority,
} from 'react-reconciler/src/ReactEventPriorities';

import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals';
const ReactDOMCurrentUpdatePriority =
ReactDOMSharedInternals.ReactDOMCurrentUpdatePriority;
Expand All @@ -21,6 +27,18 @@ export function getCurrentUpdatePriority(): EventPriority {
return ReactDOMCurrentUpdatePriority.current;
}

export function resolveUpdatePriority(): EventPriority {
const updatePriority = ReactDOMCurrentUpdatePriority.current;
if (updatePriority !== NoEventPriority) {
return updatePriority;
}
const currentEvent = window.event;
if (currentEvent === undefined) {
return DefaultEventPriority;
}
return getEventPriority(currentEvent.type);
}

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

Choose a reason for hiding this comment

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

It seems like maybe this was kind of broken to begin with since it doesn't opt out of transitions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well it's sorta legacy anyway. Aren't we going to remove from OSS entrypoints?

const previousPriority = getCurrentUpdatePriority();
try {
Expand Down
12 changes: 1 addition & 11 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* @flow
*/

import type {EventPriority} from 'react-reconciler/src/ReactEventPriorities';
import type {DOMEventName} from '../events/DOMEventNames';
import type {Fiber, FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
import type {
Expand All @@ -29,14 +28,14 @@ import type {

import {NotPending} from 'react-dom-bindings/src/shared/ReactDOMFormActions';
import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostContext';
import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities';

import hasOwnProperty from 'shared/hasOwnProperty';
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';

export {
setCurrentUpdatePriority,
getCurrentUpdatePriority,
resolveUpdatePriority,
} from './ReactDOMUpdatePriority';
import {
precacheFiberNode,
Expand Down Expand Up @@ -73,7 +72,6 @@ import {
import {
isEnabled as ReactBrowserEventEmitterIsEnabled,
setEnabled as ReactBrowserEventEmitterSetEnabled,
getEventPriority,
} from '../events/ReactDOMEventListener';
import {SVG_NAMESPACE, MATH_NAMESPACE} from './DOMNamespaces';
import {
Expand Down Expand Up @@ -576,14 +574,6 @@ export function createTextInstance(
return textNode;
}

export function getCurrentEventPriority(): EventPriority {
const currentEvent = window.event;
if (currentEvent === undefined) {
return DefaultEventPriority;
}
return getEventPriority(currentEvent.type);
}

let currentPopstateTransitionEvent: Event | null = null;
export function shouldAttemptEagerTransition(): boolean {
const event = window.event;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ function dispatchDiscreteEvent(
container: EventTarget,
nativeEvent: AnyNativeEvent,
) {
const previousPriority = getCurrentUpdatePriority();
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = null;
const previousPriority = getCurrentUpdatePriority();
try {
setCurrentUpdatePriority(DiscreteEventPriority);
dispatchEvent(domEventName, eventSystemFlags, container, nativeEvent);
Expand All @@ -135,9 +135,9 @@ function dispatchContinuousEvent(
container: EventTarget,
nativeEvent: AnyNativeEvent,
) {
const previousPriority = getCurrentUpdatePriority();
const prevTransition = ReactCurrentBatchConfig.transition;
ReactCurrentBatchConfig.transition = null;
const previousPriority = getCurrentUpdatePriority();
try {
setCurrentUpdatePriority(ContinuousEventPriority);
dispatchEvent(domEventName, eventSystemFlags, container, nativeEvent);
Expand Down
12 changes: 7 additions & 5 deletions packages/react-native-renderer/src/ReactFiberConfigFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,17 +313,19 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
}

let currentUpdatePriority: EventPriority = NoEventPriority;
export function setCurrentUpdatePriority(
newEventPriority: EventPriority,
): void {
currentUpdatePriority = newEventPriority;
export function setCurrentUpdatePriority(newPriority: EventPriority): void {
currentUpdatePriority = newPriority;
}

export function getCurrentUpdatePriority(): EventPriority {
return currentUpdatePriority;
}

export function getCurrentEventPriority(): EventPriority {
export function resolveUpdatePriority(): EventPriority {
if (currentUpdatePriority !== NoEventPriority) {
return currentUpdatePriority;
}

const currentEventPriority = fabricGetCurrentEventPriority
? fabricGetCurrentEventPriority()
: null;
Expand Down
5 changes: 4 additions & 1 deletion packages/react-native-renderer/src/ReactFiberConfigNative.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,10 @@ export function getCurrentUpdatePriority(): EventPriority {
return currentUpdatePriority;
}

export function getCurrentEventPriority(): EventPriority {
export function resolveUpdatePriority(): EventPriority {
if (currentUpdatePriority !== NoEventPriority) {
return currentUpdatePriority;
}
return DefaultEventPriority;
}

Expand Down
5 changes: 4 additions & 1 deletion packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
setCurrentUpdatePriority,
getCurrentUpdatePriority,

getCurrentEventPriority() {
resolveUpdatePriority() {
if (currentUpdatePriority !== NoEventPriority) {
return currentUpdatePriority;
}
return currentEventPriority;
},

Expand Down
4 changes: 4 additions & 0 deletions packages/react-reconciler/src/ReactEventPriorities.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ export function isHigherEventPriority(
return a !== 0 && a < b;
}

export function eventPriorityToLane(updatePriority: EventPriority): Lane {
return updatePriority;
}

export function lanesToEventPriority(lanes: Lanes): EventPriority {
const lane = getHighestPriorityLane(lanes);
if (!isHigherEventPriority(DiscreteEventPriority, lane)) {
Expand Down
4 changes: 1 addition & 3 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFrom
import isArray from 'shared/isArray';
import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {getPublicInstance, getCurrentUpdatePriority} from './ReactFiberConfig';
import {getPublicInstance} from './ReactFiberConfig';
import {
findCurrentUnmaskedContext,
processChildContext,
Expand Down Expand Up @@ -521,8 +521,6 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {
markRetryLaneIfNotHydrated(fiber, lane);
}

export {getCurrentUpdatePriority};

export {findHostInstance};

export {findHostInstanceWithWarning};
Expand Down
41 changes: 12 additions & 29 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ import {
cancelTimeout,
noTimeout,
afterActiveInstanceBlur,
getCurrentEventPriority,
startSuspendingCommit,
waitForCommitToBeReady,
preloadInstance,
supportsHydration,
getCurrentUpdatePriority,
setCurrentUpdatePriority,
getCurrentUpdatePriority,
resolveUpdatePriority,
} from './ReactFiberConfig';

import {createWorkInProgress, resetWorkInProgress} from './ReactFiber';
Expand Down Expand Up @@ -161,6 +161,7 @@ import {
DefaultEventPriority,
lowerEventPriority,
lanesToEventPriority,
eventPriorityToLane,
} from './ReactEventPriorities';
import {requestCurrentTransition} from './ReactFiberTransition';
import {
Expand Down Expand Up @@ -641,25 +642,7 @@ export function requestUpdateLane(fiber: Fiber): Lane {
requestTransitionLane(transition);
}

// Updates originating inside certain React methods, like flushSync, have
// their priority set by tracking it with a context variable.
//
// The opaque type returned by the host config is internally a lane, so we can
// use that directly.
// TODO: Move this type conversion to the event priority module.
const updateLane: Lane = (getCurrentUpdatePriority(): any);
if (updateLane !== NoLane) {
return updateLane;
}

// This update originated outside React. Ask the host environment for an
// appropriate priority, based on the type of event.
//
// The opaque type returned by the host config is internally a lane, so we can
// use that directly.
// TODO: Move this type conversion to the event priority module.
const eventLane: Lane = (getCurrentEventPriority(): any);
return eventLane;
return eventPriorityToLane(resolveUpdatePriority());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should get inlined and it's type safe over a cast

}

function requestRetryLane(fiber: Fiber) {
Expand Down Expand Up @@ -1446,12 +1429,12 @@ export function getExecutionContext(): ExecutionContext {
}

export function deferredUpdates<A>(fn: () => A): A {
const previousPriority = getCurrentUpdatePriority();
const prevTransition = ReactCurrentBatchConfig.transition;

const previousPriority = getCurrentUpdatePriority();
try {
ReactCurrentBatchConfig.transition = null;
setCurrentUpdatePriority(DefaultEventPriority);
ReactCurrentBatchConfig.transition = null;
return fn();
} finally {
setCurrentUpdatePriority(previousPriority);
Expand Down Expand Up @@ -1492,11 +1475,11 @@ export function discreteUpdates<A, B, C, D, R>(
c: C,
d: D,
): R {
const previousPriority = getCurrentUpdatePriority();
const prevTransition = ReactCurrentBatchConfig.transition;
const previousPriority = getCurrentUpdatePriority();
try {
ReactCurrentBatchConfig.transition = null;
setCurrentUpdatePriority(DiscreteEventPriority);
ReactCurrentBatchConfig.transition = null;
return fn(a, b, c, d);
} finally {
setCurrentUpdatePriority(previousPriority);
Expand Down Expand Up @@ -1533,8 +1516,8 @@ export function flushSync<R>(fn: (() => R) | void): R | void {
const previousPriority = getCurrentUpdatePriority();

try {
ReactCurrentBatchConfig.transition = null;
setCurrentUpdatePriority(DiscreteEventPriority);
ReactCurrentBatchConfig.transition = null;
if (fn) {
return fn();
} else {
Expand Down Expand Up @@ -2713,12 +2696,12 @@ function commitRoot(
) {
// TODO: This no longer makes any sense. We already wrap the mutation and
// layout phases. Should be able to remove.
const previousUpdateLanePriority = getCurrentUpdatePriority();
const prevTransition = ReactCurrentBatchConfig.transition;

const previousUpdateLanePriority = getCurrentUpdatePriority();
try {
ReactCurrentBatchConfig.transition = null;
setCurrentUpdatePriority(DiscreteEventPriority);
ReactCurrentBatchConfig.transition = null;
commitRootImpl(
root,
recoverableErrors,
Expand Down Expand Up @@ -3187,8 +3170,8 @@ export function flushPassiveEffects(): boolean {
const previousPriority = getCurrentUpdatePriority();

try {
ReactCurrentBatchConfig.transition = null;
setCurrentUpdatePriority(priority);
ReactCurrentBatchConfig.transition = null;
return flushPassiveEffectsImpl();
} finally {
setCurrentUpdatePriority(previousPriority);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ describe('ReactFiberHostContext', () => {
getCurrentUpdatePriority: function () {
return updatePriority;
},
getCurrentEventPriority: function () {
resolveUpdatePriority: function () {
if (updatePriority !== NoEventPriority) {
return updatePriority;
}
return DefaultEventPriority;
},
shouldAttemptEagerTransition() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const prepareScopeUpdate = $$$config.prepareScopeUpdate;
export const getInstanceFromScope = $$$config.getInstanceFromScope;
export const setCurrentUpdatePriority = $$$config.setCurrentUpdatePriority;
export const getCurrentUpdatePriority = $$$config.getCurrentUpdatePriority;
export const getCurrentEventPriority = $$$config.getCurrentEventPriority;
export const resolveUpdatePriority = $$$config.resolveUpdatePriority;
export const shouldAttemptEagerTransition =
$$$config.shouldAttemptEagerTransition;
export const detachDeletedInstance = $$$config.detachDeletedInstance;
Expand Down
5 changes: 4 additions & 1 deletion packages/react-test-renderer/src/ReactFiberConfigTestHost.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,10 @@ export function getCurrentUpdatePriority(): EventPriority {
return currentUpdatePriority;
}

export function getCurrentEventPriority(): EventPriority {
export function resolveUpdatePriority(): EventPriority {
if (currentUpdatePriority !== NoEventPriority) {
return currentUpdatePriority;
}
return DefaultEventPriority;
}
export function shouldAttemptEagerTransition(): boolean {
Expand Down