Fix bug with enableLegacyFBSupport click handlers (#19378)
Dominic Gannaway committed
Jul 16, 2020 at 14:56 UTC
e387c98ffabdf3808d34910b4493093dd975ec69
3 files changed
+45
-32
packages/react-dom/src/__tests__/ReactDOM-test.js
+5
-22
@@ -13,7 +13,6 @@ let React;
13
let ReactDOM;
14
let ReactDOMServer;
15
let ReactTestUtils;
16
-const ReactFeatureFlags = require('shared/ReactFeatureFlags');
16
17
describe('ReactDOM', () => {
18
beforeEach(() => {
@@ -355,27 +354,11 @@ describe('ReactDOM', () => {
354
document.body.appendChild(container);
355
try {
356
ReactDOM.render(<Wrapper />, container);
358
- let expected;
359
-
360
- if (ReactFeatureFlags.enableLegacyFBSupport) {
361
- // We expect to duplicate the 2nd handler because this test is
362
- // not really designed around how the legacy FB support system works.
363
- // This is because the above test sync fires a click() event
364
- // during that of another click event, which causes the FB support system
365
- // to duplicate adding an event listener. In practice this would never
366
- // happen, as we only apply the legacy FB logic for "click" events,
367
- // which would never stack this way in product code.
368
- expected = [
369
- '1st node clicked',
370
- "2nd node clicked imperatively from 1st's handler",
371
- "2nd node clicked imperatively from 1st's handler",
372
- ];
373
- } else {
374
- expected = [
375
- '1st node clicked',
376
- "2nd node clicked imperatively from 1st's handler",
377
- ];
378
- }
357
+
358
+ const expected = [
359
+ '1st node clicked',
360
+ "2nd node clicked imperatively from 1st's handler",
361
+ ];
362
363
expect(actual).toEqual(expected);
364
} finally {
packages/react-dom/src/events/DOMModernPluginEventSystem.js
+7
-10
@@ -478,16 +478,13 @@ function addTrappedEventListener(
478
if (enableLegacyFBSupport && isDeferredListenerForLegacyFBSupport) {
479
const originalListener = listener;
480
listener = function(...p) {
481
- try {
482
- return originalListener.apply(this, p);
483
- } finally {
484
- removeEventListener(
485
- targetContainer,
486
- rawEventName,
487
- unsubscribeListener,
488
- isCapturePhaseListener,
489
- );
490
- }
481
+ removeEventListener(
482
+ targetContainer,
483
+ rawEventName,
484
+ unsubscribeListener,
485
+ isCapturePhaseListener,
486
+ );
487
+ return originalListener.apply(this, p);
488
};
489
}
490
if (isCapturePhaseListener) {
packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js
+33
@@ -143,6 +143,39 @@ describe('DOMModernPluginEventSystem', () => {
143
expect(log[5]).toEqual(['bubble', buttonElement]);
144
});
145
146
+ it('handle propagation of click events combined with sync clicks', () => {
147
+ const buttonRef = React.createRef();
148
+ let clicks = 0;
149
+
150
+ function Test() {
151
+ const inputRef = React.useRef(null);
152
+ return (
153
+ <div>
154
+ <button
155
+ ref={buttonRef}
156
+ onClick={() => {
157
+ // Sync click
158
+ inputRef.current.click();
159
+ }}
160
+ />
161
+ <input
162
+ ref={inputRef}
163
+ onClick={() => {
164
+ clicks++;
165
+ }}
166
+ />
167
+ </div>
168
+ );
169
+ }
170
+
171
+ ReactDOM.render(<Test />, container);
172
+
173
+ const buttonElement = buttonRef.current;
174
+ dispatchClickEvent(buttonElement);
175
+
176
+ expect(clicks).toBe(1);
177
+ });
178
+
179
it('handle propagation of click events between roots', () => {
180
const buttonRef = React.createRef();
181
const divRef = React.createRef();