@samitouri / QOS-React-2 / commits / 78120032d4

Remove `flushDiscreteUpdates` from end of event (#21223)

We don't need this anymore because we flush in a microtask. This should allow us to remove the logic in the event system that tracks nested event dispatches. I added a test to confirm that nested event dispatches don't triggger a synchronous flush, like they would if we wrapped them `flushSync`. It already passed; I added it to prevent a regression.

Andrew Clark committed Apr 20, 2021 at 10:25 UTC 78120032d4ca7ee8611d630d7c67e7808885dfe9

7 files changed +106 -50

packages/react-dom/src/__tests__/ReactDOMNestedEvents-test.js new

+78

@@ -0,0 +1,78 @@

1 +/**

2 + * Copyright (c) Facebook, Inc. and its affiliates.

3 + *

4 + * This source code is licensed under the MIT license found in the

5 + * LICENSE file in the root directory of this source tree.

6 + *

7 + * @emails react-core

8 + */

9 +

10 +'use strict';

11 +

12 +describe('ReactDOMNestedEvents', () => {

13 + let React;

14 + let ReactDOM;

15 + let Scheduler;

16 + let TestUtils;

17 + let act;

18 + let useState;

19 +

20 + beforeEach(() => {

21 + jest.resetModules();

22 + React = require('react');

23 + ReactDOM = require('react-dom');

24 + Scheduler = require('scheduler');

25 + TestUtils = require('react-dom/test-utils');

26 + act = TestUtils.unstable_concurrentAct;

27 + useState = React.useState;

28 + });

29 +

30 + // @gate experimental

31 + test('nested event dispatches should not cause updates to flush', async () => {

32 + const buttonRef = React.createRef(null);

33 + function App() {

34 + const [isClicked, setIsClicked] = useState(false);

35 + const [isFocused, setIsFocused] = useState(false);

36 + const onClick = () => {

37 + setIsClicked(true);

38 + const el = buttonRef.current;

39 + el.focus();

40 + // The update triggered by the focus event should not have flushed yet.

41 + // Nor the click update. They would have if we had wrapped the focus

42 + // call in `flushSync`, though.

43 + Scheduler.unstable_yieldValue(

44 + 'Value right after focus call: ' + el.innerHTML,

45 + );

46 + };

47 + const onFocus = () => {

48 + setIsFocused(true);

49 + };

50 + return (

51 + <>

52 + <button ref={buttonRef} onFocus={onFocus} onClick={onClick}>

53 + {`Clicked: ${isClicked}, Focused: ${isFocused}`}

54 + </button>

55 + </>

56 + );

57 + }

58 +

59 + const container = document.createElement('div');

60 + document.body.appendChild(container);

61 + const root = ReactDOM.unstable_createRoot(container);

62 +

63 + await act(async () => {

64 + root.render(<App />);

65 + });

66 + expect(buttonRef.current.innerHTML).toEqual(

67 + 'Clicked: false, Focused: false',

68 + );

69 +

70 + await act(async () => {

71 + buttonRef.current.click();

72 + });

73 + expect(Scheduler).toHaveYielded([

74 + 'Value right after focus call: Clicked: false, Focused: false',

75 + ]);

76 + expect(buttonRef.current.innerHTML).toEqual('Clicked: true, Focused: true');

77 + });

78 +});

packages/react-dom/src/events/ReactDOMEventListener.js

+2 -18

@@ -25,21 +25,13 @@ import {

25 getSuspenseInstanceFromFiber,

26 } from 'react-reconciler/src/ReactFiberTreeReflection';

27 import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags';

28 -import {

29 - type EventSystemFlags,

30 - IS_CAPTURE_PHASE,

31 - IS_LEGACY_FB_SUPPORT_MODE,

32 -} from './EventSystemFlags';

28 +import {type EventSystemFlags, IS_CAPTURE_PHASE} from './EventSystemFlags';

30 import getEventTarget from './getEventTarget';

31 import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree';

37 -import {enableLegacyFBSupport} from 'shared/ReactFeatureFlags';

33 import {dispatchEventForPluginEventSystem} from './DOMPluginEventSystem';

39 -import {

40 - flushDiscreteUpdatesIfNeeded,

41 - discreteUpdates,

42 -} from './ReactDOMUpdateBatching';

34 +import {discreteUpdates} from './ReactDOMUpdateBatching';

36 import {

37 getCurrentPriorityLevel as getCurrentSchedulerPriorityLevel,

@@ -120,14 +112,6 @@ function dispatchDiscreteEvent(

112 container,

113 nativeEvent,

114 ) {

123 - if (

124 - !enableLegacyFBSupport ||

125 - // If we are in Legacy FB support mode, it means we've already

126 - // flushed for this event and we don't need to do it again.

127 - (eventSystemFlags & IS_LEGACY_FB_SUPPORT_MODE) === 0

128 - ) {

129 - flushDiscreteUpdatesIfNeeded(nativeEvent.timeStamp);

130 - }

115 discreteUpdates(

116 dispatchEvent,

117 domEventName,

packages/react-dom/src/events/ReactDOMUpdateBatching.js

-7

@@ -88,13 +88,6 @@ export function discreteUpdates(fn, a, b, c, d) {

88 }

89 }

91 -// TODO: Replace with flushSync

92 -export function flushDiscreteUpdatesIfNeeded(timeStamp: number) {

93 - if (!isInsideEventHandler) {

94 - flushDiscreteUpdatesImpl();

95 - }

96 -}

97 -

91 export function setBatchingImplementation(

92 _batchedUpdatesImpl,

93 _discreteUpdatesImpl,

packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js

+26 -10

@@ -656,7 +656,7 @@ describe('DOMPluginEventSystem', () => {

656 document.body.removeChild(parentContainer);

657 });

658

659 - it('handle click events on dynamic portals', () => {

659 + it('handle click events on dynamic portals', async () => {

660 const log = [];

661

662 function Parent() {

@@ -670,7 +670,7 @@ describe('DOMPluginEventSystem', () => {

670 ref.current,

671 ),

672 );

673 - });

673 + }, []);

674

675 return (

676 <div ref={ref} onClick={() => log.push('parent')} id="parent">

@@ -679,17 +679,25 @@ describe('DOMPluginEventSystem', () => {

679 );

680 }

681

682 - ReactDOM.render(<Parent />, container);

682 + await act(async () => {

683 + ReactDOM.render(<Parent />, container);

684 + });

685

686 const parent = container.lastChild;

687 expect(parent.id).toEqual('parent');

686 - dispatchClickEvent(parent);

688 +

689 + await act(async () => {

690 + dispatchClickEvent(parent);

691 + });

692

693 expect(log).toEqual(['parent']);

694

695 const child = parent.lastChild;

696 expect(child.id).toEqual('child');

692 - dispatchClickEvent(child);

697 +

698 + await act(async () => {

699 + dispatchClickEvent(child);

700 + });

701

702 // we add both 'child' and 'parent' due to bubbling

703 expect(log).toEqual(['parent', 'child', 'parent']);

@@ -697,7 +705,7 @@ describe('DOMPluginEventSystem', () => {

705

706 // Slight alteration to the last test, to catch

707 // a subtle difference in traversal.

700 - it('handle click events on dynamic portals #2', () => {

708 + it('handle click events on dynamic portals #2', async () => {

709 const log = [];

710

711 function Parent() {

@@ -711,7 +719,7 @@ describe('DOMPluginEventSystem', () => {

719 ref.current,

720 ),

721 );

714 - });

722 + }, []);

723

724 return (

725 <div ref={ref} onClick={() => log.push('parent')} id="parent">

@@ -720,17 +728,25 @@ describe('DOMPluginEventSystem', () => {

728 );

729 }

730

723 - ReactDOM.render(<Parent />, container);

731 + await act(async () => {

732 + ReactDOM.render(<Parent />, container);

733 + });

734

735 const parent = container.lastChild;

736 expect(parent.id).toEqual('parent');

727 - dispatchClickEvent(parent);

737 +

738 + await act(async () => {

739 + dispatchClickEvent(parent);

740 + });

741

742 expect(log).toEqual(['parent']);

743

744 const child = parent.lastChild;

745 expect(child.id).toEqual('child');

733 - dispatchClickEvent(child);

746 +

747 + await act(async () => {

748 + dispatchClickEvent(child);

749 + });

750

751 // we add both 'child' and 'parent' due to bubbling

752 expect(log).toEqual(['parent', 'child', 'parent']);

packages/react-native-renderer/src/ReactFabric.js

-2

@@ -19,7 +19,6 @@ import {

19 batchedEventUpdates,

20 batchedUpdates as batchedUpdatesImpl,

21 discreteUpdates,

22 - flushDiscreteUpdates,

22 createContainer,

23 updateContainer,

24 injectIntoDevTools,

@@ -242,7 +241,6 @@ function createPortal(

241 setBatchingImplementation(

242 batchedUpdatesImpl,

243 discreteUpdates,

245 - flushDiscreteUpdates,

244 batchedEventUpdates,

245 );

246

packages/react-native-renderer/src/ReactNativeRenderer.js

-2

@@ -19,7 +19,6 @@ import {

19 batchedUpdates as batchedUpdatesImpl,

20 batchedEventUpdates,

21 discreteUpdates,

22 - flushDiscreteUpdates,

22 createContainer,

23 updateContainer,

24 injectIntoDevTools,

@@ -241,7 +240,6 @@ function createPortal(

240 setBatchingImplementation(

241 batchedUpdatesImpl,

242 discreteUpdates,

244 - flushDiscreteUpdates,

243 batchedEventUpdates,

244 );

245

packages/react-native-renderer/src/legacy-events/ReactGenericBatching.js

-11

@@ -18,7 +18,6 @@ let batchedUpdatesImpl = function(fn, bookkeeping) {

18 let discreteUpdatesImpl = function(fn, a, b, c, d) {

19 return fn(a, b, c, d);

20 };

21 -let flushDiscreteUpdatesImpl = function() {};

21 let batchedEventUpdatesImpl = batchedUpdatesImpl;

23 let isInsideEventHandler = false;

@@ -59,25 +58,15 @@ export function discreteUpdates(fn, a, b, c, d) {

58 return discreteUpdatesImpl(fn, a, b, c, d);

59 } finally {

60 isInsideEventHandler = prevIsInsideEventHandler;

62 - if (!isInsideEventHandler) {

63 - }

64 - }

65 -}

66 -

67 -export function flushDiscreteUpdatesIfNeeded() {

68 - if (!isInsideEventHandler) {

69 - flushDiscreteUpdatesImpl();

61 }

62 }

64 export function setBatchingImplementation(

65 _batchedUpdatesImpl,

66 _discreteUpdatesImpl,

76 - _flushDiscreteUpdatesImpl,

67 _batchedEventUpdatesImpl,

68 ) {

69 batchedUpdatesImpl = _batchedUpdatesImpl;

70 discreteUpdatesImpl = _discreteUpdatesImpl;

81 - flushDiscreteUpdatesImpl = _flushDiscreteUpdatesImpl;

71 batchedEventUpdatesImpl = _batchedEventUpdatesImpl;

72 }