From d0eaf782930b6e6c575f50bf688e667b9f6795b7 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 23 Mar 2021 15:57:28 -0500 Subject: [PATCH] Move priorities to separate import to break cycle (#21060) The event priority constants exports by the reconciler package are meant to be used by the reconciler (host config) itself. So it doesn't make sense to export them from a module that requires them. To break the cycle, we can move them to a separate module and import that. This looks like a "deep import" of an internal module, which we try to avoid, but conceptually these are part of the public interface of the reconciler module. So, no different than importing from the main `react-reconciler`. We do need to be careful about not mixing these types of imports with implementation details. Those are the ones to really avoid. An unintended benefit of the reconciler fork infra is that it makes deep imports harder. Any module that we treat as "public", like this one, needs to account for the `enableNewReconciler` flag and forward to the correct implementation. --- packages/react-art/src/ReactARTHostConfig.js | 10 +---- .../src/client/ReactDOMHostConfig.js | 10 +---- .../src/events/ReactDOMEventListener.js | 38 +++++++++---------- .../src/ReactFabricHostConfig.js | 10 +---- .../src/ReactNativeHostConfig.js | 10 +---- .../src/createReactNoop.js | 5 ++- packages/react-reconciler/README.md | 14 +++++-- .../src/ReactEventPriorities.js | 37 ++++++++++++++++++ .../src/ReactEventPriorities.new.js | 15 ++++++++ .../src/ReactEventPriorities.old.js | 15 ++++++++ .../src/ReactFiberReconciler.js | 20 ---------- .../src/ReactTestHostConfig.js | 11 +----- scripts/rollup/forks.js | 20 ++++++++++ 13 files changed, 129 insertions(+), 86 deletions(-) create mode 100644 packages/react-reconciler/src/ReactEventPriorities.js create mode 100644 packages/react-reconciler/src/ReactEventPriorities.new.js create mode 100644 packages/react-reconciler/src/ReactEventPriorities.old.js diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index 7ae3941c9253a..f9971b5bb0360 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -7,17 +7,11 @@ 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 - ? DefaultLanePriority_new - : DefaultLanePriority_old; +import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities'; const pooledTransform = new Transform(); @@ -347,7 +341,7 @@ export function shouldSetTextContent(type, props) { } export function getCurrentEventPriority() { - return DefaultLanePriority; + return DefaultEventPriority; } // The ART renderer is secondary to the React DOM renderer. diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 695242ca50dbb..5d5d9393c4bf1 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -67,17 +67,11 @@ 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; +import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities'; export type Type = string; export type Props = { @@ -385,7 +379,7 @@ export function createTextInstance( export function getCurrentEventPriority(): * { const currentEvent = window.event; if (currentEvent === undefined) { - return DefaultLanePriority; + return DefaultEventPriority; } return getEventPriority(currentEvent.type); } diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 1c0bc724b7063..57ec840e61b22 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -14,6 +14,7 @@ import type { } from 'react-reconciler/src/ReactInternalTypes'; import type {Container, SuspenseInstance} from '../client/ReactDOMHostConfig'; import type {DOMEventName} from '../events/DOMEventNames'; +import type {LanePriority} from 'react-reconciler/src/ReactFiberLane.new'; import { isReplayableDiscreteEvent, @@ -49,18 +50,13 @@ import { import { InputContinuousLanePriority as InputContinuousLanePriority_old, - DefaultLanePriority as DefaultLanePriority_old, getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_old, setCurrentUpdateLanePriority as setCurrentUpdateLanePriority_old, } from 'react-reconciler/src/ReactFiberLane.old'; import { InputContinuousLanePriority as InputContinuousLanePriority_new, - DefaultLanePriority as DefaultLanePriority_new, getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_new, setCurrentUpdateLanePriority as setCurrentUpdateLanePriority_new, - SyncLanePriority, - IdleLanePriority, - NoLanePriority, } from 'react-reconciler/src/ReactFiberLane.new'; import {getCurrentPriorityLevel as getCurrentPriorityLevel_old} from 'react-reconciler/src/SchedulerWithReactIntegration.old'; import { @@ -71,14 +67,16 @@ import { NormalPriority as NormalSchedulerPriority, UserBlockingPriority as UserBlockingSchedulerPriority, } from 'react-reconciler/src/SchedulerWithReactIntegration.new'; -import type {LanePriority} from 'react-reconciler/src/ReactFiberLane.new'; +import { + DiscreteEventPriority, + ContinuousEventPriority, + DefaultEventPriority, + IdleEventPriority, +} from 'react-reconciler/src/ReactEventPriorities'; const InputContinuousLanePriority = enableNewReconciler ? InputContinuousLanePriority_new : InputContinuousLanePriority_old; -const DefaultLanePriority = enableNewReconciler - ? DefaultLanePriority_new - : DefaultLanePriority_old; const getCurrentUpdateLanePriority = enableNewReconciler ? getCurrentUpdateLanePriority_new : getCurrentUpdateLanePriority_old; @@ -94,17 +92,17 @@ function schedulerPriorityToLanePriority( ): LanePriority { switch (schedulerPriorityLevel) { case ImmediateSchedulerPriority: - return SyncLanePriority; + return DiscreteEventPriority; case UserBlockingSchedulerPriority: - return InputContinuousLanePriority; + return ContinuousEventPriority; case NormalSchedulerPriority: case LowSchedulerPriority: // TODO: Handle LowSchedulerPriority, somehow. Maybe the same lane as hydration. - return DefaultLanePriority; + return DefaultEventPriority; case IdleSchedulerPriority: - return IdleLanePriority; + return IdleEventPriority; default: - return NoLanePriority; + return DefaultEventPriority; } } @@ -142,13 +140,13 @@ export function createEventListenerWrapperWithPriority( const eventPriority = getEventPriority(domEventName); let listenerWrapper; switch (eventPriority) { - case SyncLanePriority: + case DiscreteEventPriority: listenerWrapper = dispatchDiscreteEvent; break; - case InputContinuousLanePriority: + case ContinuousEventPriority: listenerWrapper = dispatchContinuousEvent; break; - case DefaultLanePriority: + case DefaultEventPriority: default: listenerWrapper = dispatchEvent; break; @@ -407,7 +405,7 @@ export function getEventPriority(domEventName: DOMEventName): * { case 'popstate': case 'select': case 'selectstart': - return SyncLanePriority; + return DiscreteEventPriority; case 'drag': case 'dragenter': case 'dragexit': @@ -427,7 +425,7 @@ export function getEventPriority(domEventName: DOMEventName): * { // eslint-disable-next-line no-fallthrough case 'mouseenter': case 'mouseleave': - return InputContinuousLanePriority; + return ContinuousEventPriority; case 'message': { // We might be in the Scheduler callback. // Eventually this mechanism will be replaced by a check @@ -436,6 +434,6 @@ export function getEventPriority(domEventName: DOMEventName): * { return schedulerPriorityToLanePriority(schedulerPriority); } default: - return DefaultLanePriority; + return DefaultEventPriority; } } diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 347ace3b79190..ac4a97bb4609e 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -21,17 +21,11 @@ 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; +import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities'; // Modules provided by RN: import { @@ -349,7 +343,7 @@ export function shouldSetTextContent(type: string, props: Props): boolean { } export function getCurrentEventPriority(): * { - return DefaultLanePriority; + return DefaultEventPriority; } // The Fabric renderer is secondary to the existing React Native renderer. diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 84538532979e1..205f8189b7a7c 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -10,7 +10,6 @@ import type {TouchedViewDataAtPoint} from './ReactNativeTypes'; import invariant from 'shared/invariant'; -import {enableNewReconciler} from 'shared/ReactFeatureFlags'; // Modules provided by RN: import { @@ -27,12 +26,7 @@ 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; +import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities'; const {get: getViewConfigForType} = ReactNativeViewConfigRegistry; @@ -268,7 +262,7 @@ export function shouldSetTextContent(type: string, props: Props): boolean { } export function getCurrentEventPriority(): * { - return DefaultLanePriority; + return DefaultEventPriority; } // ------------------- diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index dd304fce6d878..6cef4f7ade498 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -27,6 +27,9 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import enqueueTask from 'shared/enqueueTask'; const {IsSomeRendererActing} = ReactSharedInternals; +// TODO: Publish public entry point that exports the event priority constants +const DefaultEventPriority = 8; + type Container = { rootID: string, children: Array, @@ -587,7 +590,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { const roots = new Map(); const DEFAULT_ROOT_ID = ''; - let currentEventPriority = NoopRenderer.DefaultEventPriority; + let currentEventPriority = DefaultEventPriority; function childToJSX(child, text) { if (text !== null) { diff --git a/packages/react-reconciler/README.md b/packages/react-reconciler/README.md index 60c515610c212..4a0ce2aa427d9 100644 --- a/packages/react-reconciler/README.md +++ b/packages/react-reconciler/README.md @@ -219,10 +219,16 @@ This is a property (not a function) that should be set to `true` if your rendere To implement this method, you'll need some constants available on the _returned_ `Renderer` object: ```js +import { + DiscreteEventPriority, + ContinuousEventPriority, + DefaultEventPriority, +} from './ReactFiberReconciler/src/ReactEventPriorities'; + const HostConfig = { // ... getCurrentEventPriority() { - return MyRenderer.DefaultEventPriority; + return DefaultEventPriority; }, // ... } @@ -232,11 +238,11 @@ 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. +* **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 `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. +* **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 `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. +* **Other events / No active event:** In all other cases, return `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. diff --git a/packages/react-reconciler/src/ReactEventPriorities.js b/packages/react-reconciler/src/ReactEventPriorities.js new file mode 100644 index 0000000000000..76385f612afe9 --- /dev/null +++ b/packages/react-reconciler/src/ReactEventPriorities.js @@ -0,0 +1,37 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {enableNewReconciler} from 'shared/ReactFeatureFlags'; + +import { + DiscreteEventPriority as DiscreteEventPriority_old, + ContinuousEventPriority as ContinuousEventPriority_old, + DefaultEventPriority as DefaultEventPriority_old, + IdleEventPriority as IdleEventPriority_old, +} from './ReactEventPriorities.old'; + +import { + DiscreteEventPriority as DiscreteEventPriority_new, + ContinuousEventPriority as ContinuousEventPriority_new, + DefaultEventPriority as DefaultEventPriority_new, + IdleEventPriority as IdleEventPriority_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; diff --git a/packages/react-reconciler/src/ReactEventPriorities.new.js b/packages/react-reconciler/src/ReactEventPriorities.new.js new file mode 100644 index 0000000000000..4f127a522783c --- /dev/null +++ b/packages/react-reconciler/src/ReactEventPriorities.new.js @@ -0,0 +1,15 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export { + SyncLanePriority as DiscreteEventPriority, + InputContinuousLanePriority as ContinuousEventPriority, + DefaultLanePriority as DefaultEventPriority, + IdleLanePriority as IdleEventPriority, +} from './ReactFiberLane.new'; diff --git a/packages/react-reconciler/src/ReactEventPriorities.old.js b/packages/react-reconciler/src/ReactEventPriorities.old.js new file mode 100644 index 0000000000000..2d6a60eb88178 --- /dev/null +++ b/packages/react-reconciler/src/ReactEventPriorities.old.js @@ -0,0 +1,15 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export { + SyncLanePriority as DiscreteEventPriority, + InputContinuousLanePriority as ContinuousEventPriority, + DefaultLanePriority as DefaultEventPriority, + IdleLanePriority as IdleEventPriority, +} from './ReactFiberLane.old'; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index f1e1ac64645c8..56e517f9f53dd 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -52,10 +52,6 @@ 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, - IdleEventPriority as IdleEventPriority_old, } from './ReactFiberReconciler.old'; import { @@ -96,10 +92,6 @@ 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, - IdleEventPriority as IdleEventPriority_new, } from './ReactFiberReconciler.new'; export const createContainer = enableNewReconciler @@ -176,18 +168,6 @@ 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; -export const IdleEventPriority = enableNewReconciler - ? IdleEventPriority_new - : IdleEventPriority_old; //TODO: "psuedo" is spelled "pseudo" export const createHasPseudoClassSelector = enableNewReconciler diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index bac900c469fa7..596615ea05705 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -8,14 +8,7 @@ */ 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; +import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities'; export type Type = string; export type Props = Object; @@ -223,7 +216,7 @@ export function createTextInstance( } export function getCurrentEventPriority(): * { - return DefaultLanePriority; + return DefaultEventPriority; } export const isPrimaryRenderer = false; diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js index af295ad53c3b8..2c64c6219e35d 100644 --- a/scripts/rollup/forks.js +++ b/scripts/rollup/forks.js @@ -279,6 +279,26 @@ const forks = Object.freeze({ return 'react-reconciler/src/ReactFiberReconciler.old.js'; }, + 'react-reconciler/src/ReactEventPriorities': ( + bundleType, + entry, + dependencies, + moduleType, + bundle + ) => { + if (bundle.enableNewReconciler) { + switch (bundleType) { + case FB_WWW_DEV: + case FB_WWW_PROD: + case FB_WWW_PROFILING: + // Use the forked version of the reconciler + return 'react-reconciler/src/ReactEventPriorities.new.js'; + } + } + // Otherwise, use the non-forked version. + return 'react-reconciler/src/ReactEventPriorities.old.js'; + }, + 'react-reconciler/src/ReactFiberHotReloading': ( bundleType, entry,