@samitouri / QOS-React / commits / faa697f4f9

Set current update lane priority for user blocking events (#19342)

* Set current update lane priority for user blocking events * Update to use LanePriority and not use runWithPriority * Remove unused imports * Fix tests, and I missed ReactDOMEventListener * Fix more tests * Add try/finally and hardcode lane priorities instead * Also hard code InputContinuousLanePriority in tests * Remove un-needed exports * Comment rollbacks

Ricky committed Jul 17, 2020 at 12:58 UTC faa697f4f9afe9f1c98e315b2a9e70f5a74a7a74
10 files changed +106 -104
packages/react-dom/src/client/ReactDOM.js
+2 -2
@@ -36,7 +36,7 @@ import {
36 attemptContinuousHydration,
37 attemptHydrationAtCurrentPriority,
38 runWithPriority,
39 - getCurrentUpdatePriority,
39 + getCurrentUpdateLanePriority,
40 } from 'react-reconciler/src/ReactFiberReconciler';
41 import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
42 import {canUseDOM} from 'shared/ExecutionEnvironment';
@@ -74,7 +74,7 @@ setAttemptSynchronousHydration(attemptSynchronousHydration);
74 setAttemptUserBlockingHydration(attemptUserBlockingHydration);
75 setAttemptContinuousHydration(attemptContinuousHydration);
76 setAttemptHydrationAtCurrentPriority(attemptHydrationAtCurrentPriority);
77 -setGetCurrentUpdatePriority(getCurrentUpdatePriority);
77 +setGetCurrentUpdatePriority(getCurrentUpdateLanePriority);
78 setAttemptHydrationAtPriority(runWithPriority);
79
80 let didWarnAboutUnstableCreatePortal = false;
packages/react-dom/src/events/DeprecatedDOMEventResponderSystem.js
+16 -3
@@ -56,6 +56,13 @@ import {
56 // Intentionally not named imports because Rollup would use dynamic dispatch for
57 // CommonJS interop named imports.
58 import * as Scheduler from 'scheduler';
59 +
60 +import {
61 + InputContinuousLanePriority,
62 + getCurrentUpdateLanePriority,
63 + setCurrentUpdateLanePriority,
64 +} from 'react-reconciler/src/ReactFiberLane';
65 +
66 const {
67 unstable_UserBlockingPriority: UserBlockingPriority,
68 unstable_runWithPriority: runWithPriority,
@@ -101,9 +108,15 @@ const eventResponderContext: ReactDOMResponderContext = {
108 break;
109 }
110 case UserBlockingEvent: {
104 - runWithPriority(UserBlockingPriority, () =>
105 - executeUserEventHandler(eventListener, eventValue),
106 - );
111 + const previousPriority = getCurrentUpdateLanePriority();
112 + try {
113 + setCurrentUpdateLanePriority(InputContinuousLanePriority);
114 + runWithPriority(UserBlockingPriority, () =>
115 + executeUserEventHandler(eventListener, eventValue),
116 + );
117 + } finally {
118 + setCurrentUpdateLanePriority(previousPriority);
119 + }
120 break;
121 }
122 case ContinuousEvent: {
packages/react-dom/src/events/ReactDOMEventListener.js
+22 -10
@@ -56,6 +56,11 @@ import {
56 flushDiscreteUpdatesIfNeeded,
57 discreteUpdates,
58 } from './ReactDOMUpdateBatching';
59 +import {
60 + InputContinuousLanePriority,
61 + getCurrentUpdateLanePriority,
62 + setCurrentUpdateLanePriority,
63 +} from 'react-reconciler/src/ReactFiberLane';
64
65 const {
66 unstable_UserBlockingPriority: UserBlockingPriority,
@@ -148,16 +153,23 @@ function dispatchUserBlockingUpdate(
153 container,
154 nativeEvent,
155 ) {
151 - runWithPriority(
152 - UserBlockingPriority,
153 - dispatchEvent.bind(
154 - null,
155 - topLevelType,
156 - eventSystemFlags,
157 - container,
158 - nativeEvent,
159 - ),
160 - );
156 + // TODO: Double wrapping is necessary while we decouple Scheduler priority.
157 + const previousPriority = getCurrentUpdateLanePriority();
158 + try {
159 + setCurrentUpdateLanePriority(InputContinuousLanePriority);
160 + runWithPriority(
161 + UserBlockingPriority,
162 + dispatchEvent.bind(
163 + null,
164 + topLevelType,
165 + eventSystemFlags,
166 + container,
167 + nativeEvent,
168 + ),
169 + );
170 + } finally {
171 + setCurrentUpdateLanePriority(previousPriority);
172 + }
173 }
174
175 export function dispatchEvent(
packages/react-dom/src/events/ReactDOMEventReplaying.js
+7 -12
@@ -12,10 +12,8 @@ import type {Container, SuspenseInstance} from '../client/ReactDOMHostConfig';
12 import type {DOMTopLevelEventType} from '../events/TopLevelEventTypes';
13 import type {ElementListenerMap} from '../client/ReactDOMComponentTree';
14 import type {EventSystemFlags} from './EventSystemFlags';
15 -import type {
16 - FiberRoot,
17 - ReactPriorityLevel,
18 -} from 'react-reconciler/src/ReactInternalTypes';
15 +import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
16 +import type {LanePriority} from 'react-reconciler/src/ReactFiberLane';
17
18 import {
19 enableDeprecatedFlareAPI,
@@ -67,19 +65,16 @@ export function setAttemptHydrationAtCurrentPriority(
65 attemptHydrationAtCurrentPriority = fn;
66 }
67
70 -let getCurrentUpdatePriority: () => ReactPriorityLevel;
68 +let getCurrentUpdatePriority: () => LanePriority;
69
72 -export function setGetCurrentUpdatePriority(fn: () => ReactPriorityLevel) {
70 +export function setGetCurrentUpdatePriority(fn: () => LanePriority) {
71 getCurrentUpdatePriority = fn;
72 }
73
76 -let attemptHydrationAtPriority: <T>(
77 - priority: ReactPriorityLevel,
78 - fn: () => T,
79 -) => T;
74 +let attemptHydrationAtPriority: <T>(priority: LanePriority, fn: () => T) => T;
75
76 export function setAttemptHydrationAtPriority(
82 - fn: <T>(priority: ReactPriorityLevel, fn: () => T) => T,
77 + fn: <T>(priority: LanePriority, fn: () => T) => T,
78 ) {
79 attemptHydrationAtPriority = fn;
80 }
@@ -170,7 +165,7 @@ type QueuedHydrationTarget = {|
165 blockedOn: null | Container | SuspenseInstance,
166 target: Node,
167 priority: number,
173 - lanePriority: ReactPriorityLevel,
168 + lanePriority: LanePriority,
169 |};
170 const queuedExplicitHydrationTargets: Array<QueuedHydrationTarget> = [];
171
packages/react-noop-renderer/src/ReactNoop.js
+1 -2
@@ -47,8 +47,7 @@ export const {
47 act,
48 dumpTree,
49 getRoot,
50 - // TODO: Remove this once callers migrate to alternatives.
51 - // This should only be used by React internals.
50 + // TODO: Remove this after callers migrate to alternatives.
51 unstable_runWithPriority,
52 } = createReactNoop(
53 ReactFiberReconciler, // reconciler
packages/react-reconciler/src/ReactFiberReconciler.js
+5 -6
@@ -51,7 +51,7 @@ import {
51 observeVisibleRects as observeVisibleRects_old,
52 registerMutableSourceForHydration as registerMutableSourceForHydration_old,
53 runWithPriority as runWithPriority_old,
54 - getCurrentUpdatePriority as getCurrentUpdatePriority_old,
54 + getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_old,
55 } from './ReactFiberReconciler.old';
56
57 import {
@@ -91,7 +91,7 @@ import {
91 observeVisibleRects as observeVisibleRects_new,
92 registerMutableSourceForHydration as registerMutableSourceForHydration_new,
93 runWithPriority as runWithPriority_new,
94 - getCurrentUpdatePriority as getCurrentUpdatePriority_new,
94 + getCurrentUpdateLanePriority as getCurrentUpdateLanePriority_new,
95 } from './ReactFiberReconciler.new';
96
97 export const createContainer = enableNewReconciler
@@ -143,9 +143,9 @@ export const attemptContinuousHydration = enableNewReconciler
143 export const attemptHydrationAtCurrentPriority = enableNewReconciler
144 ? attemptHydrationAtCurrentPriority_new
145 : attemptHydrationAtCurrentPriority_old;
146 -export const getCurrentUpdatePriority = enableNewReconciler
147 - ? getCurrentUpdatePriority_new
148 - : getCurrentUpdatePriority_old;
146 +export const getCurrentUpdateLanePriority = enableNewReconciler
147 + ? getCurrentUpdateLanePriority_new
148 + : getCurrentUpdateLanePriority_old;
149 export const findHostInstance = enableNewReconciler
150 ? findHostInstance_new
151 : findHostInstance_old;
@@ -197,7 +197,6 @@ export const focusWithin = enableNewReconciler
197 export const observeVisibleRects = enableNewReconciler
198 ? observeVisibleRects_new
199 : observeVisibleRects_old;
200 -
200 export const registerMutableSourceForHydration = enableNewReconciler
201 ? registerMutableSourceForHydration_new
202 : registerMutableSourceForHydration_old;
packages/react-reconciler/src/ReactFiberReconciler.new.js
+5 -13
@@ -7,11 +7,7 @@
7 * @flow
8 */
9
10 -import type {
11 - Fiber,
12 - ReactPriorityLevel,
13 - SuspenseHydrationCallbacks,
14 -} from './ReactInternalTypes';
10 +import type {Fiber, SuspenseHydrationCallbacks} from './ReactInternalTypes';
11 import type {FiberRoot} from './ReactInternalTypes';
12 import type {RootTag} from './ReactRootTags';
13 import type {
@@ -23,7 +19,7 @@ import type {
19 import type {RendererInspectionConfig} from './ReactFiberHostConfig';
20 import {FundamentalComponent} from './ReactWorkTags';
21 import type {ReactNodeList} from 'shared/ReactTypes';
26 -import type {Lane} from './ReactFiberLane';
22 +import type {Lane, LanePriority} from './ReactFiberLane';
23 import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
24
25 import {
@@ -86,8 +82,6 @@ import {
82 higherPriorityLane,
83 getCurrentUpdateLanePriority,
84 setCurrentUpdateLanePriority,
89 - schedulerPriorityToLanePriority,
90 - lanePriorityToSchedulerPriority,
85 } from './ReactFiberLane';
86 import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
87 import {
@@ -438,19 +432,17 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {
432 markRetryLaneIfNotHydrated(fiber, lane);
433 }
434
441 -export function runWithPriority<T>(priority: ReactPriorityLevel, fn: () => T) {
435 +export function runWithPriority<T>(priority: LanePriority, fn: () => T) {
436 const previousPriority = getCurrentUpdateLanePriority();
437 try {
444 - setCurrentUpdateLanePriority(schedulerPriorityToLanePriority(priority));
438 + setCurrentUpdateLanePriority(priority);
439 return fn();
440 } finally {
441 setCurrentUpdateLanePriority(previousPriority);
442 }
443 }
444
451 -export function getCurrentUpdatePriority(): ReactPriorityLevel {
452 - return lanePriorityToSchedulerPriority(getCurrentUpdateLanePriority());
453 -}
445 +export {getCurrentUpdateLanePriority};
446
447 export {findHostInstance};
448
packages/react-reconciler/src/ReactFiberReconciler.old.js
+5 -13
@@ -7,11 +7,7 @@
7 * @flow
8 */
9
10 -import type {
11 - Fiber,
12 - ReactPriorityLevel,
13 - SuspenseHydrationCallbacks,
14 -} from './ReactInternalTypes';
10 +import type {Fiber, SuspenseHydrationCallbacks} from './ReactInternalTypes';
11 import type {FiberRoot} from './ReactInternalTypes';
12 import type {RootTag} from './ReactRootTags';
13 import type {
@@ -23,7 +19,7 @@ import type {
19 import type {RendererInspectionConfig} from './ReactFiberHostConfig';
20 import {FundamentalComponent} from './ReactWorkTags';
21 import type {ReactNodeList} from 'shared/ReactTypes';
26 -import type {Lane} from './ReactFiberLane';
22 +import type {Lane, LanePriority} from './ReactFiberLane';
23 import type {SuspenseState} from './ReactFiberSuspenseComponent.old';
24
25 import {
@@ -86,8 +82,6 @@ import {
82 higherPriorityLane,
83 getCurrentUpdateLanePriority,
84 setCurrentUpdateLanePriority,
89 - schedulerPriorityToLanePriority,
90 - lanePriorityToSchedulerPriority,
85 } from './ReactFiberLane';
86 import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
87 import {
@@ -438,19 +432,17 @@ export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {
432 markRetryLaneIfNotHydrated(fiber, lane);
433 }
434
441 -export function runWithPriority<T>(priority: ReactPriorityLevel, fn: () => T) {
435 +export function runWithPriority<T>(priority: LanePriority, fn: () => T) {
436 const previousPriority = getCurrentUpdateLanePriority();
437 try {
444 - setCurrentUpdateLanePriority(schedulerPriorityToLanePriority(priority));
438 + setCurrentUpdateLanePriority(priority);
439 return fn();
440 } finally {
441 setCurrentUpdateLanePriority(previousPriority);
442 }
443 }
444
451 -export function getCurrentUpdatePriority(): ReactPriorityLevel {
452 - return lanePriorityToSchedulerPriority(getCurrentUpdateLanePriority());
453 -}
445 +export {getCurrentUpdateLanePriority};
446
447 export {findHostInstance};
448
packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
+33 -36
@@ -14,6 +14,11 @@ let React;
14 let ReactNoop;
15 let Scheduler;
16
17 +// Copied from ReactFiberLanes. Don't do this!
18 +// This is hard coded directly to avoid needing to import, and
19 +// we'll remove this as we replace runWithPriority with React APIs.
20 +const InputContinuousLanePriority = 12;
21 +
22 describe('ReactIncrementalUpdates', () => {
23 beforeEach(() => {
24 jest.resetModuleRegistry();
@@ -517,15 +522,13 @@ describe('ReactIncrementalUpdates', () => {
522 if (log === 'B') {
523 // Right after B commits, schedule additional updates.
524 // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority.
520 - ReactNoop.unstable_runWithPriority(
521 - Scheduler.unstable_UserBlockingPriority,
522 - () =>
523 - Scheduler.unstable_runWithPriority(
524 - Scheduler.unstable_UserBlockingPriority,
525 - () => {
526 - pushToLog('C');
527 - },
528 - ),
525 + ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () =>
526 + Scheduler.unstable_runWithPriority(
527 + Scheduler.unstable_UserBlockingPriority,
528 + () => {
529 + pushToLog('C');
530 + },
531 + ),
532 );
533 setLog(prevLog => prevLog + 'D');
534 }
@@ -545,15 +548,13 @@ describe('ReactIncrementalUpdates', () => {
548 pushToLog('A');
549
550 // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority.
548 - ReactNoop.unstable_runWithPriority(
549 - Scheduler.unstable_UserBlockingPriority,
550 - () =>
551 - Scheduler.unstable_runWithPriority(
552 - Scheduler.unstable_UserBlockingPriority,
553 - () => {
554 - pushToLog('B');
555 - },
556 - ),
551 + ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () =>
552 + Scheduler.unstable_runWithPriority(
553 + Scheduler.unstable_UserBlockingPriority,
554 + () => {
555 + pushToLog('B');
556 + },
557 + ),
558 );
559 });
560 expect(Scheduler).toHaveYielded([
@@ -586,15 +587,13 @@ describe('ReactIncrementalUpdates', () => {
587 if (this.state.log === 'B') {
588 // Right after B commits, schedule additional updates.
589 // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority.
589 - ReactNoop.unstable_runWithPriority(
590 - Scheduler.unstable_UserBlockingPriority,
591 - () =>
592 - Scheduler.unstable_runWithPriority(
593 - Scheduler.unstable_UserBlockingPriority,
594 - () => {
595 - this.pushToLog('C');
596 - },
597 - ),
590 + ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () =>
591 + Scheduler.unstable_runWithPriority(
592 + Scheduler.unstable_UserBlockingPriority,
593 + () => {
594 + this.pushToLog('C');
595 + },
596 + ),
597 );
598 this.pushToLog('D');
599 }
@@ -615,15 +614,13 @@ describe('ReactIncrementalUpdates', () => {
614 await ReactNoop.act(async () => {
615 pushToLog('A');
616 // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority.
618 - ReactNoop.unstable_runWithPriority(
619 - Scheduler.unstable_UserBlockingPriority,
620 - () =>
621 - Scheduler.unstable_runWithPriority(
622 - Scheduler.unstable_UserBlockingPriority,
623 - () => {
624 - pushToLog('B');
625 - },
626 - ),
617 + ReactNoop.unstable_runWithPriority(InputContinuousLanePriority, () =>
618 + Scheduler.unstable_runWithPriority(
619 + Scheduler.unstable_UserBlockingPriority,
620 + () => {
621 + pushToLog('B');
622 + },
623 + ),
624 );
625 });
626 expect(Scheduler).toHaveYielded([
packages/react/src/__tests__/ReactDOMTracing-test.internal.js
+10 -7
@@ -23,6 +23,11 @@ let onWorkScheduled;
23 let onWorkStarted;
24 let onWorkStopped;
25
26 +// Copied from ReactFiberLanes. Don't do this!
27 +// This is hard coded directly to avoid needing to import, and
28 +// we'll remove this as we replace runWithPriority with React APIs.
29 +const IdleLanePriority = 2;
30 +
31 function loadModules() {
32 ReactFeatureFlags = require('shared/ReactFeatureFlags');
33
@@ -233,13 +238,11 @@ describe('ReactDOMTracing', () => {
238 } else {
239 Scheduler.unstable_yieldValue('Child:mount');
240 // TODO: Double wrapping is temporary while we remove Scheduler runWithPriority.
236 - ReactDOM.unstable_runWithPriority(
237 - Scheduler.unstable_IdlePriority,
238 - () =>
239 - Scheduler.unstable_runWithPriority(
240 - Scheduler.unstable_IdlePriority,
241 - () => setDidMount(true),
242 - ),
241 + ReactDOM.unstable_runWithPriority(IdleLanePriority, () =>
242 + Scheduler.unstable_runWithPriority(
243 + Scheduler.unstable_IdlePriority,
244 + () => setDidMount(true),
245 + ),
246 );
247 }
248 }, [didMount]);