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

Experiment: Infer the current event priority from the native event #20748

Merged
merged 5 commits into from
Feb 9, 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
12 changes: 12 additions & 0 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@

import Transform from 'art/core/transform';
import Mode from 'art/modes/current';
import {enableNewReconciler} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';

import {TYPES, EVENT_TYPES, childrenAsString} from './ReactARTInternals';

import {DefaultLanePriority as DefaultLanePriority_old} from 'react-reconciler/src/ReactFiberLane.old';
import {DefaultLanePriority as DefaultLanePriority_new} from 'react-reconciler/src/ReactFiberLane.new';

const DefaultLanePriority = enableNewReconciler
Copy link
Contributor

Choose a reason for hiding this comment

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

To address @sebmarkbage's concern about leaking internals, can we move this to a reconciler export? Something like:

import {DefaultEventPriority} from 'react-reconciler'

and

// ReactFiberReconciler.js
export const DefaultEventPriority =  enableNewReconciler ? DefaultLanePriority_new : DefaultLanePriority_old;

Then later we can change that export to be whatever we want, even if it requires us casting to any (though they're all just number anyway).

Copy link
Contributor

Choose a reason for hiding this comment

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

So for example, a future PR would change ReactFiberReconciler to look like:

export const DefaultEventPriority =  enableNewReconciler ? DefaultLane_new : DefaultLane_old;

without needing to change any of the renderers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For default only?

Copy link
Contributor

Choose a reason for hiding this comment

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

All of them

Copy link
Contributor

Choose a reason for hiding this comment

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

Well all of the ones that are needed by the event system, I mean

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can't do this after all, because it creates a circular import chain (Reconciler -> HostConfig -> Reconciler). There are probably smarter ways to break it off but for now I'm keeping the old structure (direct imports) but exporting some constants just for third-party renderers.

? DefaultLanePriority_new
: DefaultLanePriority_old;

const pooledTransform = new Transform();

const NO_CONTEXT = {};
Expand Down Expand Up @@ -340,6 +348,10 @@ export function shouldSetTextContent(type, props) {
);
}

export function getCurrentEventPriority() {
return DefaultLanePriority;
}

// The ART renderer is secondary to the React DOM renderer.
export const isPrimaryRenderer = false;

