@samitouri / QOS-React-2 / commits / acde654698

Unify InputDiscreteLane with SyncLane (#20968)

* Unify sync priority and input discrete * Fix lint * Use update lane instead * Update sync lane labels

Ricky committed Mar 10, 2021 at 17:36 UTC acde654698659326af0ac4eded4a3ae7cb780a55
9 files changed +31 -85
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
+11 -30
@@ -386,63 +386,44 @@ describe('ReactDOMFiberAsync', () => {
386 });
387
388 // @gate experimental
389 - it('ignores discrete events on a pending removed element', () => {
389 + it('ignores discrete events on a pending removed element', async () => {
390 const disableButtonRef = React.createRef();
391 const submitButtonRef = React.createRef();
392
393 - let formSubmitted = false;
394 -
393 function Form() {
394 const [active, setActive] = React.useState(true);
395 function disableForm() {
396 setActive(false);
397 }
400 - function submitForm() {
401 - formSubmitted = true; // This should not get invoked
402 - }
398 +
399 return (
400 <div>
401 <button onClick={disableForm} ref={disableButtonRef}>
402 Disable
403 </button>
408 - {active ? (
409 - <button onClick={submitForm} ref={submitButtonRef}>
410 - Submit
411 - </button>
412 - ) : null}
404 + {active ? <button ref={submitButtonRef}>Submit</button> : null}
405 </div>
406 );
407 }
408
409 const root = ReactDOM.unstable_createRoot(container);
418 - root.render(<Form />);
419 - // Flush
420 - Scheduler.unstable_flushAll();
410 + await act(async () => {
411 + root.render(<Form />);
412 + });
413
414 const disableButton = disableButtonRef.current;
415 expect(disableButton.tagName).toBe('BUTTON');
416
417 + const submitButton = submitButtonRef.current;
418 + expect(submitButton.tagName).toBe('BUTTON');
419 +
420 // Dispatch a click event on the Disable-button.
421 const firstEvent = document.createEvent('Event');
422 firstEvent.initEvent('click', true, true);
423 disableButton.dispatchEvent(firstEvent);
424
430 - // There should now be a pending update to disable the form.
431 -
432 - // This should not have flushed yet since it's in concurrent mode.
433 - const submitButton = submitButtonRef.current;
434 - expect(submitButton.tagName).toBe('BUTTON');
435 -
436 - // In the meantime, we can dispatch a new client event on the submit button.
437 - const secondEvent = document.createEvent('Event');
438 - secondEvent.initEvent('click', true, true);
439 - // This should force the pending update to flush which disables the submit button before the event is invoked.
440 - submitButton.dispatchEvent(secondEvent);
441 -
442 - // Therefore the form should never have been submitted.
443 - expect(formSubmitted).toBe(false);
444 -
445 - expect(submitButtonRef.current).toBe(null);
425 + // The click event is flushed synchronously, even in concurrent mode.
426 + expect(submitButton.current).toBe(undefined);
427 });
428
429 // @gate experimental
packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js
+3 -8
@@ -67,9 +67,9 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
67 }
68
69 const root = ReactDOM.unstable_createRoot(container);
70 - root.render(<Form />);
71 - // Flush
72 - Scheduler.unstable_flushAll();
70 + await act(() => {
71 + root.render(<Form />);
72 + });
73
74 const disableButton = disableButtonRef.current;
75 expect(disableButton.tagName).toBe('BUTTON');
@@ -81,11 +81,6 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
81 dispatchAndSetCurrentEvent(disableButton, firstEvent),
82 ).toErrorDev(['An update to Form inside a test was not wrapped in act']);
83
84 - // There should now be a pending update to disable the form.
85 - // This should not have flushed yet since it's in concurrent mode.
86 - const submitButton = submitButtonRef.current;
87 - expect(submitButton.tagName).toBe('BUTTON');
88 -
84 // Discrete events should be flushed in a microtask.
85 // Verify that the second button was removed.
86 await null;
packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js
+1 -4
@@ -685,7 +685,7 @@ describe('ChangeEventPlugin', () => {
685 });
686
687 // @gate experimental
688 - it('is async for non-input events', async () => {
688 + it('is sync for non-input events', async () => {
689 const root = ReactDOM.unstable_createRoot(container);
690 let input;
691
@@ -724,9 +724,6 @@ describe('ChangeEventPlugin', () => {
724 input.dispatchEvent(
725 new Event('click', {bubbles: true, cancelable: true}),
726 );
727 - // Nothing should have changed
728 - expect(Scheduler).toHaveYielded([]);
729 - expect(input.value).toBe('initial');
727
728 // Flush microtask queue.
729 await null;
packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js
+13 -24
@@ -240,7 +240,7 @@ describe('SimpleEventPlugin', function() {
240 });
241
242 // @gate experimental
243 - it('flushes pending interactive work before extracting event handler', () => {
243 + it('flushes pending interactive work before exiting event handler', () => {
244 container = document.createElement('div');
245 const root = ReactDOM.unstable_createRoot(container);
246 document.body.appendChild(container);
@@ -292,17 +292,14 @@ describe('SimpleEventPlugin', function() {
292 expect(Scheduler).toHaveYielded([
293 // The handler fired
294 'Side-effect',
295 - // but the component did not re-render yet, because it's async
295 + // The component re-rendered synchronously, even in concurrent mode.
296 + 'render button: disabled',
297 ]);
298
299 // Click the button again
300 click();
301 expect(Scheduler).toHaveYielded([
301 - // Before handling this second click event, the previous interactive
302 - // update is flushed
303 - 'render button: disabled',
304 - // The event handler was removed from the button, so there's no second
305 - // side-effect
302 + // The event handler was removed from the button, so there's no effect.
303 ]);
304
305 // The handler should not fire again no matter how many times we
@@ -359,8 +356,8 @@ describe('SimpleEventPlugin', function() {
356
357 // Click the button a single time
358 click();
362 - // The counter should not have updated yet because it's async
363 - expect(button.textContent).toEqual('Count: 0');
359 + // The counter should update synchronously, even in concurrent mode.
360 + expect(button.textContent).toEqual('Count: 1');
361
362 // Click the button many more times
363 await TestUtils.act(async () => {
@@ -442,15 +439,10 @@ describe('SimpleEventPlugin', function() {
439 button.dispatchEvent(event);
440 }
441
445 - // Click the button a single time
446 - click();
447 - // Nothing should flush on the first click.
448 - expect(Scheduler).toHaveYielded([]);
449 - // Click again. This will force the previous discrete update to flush. But
450 - // only the high-pri count will increase.
442 + // Click the button a single time.
443 + // This will flush at the end of the event, even in concurrent mode.
444 click();
445 expect(Scheduler).toHaveYielded(['High-pri count: 1, Low-pri count: 0']);
453 - expect(button.textContent).toEqual('High-pri count: 1, Low-pri count: 0');
446
447 // Click the button many more times
448 click();
@@ -460,7 +452,7 @@ describe('SimpleEventPlugin', function() {
452 click();
453 click();
454
463 - // Flush the remaining work.
455 + // Each update should synchronously flush, even in concurrent mode.
456 expect(Scheduler).toHaveYielded([
457 'High-pri count: 2, Low-pri count: 0',
458 'High-pri count: 3, Low-pri count: 0',
@@ -470,16 +462,13 @@ describe('SimpleEventPlugin', function() {
462 'High-pri count: 7, Low-pri count: 0',
463 ]);
464
473 - // Flush the microtask queue
474 - await null;
475 -
476 - // At the end, both counters should equal the total number of clicks
477 - expect(Scheduler).toHaveYielded(['High-pri count: 8, Low-pri count: 0']);
465 + // Now flush the scheduler to apply the transition updates.
466 + // At the end, both counters should equal the total number of clicks.
467 expect(Scheduler).toFlushAndYield([
479 - 'High-pri count: 8, Low-pri count: 8',
468 + 'High-pri count: 7, Low-pri count: 7',
469 ]);
470
482 - expect(button.textContent).toEqual('High-pri count: 8, Low-pri count: 8');
471 + expect(button.textContent).toEqual('High-pri count: 7, Low-pri count: 7');
472 });
473 });
474
packages/react-reconciler/src/ReactFiberLane.new.js
+1 -1
@@ -566,7 +566,7 @@ export function findUpdateLane(lanePriority: LanePriority): Lane {
566 case SyncBatchedLanePriority:
567 return SyncBatchedLane;
568 case InputDiscreteLanePriority:
569 - return InputDiscreteLane;
569 + return SyncLane;
570 case InputContinuousLanePriority:
571 return InputContinuousLane;
572 case DefaultLanePriority:
packages/react-reconciler/src/ReactFiberLane.old.js
+1 -1
@@ -566,7 +566,7 @@ export function findUpdateLane(lanePriority: LanePriority): Lane {
566 case SyncBatchedLanePriority:
567 return SyncBatchedLane;
568 case InputDiscreteLanePriority:
569 - return InputDiscreteLane;
569 + return SyncLane;
570 case InputContinuousLanePriority:
571 return InputContinuousLane;
572 case DefaultLanePriority:
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
-7
@@ -90,7 +90,6 @@ import {
90 clearContainer,
91 getCurrentEventPriority,
92 supportsMicrotasks,
93 - scheduleMicrotask,
93 } from './ReactFiberHostConfig';
94
95 import {
@@ -737,12 +736,6 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
736 ImmediateSchedulerPriority,
737 performSyncWorkOnRoot.bind(null, root),
738 );
740 - } else if (
741 - supportsMicrotasks &&
742 - newCallbackPriority === InputDiscreteLanePriority
743 - ) {
744 - scheduleMicrotask(performSyncWorkOnRoot.bind(null, root));
745 - newCallbackNode = null;
739 } else {
740 const schedulerPriorityLevel = lanePriorityToSchedulerPriority(
741 newCallbackPriority,
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
-7
@@ -90,7 +90,6 @@ import {
90 clearContainer,
91 getCurrentEventPriority,
92 supportsMicrotasks,
93 - scheduleMicrotask,
93 } from './ReactFiberHostConfig';
94
95 import {
@@ -737,12 +736,6 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
736 ImmediateSchedulerPriority,
737 performSyncWorkOnRoot.bind(null, root),
738 );
740 - } else if (
741 - supportsMicrotasks &&
742 - newCallbackPriority === InputDiscreteLanePriority
743 - ) {
744 - scheduleMicrotask(performSyncWorkOnRoot.bind(null, root));
745 - newCallbackNode = null;
739 } else {
740 const schedulerPriorityLevel = lanePriorityToSchedulerPriority(
741 newCallbackPriority,
packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js
+1 -3
@@ -140,9 +140,7 @@ describe('SchedulingProfiler labels', () => {
140 targetRef.current.click();
141 });
142 expect(clearedMarks).toContain(
143 - `--schedule-state-update-${formatLanes(
144 - ReactFiberLane.InputDiscreteLane,
145 - )}-App`,
143 + `--schedule-state-update-${formatLanes(ReactFiberLane.SyncLane)}-App`,
144 );
145 });
146