diff --git a/packages/react-dom/src/client/inputValueTracking.js b/packages/react-dom/src/client/inputValueTracking.js index 4ccc80ac85b1a..b2addf7f5074f 100644 --- a/packages/react-dom/src/client/inputValueTracking.js +++ b/packages/react-dom/src/client/inputValueTracking.js @@ -123,7 +123,6 @@ export function updateValueIfChanged(node: ElementWithValueTracker) { if (!tracker) { return true; } - const lastValue = tracker.getValue(); const nextValue = getValueFromNode(node); if (nextValue !== lastValue) { diff --git a/packages/react-dom/src/events/ChangeEventPlugin.js b/packages/react-dom/src/events/ChangeEventPlugin.js index e5f65ceff282f..26177cd7f4f39 100644 --- a/packages/react-dom/src/events/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/ChangeEventPlugin.js @@ -207,10 +207,7 @@ function getTargetInstForInputEventPolyfill(topLevelType, targetInst) { /** * SECTION: handle `click` event */ -function shouldUseClickEvent(elem) { - // Use the `click` event to detect changes to checkbox and radio inputs. - // This approach works across all browsers, whereas `change` does not fire - // until `blur` in IE8. +function isCheckableInput(elem) { const nodeName = elem.nodeName; return ( nodeName && @@ -219,12 +216,6 @@ function shouldUseClickEvent(elem) { ); } -function getTargetInstForClickEvent(topLevelType, targetInst) { - if (topLevelType === TOP_CLICK) { - return getInstIfValueChanged(targetInst); - } -} - function getTargetInstForInputOrChangeEvent(topLevelType, targetInst) { if (topLevelType === TOP_INPUT || topLevelType === TOP_CHANGE) { return getInstIfValueChanged(targetInst); @@ -275,12 +266,13 @@ const ChangeEventPlugin = { getTargetInstFunc = getTargetInstForInputEventPolyfill; handleEventFunc = handleEventsForInputEventPolyfill; } - } else if (shouldUseClickEvent(targetNode)) { - getTargetInstFunc = getTargetInstForClickEvent; + } else if (isCheckableInput(targetNode)) { + getTargetInstFunc = getTargetInstForInputOrChangeEvent; } if (getTargetInstFunc) { const inst = getTargetInstFunc(topLevelType, targetInst); + if (inst) { const event = createAndAccumulateChangeEvent( inst, diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js index 23f494172c9a7..52068e4fb7e16 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js @@ -104,13 +104,7 @@ describe('ChangeEventPlugin', () => { container, ); - // Secretly, set `checked` to false, so that dispatching the `click` will - // make it `true` again. Thus, at the time of the event, React should not - // consider it a change from the initial `true` value. - setUntrackedChecked.call(node, false); - node.dispatchEvent( - new MouseEvent('click', {bubbles: true, cancelable: true}), - ); + node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); // There should be no React change events because the value stayed the same. expect(called).toBe(0); }); @@ -128,13 +122,7 @@ describe('ChangeEventPlugin', () => { container, ); - // Secretly, set `checked` to true, so that dispatching the `click` will - // make it `false` again. Thus, at the time of the event, React should not - // consider it a change from the initial `false` value. - setUntrackedChecked.call(node, true); - node.dispatchEvent( - new MouseEvent('click', {bubbles: true, cancelable: true}), - ); + node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); // There should be no React change events because the value stayed the same. expect(called).toBe(0); }); @@ -153,18 +141,15 @@ describe('ChangeEventPlugin', () => { ); expect(node.checked).toBe(false); - node.dispatchEvent( - new MouseEvent('click', {bubbles: true, cancelable: true}), - ); - // Note: unlike with text input events, dispatching `click` actually - // toggles the checkbox and updates its `checked` value. + setUntrackedChecked.call(node, true); + node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); + expect(node.checked).toBe(true); expect(called).toBe(1); expect(node.checked).toBe(true); - node.dispatchEvent( - new MouseEvent('click', {bubbles: true, cancelable: true}), - ); + setUntrackedChecked.call(node, false); + node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); expect(node.checked).toBe(false); expect(called).toBe(2); }); @@ -243,22 +228,29 @@ describe('ChangeEventPlugin', () => { container, ); - // Set the value, updating the "current" value that React tracks to true. + // Set it programmatically. input.checked = true; - // Under the hood, uncheck the box so that the click will "check" it again. - setUntrackedChecked.call(input, false); - input.dispatchEvent( - new MouseEvent('click', {bubbles: true, cancelable: true}), - ); + + // Even if a DOM event fires, React sees that the real input value now + // ('bar') is the same as the "current" one we already recorded. + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); expect(input.checked).toBe(true); - // We don't expect a React event because at the time of the click, the real - // checked value (true) was the same as the last recorded "current" value - // (also true). + // In this case we don't expect to get a React event. expect(called).toBe(0); - // However, simulating a normal click should fire a React event because the - // real value (false) would have changed from the last tracked value (true). - input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + // However, we can simulate user toggling the checkbox by calling + // the underlying setter. + setUntrackedChecked.call(input, false); + + // Now, when the event fires, the real input value (false) differs from the + // "current" one we previously recorded (false). + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + expect(input.checked).toBe(false); + // In this case React should fire an event for it. + expect(called).toBe(1); + + // Verify again that extra events without real changes are ignored. + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); expect(called).toBe(1); }); @@ -282,8 +274,8 @@ describe('ChangeEventPlugin', () => { ); setUntrackedChecked.call(input, true); - input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); - input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); expect(called).toBe(1); }); @@ -312,23 +304,26 @@ describe('ChangeEventPlugin', () => { const option2 = div.childNodes[1]; // Select first option. + setUntrackedChecked.call(option1, true); option1.dispatchEvent( - new Event('click', {bubbles: true, cancelable: true}), + new Event('change', {bubbles: true, cancelable: true}), ); expect(called1).toBe(1); expect(called2).toBe(0); // Select second option. + setUntrackedChecked.call(option2, true); option2.dispatchEvent( - new Event('click', {bubbles: true, cancelable: true}), + new Event('change', {bubbles: true, cancelable: true}), ); expect(called1).toBe(1); expect(called2).toBe(1); // Select the first option. // It should receive the React change event again. + setUntrackedChecked.call(option1, true); option1.dispatchEvent( - new Event('click', {bubbles: true, cancelable: true}), + new Event('change', {bubbles: true, cancelable: true}), ); expect(called1).toBe(2); expect(called2).toBe(1);