Expand Down
17 changes: 17 additions & 0 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {validateDOMNesting, updatedAncestorInfo} from './validateDOMNesting';
import {
isEnabled as ReactBrowserEventEmitterIsEnabled,
setEnabled as ReactBrowserEventEmitterSetEnabled,
getEventPriority,
} from '../events/ReactDOMEventListener';
import {getChildNamespace} from '../shared/DOMNamespaces';
import {
Expand All @@ -65,10 +66,18 @@ import {
enableSuspenseServerRenderer,
enableCreateEventHandleAPI,
enableScopeAPI,
enableNewReconciler,
} from 'shared/ReactFeatureFlags';
import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';

import {DefaultLanePriority as DefaultLanePriority_old} from 'react-reconciler/src/ReactFiberLane.old';
import {DefaultLanePriority as DefaultLanePriority_new} from 'react-reconciler/src/ReactFiberLane.new';

const DefaultLanePriority = enableNewReconciler
? DefaultLanePriority_new
: DefaultLanePriority_old;

export type Type = string;
export type Props = {
autoFocus?: boolean,
Expand Down Expand Up @@ -372,6 +381,14 @@ export function createTextInstance(
return textNode;
}

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

export const isPrimaryRenderer = true;
export const warnsIfNotActing = true;
// This initialization code may run even on server environments
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/events/ReactDOMEventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ export function attemptToDispatchEvent(
return null;
}

function getEventPriority(domEventName: DOMEventName) {
export function getEventPriority(domEventName: DOMEventName): * {
switch (domEventName) {
// Used by SimpleEventPlugin:
case 'cancel':
Expand Down
12 changes: 12 additions & 0 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,18 @@ import type {
import {mountSafeCallback_NOT_REALLY_SAFE} from './NativeMethodsMixinUtils';
import {create, diff} from './ReactNativeAttributePayload';

import {enableNewReconciler} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';

import {dispatchEvent} from './ReactFabricEventEmitter';

import {DefaultLanePriority as DefaultLanePriority_old} from 'react-reconciler/src/ReactFiberLane.old';
import {DefaultLanePriority as DefaultLanePriority_new} from 'react-reconciler/src/ReactFiberLane.new';

const DefaultLanePriority = enableNewReconciler
? DefaultLanePriority_new
: DefaultLanePriority_old;

// Modules provided by RN:
import {
ReactNativeViewConfigRegistry,
Expand Down Expand Up @@ -339,6 +347,10 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
return false;
}

export function getCurrentEventPriority(): * {
return DefaultLanePriority;
}

// The Fabric renderer is secondary to the existing React Native renderer.
export const isPrimaryRenderer = false;

Expand Down
12 changes: 12 additions & 0 deletions packages/react-native-renderer/src/ReactNativeHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type {TouchedViewDataAtPoint} from './ReactNativeTypes';

import invariant from 'shared/invariant';
import {enableNewReconciler} from 'shared/ReactFeatureFlags';

// Modules provided by RN:
import {
Expand All @@ -26,6 +27,13 @@ import {
} from './ReactNativeComponentTree';
import ReactNativeFiberHostComponent from './ReactNativeFiberHostComponent';

import {DefaultLanePriority as DefaultLanePriority_old} from 'react-reconciler/src/ReactFiberLane.old';
import {DefaultLanePriority as DefaultLanePriority_new} from 'react-reconciler/src/ReactFiberLane.new';

const DefaultLanePriority = enableNewReconciler
? DefaultLanePriority_new
: DefaultLanePriority_old;

const {get: getViewConfigForType} = ReactNativeViewConfigRegistry;

export type Type = string;
Expand Down Expand Up @@ -261,6 +269,10 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
return false;
}

export function getCurrentEventPriority(): * {
return DefaultLanePriority;
}

// -------------------
// 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 @@ -391,6 +391,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

resetAfterCommit(): void {},

getCurrentEventPriority() {
return NoopRenderer.DefaultEventPriority;
},

now: Scheduler.unstable_now,

isPrimaryRenderer: true,
Expand Down
26 changes: 26 additions & 0 deletions packages/react-reconciler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,32 @@ You can proxy this to `queueMicrotask` or its equivalent in your environment.

This is a property (not a function) that should be set to `true` if your renderer is the main one on the page. For example, if you're writing a renderer for the Terminal, it makes sense to set it to `true`, but if your renderer is used *on top of* React DOM or some other existing renderer, set it to `false`.

#### `getCurrentEventPriority`

To implement this method, you'll need some constants available on the _returned_ `Renderer` object:

```js
const HostConfig = {
// ...
getCurrentEventPriority() {
return MyRenderer.DefaultEventPriority;
},
// ...
}

const MyRenderer = Reconciler(HostConfig);
```

The constant you return depends on which event, if any, is being handled right now. (In the browser, you can check this using `window.event && window.event.type`).

* **Discrete events:** If the active event is _directly caused by the user_ (such as mouse and keyboard events) and _each event in a sequence is intentional_ (e.g. `click`), return `MyRenderer.DiscreteEventPriority`. This tells React that they should interrupt any background work and cannot be batched across time.

* **Continuous events:** If the active event is _directly caused by the user_ but _the user can't distinguish between individual events in a sequence_ (e.g. `mouseover`), return `MyRenderer.ContinuousEventPriority`. This tells React they should interrupt any background work but can be batched across time.

* **Other events / No active event:** In all other cases, return `MyRenderer.DefaultEventPriority`. This tells React that this event is considered background work, and interactive events will be prioritized over it.

You can consult the `getCurrentEventPriority()` implementation in `ReactDOMHostConfig.js` for a reference implementation.

### Mutation Methods

If you're using React in mutation mode (you probably do), you'll need to implement a few more methods.
Expand Down
15 changes: 15 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ import {
registerMutableSourceForHydration as registerMutableSourceForHydration_old,
runWithPriority as runWithPriority_old,
getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_old,
DefaultEventPriority as DefaultEventPriority_old,
DiscreteEventPriority as DiscreteEventPriority_old,
ContinuousEventPriority as ContinuousEventPriority_old,
} from './ReactFiberReconciler.old';

import {
Expand Down Expand Up @@ -92,6 +95,9 @@ import {
registerMutableSourceForHydration as registerMutableSourceForHydration_new,
runWithPriority as runWithPriority_new,
getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_new,
DefaultEventPriority as DefaultEventPriority_new,
DiscreteEventPriority as DiscreteEventPriority_new,
ContinuousEventPriority as ContinuousEventPriority_new,
} from './ReactFiberReconciler.new';

export const createContainer = enableNewReconciler
Expand Down Expand Up @@ -168,6 +174,15 @@ export const createPortal = enableNewReconciler
export const createComponentSelector = enableNewReconciler
? createComponentSelector_new
: createComponentSelector_old;
export const DefaultEventPriority = enableNewReconciler
? DefaultEventPriority_new
: DefaultEventPriority_old;
export const DiscreteEventPriority = enableNewReconciler
? DiscreteEventPriority_new
: DiscreteEventPriority_old;
export const ContinuousEventPriority = enableNewReconciler
? ContinuousEventPriority_new
: ContinuousEventPriority_old;

//TODO: "psuedo" is spelled "pseudo"
export const createHasPsuedoClassSelector = enableNewReconciler
Expand Down
9 changes: 9 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ import {
} from './ReactFiberHotReloading.new';
import {markRenderScheduled} from './SchedulingProfiler';

// Ideally host configs would import these constants from the reconciler
// entry point, but we can't do this because of a circular dependency.
// They are used by third-party renderers so they need to stay up to date.
export {
InputDiscreteLanePriority as DiscreteEventPriority,
InputContinuousLanePriority as ContinuousEventPriority,
DefaultLanePriority as DefaultEventPriority,
} from './ReactFiberLane.new';

export {registerMutableSourceForHydration} from './ReactMutableSource.new';
export {createPortal} from './ReactPortal';
export {
Expand Down
9 changes: 9 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ import {
} from './ReactFiberHotReloading.old';
import {markRenderScheduled} from './SchedulingProfiler';

// Ideally host configs would import these constants from the reconciler
// entry point, but we can't do this because of a circular dependency.
// They are used by third-party renderers so they need to stay up to date.
export {
InputDiscreteLanePriority as DiscreteEventPriority,
InputContinuousLanePriority as ContinuousEventPriority,
DefaultLanePriority as DefaultEventPriority,
} from './ReactFiberLane.old';

export {registerMutableSourceForHydration} from './ReactMutableSource.new';
export {createPortal} from './ReactPortal';
export {
Expand Down
24 changes: 19 additions & 5 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
enableDoubleInvokingEffects,
skipUnmountedBoundaries,
enableTransitionEntanglement,
enableNativeEventPriorityInference,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -94,6 +95,7 @@ import {
afterActiveInstanceBlur,
clearContainer,
scheduleMicrotask,
getCurrentEventPriority,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -461,11 +463,23 @@ export function requestUpdateLane(fiber: Fiber): Lane {
const currentLanePriority = getCurrentUpdateLanePriority();
lane = findUpdateLane(currentLanePriority, currentEventWipLanes);
} else {
const schedulerLanePriority = schedulerPriorityToLanePriority(
schedulerPriority,
);

lane = findUpdateLane(schedulerLanePriority, currentEventWipLanes);
if (enableNativeEventPriorityInference) {
const eventLanePriority = getCurrentEventPriority();
if (eventLanePriority === DefaultLanePriority) {
// TODO: move this case into the ReactDOM host config.
const schedulerLanePriority = schedulerPriorityToLanePriority(
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be in a follow-up, but let's move this Scheduler consultation to React DOM. We only need to check it for message events, since that's the event Scheduler uses internally.

Conceptually, we're asking the renderer "what is the priority of the current platform event?" Scheduler isn't part of the The Platform but it's standing in for postTask which is. So one day we can swap it out for that.

And React Native will probably do something completely different, so makes sense to move this logic to the renderer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, this can return Idle or NoLane priorities. Are these also valid? E.g. would we expose the associated constants and consider them fair game to return from third party renderers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, would it be ok for ReactDOM to reach into schedulerPriorityToLanePriority which is conceptually an internal? I guess we already do in the event system.

schedulerPriority,
);
lane = findUpdateLane(schedulerLanePriority, currentEventWipLanes);
} else {
lane = findUpdateLane(eventLanePriority, currentEventWipLanes);
}
} else {
const schedulerLanePriority = schedulerPriorityToLanePriority(
schedulerPriority,
);
lane = findUpdateLane(schedulerLanePriority, currentEventWipLanes);
}
}

return lane;
Expand Down
24 changes: 19 additions & 5 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
enableDoubleInvokingEffects,
skipUnmountedBoundaries,
enableTransitionEntanglement,
enableNativeEventPriorityInference,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -94,6 +95,7 @@ import {
afterActiveInstanceBlur,
clearContainer,
scheduleMicrotask,
getCurrentEventPriority,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -461,11 +463,23 @@ export function requestUpdateLane(fiber: Fiber): Lane {
const currentLanePriority = getCurrentUpdateLanePriority();
lane = findUpdateLane(currentLanePriority, currentEventWipLanes);
} else {
const schedulerLanePriority = schedulerPriorityToLanePriority(
schedulerPriority,
);

lane = findUpdateLane(schedulerLanePriority, currentEventWipLanes);
if (enableNativeEventPriorityInference) {
const eventLanePriority = getCurrentEventPriority();
if (eventLanePriority === DefaultLanePriority) {
// TODO: move this case into the ReactDOM host config.
const schedulerLanePriority = schedulerPriorityToLanePriority(
schedulerPriority,
);
lane = findUpdateLane(schedulerLanePriority, currentEventWipLanes);
} else {
lane = findUpdateLane(eventLanePriority, currentEventWipLanes);
}
} else {
const schedulerLanePriority = schedulerPriorityToLanePriority(
schedulerPriority,
);
lane = findUpdateLane(schedulerLanePriority, currentEventWipLanes);
}
}

return lane;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export const afterActiveInstanceBlur = $$$hostConfig.afterActiveInstanceBlur;
export const preparePortalMount = $$$hostConfig.preparePortalMount;
export const prepareScopeUpdate = $$$hostConfig.preparePortalMount;
export const getInstanceFromScope = $$$hostConfig.getInstanceFromScope;
export const getCurrentEventPriority = $$$hostConfig.getCurrentEventPriority;

// -------------------
// Test selectors
Expand Down
12 changes: 12 additions & 0 deletions packages/react-test-renderer/src/ReactTestHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
*/

import {REACT_OPAQUE_ID_TYPE} from 'shared/ReactSymbols';
import {enableNewReconciler} from 'shared/ReactFeatureFlags';

import {DefaultLanePriority as DefaultLanePriority_old} from 'react-reconciler/src/ReactFiberLane.old';
import {DefaultLanePriority as DefaultLanePriority_new} from 'react-reconciler/src/ReactFiberLane.new';

const DefaultLanePriority = enableNewReconciler
? DefaultLanePriority_new
: DefaultLanePriority_old;

export type Type = string;
export type Props = Object;
Expand Down Expand Up @@ -213,6 +221,10 @@ export function createTextInstance(
};
}

export function getCurrentEventPriority(): * {
return DefaultLanePriority;
}

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

Expand Down
2 changes: 2 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,5 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;

export const enableDiscreteEventMicroTasks = false;

export const enableNativeEventPriorityInference = false;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;
export const enableNativeEventPriorityInference = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Loading