From 6995565e445276ce0fababec761bcd43b4b1148e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 15 Jul 2020 22:05:13 +0100 Subject: [PATCH] Fix bug with enableLegacyFBSupport click handlers Fix other test Prettier --- .../react-dom/src/__tests__/ReactDOM-test.js | 27 +++------------ .../src/events/DOMModernPluginEventSystem.js | 17 ++++------ ...OMModernPluginEventSystem-test.internal.js | 33 +++++++++++++++++++ 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 0a4b5a36cbf9..6c3de6978046 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -13,7 +13,6 @@ let React; let ReactDOM; let ReactDOMServer; let ReactTestUtils; -const ReactFeatureFlags = require('shared/ReactFeatureFlags'); describe('ReactDOM', () => { beforeEach(() => { @@ -355,27 +354,11 @@ describe('ReactDOM', () => { document.body.appendChild(container); try { ReactDOM.render(, container); - let expected; - - if (ReactFeatureFlags.enableLegacyFBSupport) { - // We expect to duplicate the 2nd handler because this test is - // not really designed around how the legacy FB support system works. - // This is because the above test sync fires a click() event - // during that of another click event, which causes the FB support system - // to duplicate adding an event listener. In practice this would never - // happen, as we only apply the legacy FB logic for "click" events, - // which would never stack this way in product code. - expected = [ - '1st node clicked', - "2nd node clicked imperatively from 1st's handler", - "2nd node clicked imperatively from 1st's handler", - ]; - } else { - expected = [ - '1st node clicked', - "2nd node clicked imperatively from 1st's handler", - ]; - } + + const expected = [ + '1st node clicked', + "2nd node clicked imperatively from 1st's handler", + ]; expect(actual).toEqual(expected); } finally { diff --git a/packages/react-dom/src/events/DOMModernPluginEventSystem.js b/packages/react-dom/src/events/DOMModernPluginEventSystem.js index a36c934abf7d..a1362743bf65 100644 --- a/packages/react-dom/src/events/DOMModernPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMModernPluginEventSystem.js @@ -478,16 +478,13 @@ function addTrappedEventListener( if (enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport) { const originalListener = listener; listener = function(...p) { - try { - return originalListener.apply(this, p); - } finally { - removeEventListener( - targetContainer, - rawEventName, - unsubscribeListener, - isCapturePhaseListener, - ); - } + removeEventListener( + targetContainer, + rawEventName, + unsubscribeListener, + isCapturePhaseListener, + ); + return originalListener.apply(this, p); }; } if (isCapturePhaseListener) { diff --git a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js index db46c30c407b..ad2d85d08b0a 100644 --- a/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js @@ -143,6 +143,39 @@ describe('DOMModernPluginEventSystem', () => { expect(log[5]).toEqual(['bubble', buttonElement]); }); + it('handle propagation of click events combined with sync clicks', () => { + const buttonRef = React.createRef(); + let clicks = 0; + + function Test() { + const inputRef = React.useRef(null); + return ( +
+
+ ); + } + + ReactDOM.render(, container); + + const buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + + expect(clicks).toBe(1); + }); + it('handle propagation of click events between roots', () => { const buttonRef = React.createRef(); const divRef = React.createRef();