-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Move priorities to separate import to break cycle #21060
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sentence is no longer true |
||
|
||
```js | ||
import { | ||
DiscreteEventPriority, | ||
ContinuousEventPriority, | ||
DefaultEventPriority, | ||
} from './ReactFiberReconciler/src/ReactEventPriorities'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. How does this work? It doesn't exist in the open source npm package. Should this suggest to copy-paste from our repo? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll send a follow-up PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
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. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We currently reference these values in README. If we're removing them, can you update the README with a recommendation of what to do? E.g. just hardcoding numbers or telling people to look at this particular file. https://github.com/facebook/react/blob/master/packages/react-reconciler/README.md#getcurrenteventpriority |
||
|
||
//TODO: "psuedo" is spelled "pseudo" | ||
export const createHasPseudoClassSelector = enableNewReconciler | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This used to be NoLane, now it's DefaultLane. Any differences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that was just a mistake before. NoLanePriority would get passed to
findUpdateLane
, which would throw:react/packages/react-reconciler/src/ReactFiberLane.new.js
Lines 544 to 571 in 25bfa28
I suppose we never hit this internally because we don't have any native event handlers that aren't part of the big switch.
I'll add a regression test as a follow up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I'll address this in the next PR. We're about to remove the LanePriority type in favor of returning a lane directly.