From 29253ec9265000fb46600abb2002b5d598139961 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Mon, 23 Oct 2017 00:33:00 -0400 Subject: [PATCH 01/10] Use only public API for ChangeEventPlugin-test.js --- .../__tests__/ChangeEventPlugin-test.js | 60 +++++++++---------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js index 15adf32e99862..106874b0ee698 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js @@ -12,31 +12,25 @@ var React = require('react'); var ReactDOM = require('react-dom'); var ReactTestUtils = require('react-dom/test-utils'); -// TODO: can we express this test with only public API? -var ChangeEventPlugin = require('ChangeEventPlugin'); -var inputValueTracking = require('inputValueTracking'); +var TestRenderer = require('react-test-renderer'); function getTrackedValue(elem) { - var tracker = inputValueTracking._getTrackerFromNode(elem); - return tracker.getValue(); + return elem.value; } function setTrackedValue(elem, value) { - var tracker = inputValueTracking._getTrackerFromNode(elem); - tracker.setValue(value); + elem.value = value; } -function setUntrackedValue(elem, value) { - var tracker = inputValueTracking._getTrackerFromNode(elem); - var current = tracker.getValue(); +var setUntrackedChecked = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'checked' +).set; - if (elem.type === 'checkbox' || elem.type === 'radio') { - elem.checked = value; - } else { - elem.value = value; - } - tracker.setValue(current); -} +var setUntrackedValue = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'value' +).set; describe('ChangeEventPlugin', () => { it('should fire change for checkbox input', () => { @@ -48,10 +42,10 @@ describe('ChangeEventPlugin', () => { } var input = ReactTestUtils.renderIntoDocument( - , + ); - setUntrackedValue(input, true); + setUntrackedChecked.call(input, true); ReactTestUtils.SimulateNative.click(input); expect(called).toBe(1); @@ -82,7 +76,7 @@ describe('ChangeEventPlugin', () => { ReactTestUtils.SimulateNative.change(input); expect(called).toBe(0); - setUntrackedValue(input, 'foo'); + setUntrackedValue.call(input, 'foo'); ReactTestUtils.SimulateNative.change(input); expect(called).toBe(1); @@ -104,8 +98,7 @@ describe('ChangeEventPlugin', () => { ReactTestUtils.SimulateNative.click(input); expect(called).toBe(0); - input.checked = false; - setTrackedValue(input, undefined); + setUntrackedChecked.call(input, false); ReactTestUtils.SimulateNative.click(input); expect(called).toBe(1); @@ -128,7 +121,7 @@ describe('ChangeEventPlugin', () => { var input = ReactTestUtils.renderIntoDocument( , ); - setUntrackedValue(input, true); + setUntrackedChecked.call(input, true); ReactTestUtils.SimulateNative.click(input); ReactTestUtils.SimulateNative.click(input); expect(called).toBe(1); @@ -151,21 +144,21 @@ describe('ChangeEventPlugin', () => { called = 0; input = ReactTestUtils.renderIntoDocument(element); - setUntrackedValue(input, '40'); + setUntrackedValue.call(input, '40'); ReactTestUtils.SimulateNative.change(input); ReactTestUtils.SimulateNative.change(input); expect(called).toBe(1); called = 0; input = ReactTestUtils.renderIntoDocument(element); - setUntrackedValue(input, '40'); + setUntrackedValue.call(input, '40'); ReactTestUtils.SimulateNative.input(input); ReactTestUtils.SimulateNative.input(input); expect(called).toBe(1); called = 0; input = ReactTestUtils.renderIntoDocument(element); - setUntrackedValue(input, '40'); + setUntrackedValue.call(input, '40'); ReactTestUtils.SimulateNative.input(input); ReactTestUtils.SimulateNative.change(input); expect(called).toBe(1); @@ -180,18 +173,19 @@ describe('ChangeEventPlugin', () => { expect(e.type).toBe('change'); } - if (!ChangeEventPlugin._isInputEventSupported) { - return; - } + // Do not understand this method. Please advise + // if (!ChangeEventPlugin._isInputEventSupported) { + // return; + // } var input = ReactTestUtils.renderIntoDocument( , ); - setUntrackedValue(input, 'bar'); + setUntrackedValue.call(input, 'bar'); ReactTestUtils.SimulateNative.input(input); - setUntrackedValue(input, 'foo'); + setUntrackedValue.call(input, 'foo'); ReactTestUtils.SimulateNative.change(input); @@ -209,11 +203,11 @@ describe('ChangeEventPlugin', () => { var input = ReactTestUtils.renderIntoDocument( , ); - setUntrackedValue(input, '40'); + setUntrackedValue.call(input, '40'); ReactTestUtils.SimulateNative.input(input); ReactTestUtils.SimulateNative.change(input); - setUntrackedValue(input, 'foo'); + setUntrackedValue.call(input, 'foo'); ReactTestUtils.SimulateNative.input(input); ReactTestUtils.SimulateNative.change(input); From df01d2b5543fd2576d67242050bab6e6c3a482f7 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Mon, 23 Oct 2017 00:37:06 -0400 Subject: [PATCH 02/10] precommit commands complete --- .../src/events/__tests__/ChangeEventPlugin-test.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js index 106874b0ee698..9031e176b41a8 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js @@ -12,24 +12,19 @@ var React = require('react'); var ReactDOM = require('react-dom'); var ReactTestUtils = require('react-dom/test-utils'); -var TestRenderer = require('react-test-renderer'); function getTrackedValue(elem) { return elem.value; } -function setTrackedValue(elem, value) { - elem.value = value; -} - var setUntrackedChecked = Object.getOwnPropertyDescriptor( HTMLInputElement.prototype, - 'checked' + 'checked', ).set; var setUntrackedValue = Object.getOwnPropertyDescriptor( HTMLInputElement.prototype, - 'value' + 'value', ).set; describe('ChangeEventPlugin', () => { @@ -42,7 +37,7 @@ describe('ChangeEventPlugin', () => { } var input = ReactTestUtils.renderIntoDocument( - + , ); setUntrackedChecked.call(input, true); From c7777f99564a75701628213798c2598b6525cf3e Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Wed, 25 Oct 2017 01:59:31 -0400 Subject: [PATCH 03/10] Removed comments --- .../react-dom/src/events/__tests__/ChangeEventPlugin-test.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js index 9031e176b41a8..783af04e89f5f 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js @@ -168,11 +168,6 @@ describe('ChangeEventPlugin', () => { expect(e.type).toBe('change'); } - // Do not understand this method. Please advise - // if (!ChangeEventPlugin._isInputEventSupported) { - // return; - // } - var input = ReactTestUtils.renderIntoDocument( , ); From bff4f3be2d5b64d8e5f1f46ec741773f19e6a5e4 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Sun, 12 Nov 2017 13:33:29 -0800 Subject: [PATCH 04/10] Improving event dispatchers --- .../__tests__/ChangeEventPlugin-test.js | 89 ++++++++++++------- 1 file changed, 59 insertions(+), 30 deletions(-) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js index 783af04e89f5f..353145127682c 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js @@ -17,6 +17,8 @@ function getTrackedValue(elem) { return elem.value; } + + var setUntrackedChecked = Object.getOwnPropertyDescriptor( HTMLInputElement.prototype, 'checked', @@ -27,31 +29,55 @@ var setUntrackedValue = Object.getOwnPropertyDescriptor( 'value', ).set; +function getEvent(type, opts) { + var event = document.createEvent('Event'); + event.initEvent(type, true, true); + return event; +} + +function dispatchEventOnNode(node, type) { + //console.log(`Dispatch ${type}`, node.checked); + var res = node.dispatchEvent( + new Event( type, {bubbles: true, cancelable: true} )); +} + describe('ChangeEventPlugin', () => { - it('should fire change for checkbox input', () => { + it('should fire click for checkbox input', () => { var called = 0; function cb(e) { called = 1; - expect(e.type).toBe('change'); + expect(e.type).toBe('click'); } - var input = ReactTestUtils.renderIntoDocument( - , + var container = document.createElement('div'); + + var stub = ReactDOM.render( + , + container ); - setUntrackedChecked.call(input, true); - ReactTestUtils.SimulateNative.click(input); + document.body.appendChild(container); - expect(called).toBe(1); + var node = ReactDOM.findDOMNode(stub); + + setUntrackedChecked.call(node, false); // track and set to false + dispatchEventOnNode(node, 'click'); // make it true + + expect(called).toBe(1); // onClick has been called + expect(node.checked).toBe(true); // tracked property is now true + + document.body.removeChild(container); }); it('should catch setting the value programmatically', () => { - var input = ReactTestUtils.renderIntoDocument( - , + var container = document.createElement('div'); + var ref = ReactDOM.render( + , container ); + var input = ReactDOM.findDOMNode(ref); + setUntrackedValue.call(input, 'bar'); - input.value = 'bar'; expect(getTrackedValue(input)).toBe('bar'); }); @@ -63,17 +89,20 @@ describe('ChangeEventPlugin', () => { expect(e.type).toBe('change'); } - var input = ReactTestUtils.renderIntoDocument( - , + var container = document.createElement('div'); + var ref = ReactDOM.render( + , container ); + var input = ReactDOM.findDOMNode(ref); input.value = 'bar'; - ReactTestUtils.SimulateNative.change(input); + dispatchEventOnNode(input, 'change'); expect(called).toBe(0); setUntrackedValue.call(input, 'foo'); - ReactTestUtils.SimulateNative.change(input); + expect(input.value).toBe('foo'); + dispatchEventOnNode(input, 'change'); expect(called).toBe(1); }); @@ -90,11 +119,11 @@ describe('ChangeEventPlugin', () => { ); input.checked = true; - ReactTestUtils.SimulateNative.click(input); + input.dispatchEvent(getEvent('click')); expect(called).toBe(0); setUntrackedChecked.call(input, false); - ReactTestUtils.SimulateNative.click(input); + input.dispatchEvent(getEvent('click')); expect(called).toBe(1); }); @@ -117,8 +146,8 @@ describe('ChangeEventPlugin', () => { , ); setUntrackedChecked.call(input, true); - ReactTestUtils.SimulateNative.click(input); - ReactTestUtils.SimulateNative.click(input); + input.dispatchEvent(getEvent('click')); + input.dispatchEvent(getEvent('click')); expect(called).toBe(1); }); @@ -140,22 +169,22 @@ describe('ChangeEventPlugin', () => { input = ReactTestUtils.renderIntoDocument(element); setUntrackedValue.call(input, '40'); - ReactTestUtils.SimulateNative.change(input); - ReactTestUtils.SimulateNative.change(input); + input.dispatchEvent(getEvent('change')); + input.dispatchEvent(getEvent('change')); expect(called).toBe(1); called = 0; input = ReactTestUtils.renderIntoDocument(element); setUntrackedValue.call(input, '40'); - ReactTestUtils.SimulateNative.input(input); - ReactTestUtils.SimulateNative.input(input); + input.dispatchEvent(getEvent('input')); + input.dispatchEvent(getEvent('input')); expect(called).toBe(1); called = 0; input = ReactTestUtils.renderIntoDocument(element); setUntrackedValue.call(input, '40'); - ReactTestUtils.SimulateNative.input(input); - ReactTestUtils.SimulateNative.change(input); + input.dispatchEvent(getEvent('input')); + input.dispatchEvent(getEvent('change')); expect(called).toBe(1); }); }); @@ -173,11 +202,11 @@ describe('ChangeEventPlugin', () => { ); setUntrackedValue.call(input, 'bar'); - ReactTestUtils.SimulateNative.input(input); + input.dispatchEvent(getEvent('input')); setUntrackedValue.call(input, 'foo'); - ReactTestUtils.SimulateNative.change(input); + input.dispatchEvent(getEvent('change')); expect(called).toBe(2); }); @@ -194,13 +223,13 @@ describe('ChangeEventPlugin', () => { , ); setUntrackedValue.call(input, '40'); - ReactTestUtils.SimulateNative.input(input); - ReactTestUtils.SimulateNative.change(input); + input.dispatchEvent(getEvent('input')); + input.dispatchEvent(getEvent('change')); setUntrackedValue.call(input, 'foo'); - ReactTestUtils.SimulateNative.input(input); - ReactTestUtils.SimulateNative.change(input); + input.dispatchEvent(getEvent('input')); + input.dispatchEvent(getEvent('change')); expect(called).toBe(2); }); }); From 054c9557d88aa58ea6d3f4158c6f602a9cbccd78 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Fri, 24 Nov 2017 01:43:02 -0500 Subject: [PATCH 05/10] Updated tests --- .../ChangeEventPlugin-test.internal.js | 212 +++++++++--------- 1 file changed, 103 insertions(+), 109 deletions(-) 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 353145127682c..e009f71a479d1 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js @@ -11,14 +11,11 @@ var React = require('react'); var ReactDOM = require('react-dom'); -var ReactTestUtils = require('react-dom/test-utils'); function getTrackedValue(elem) { return elem.value; } - - var setUntrackedChecked = Object.getOwnPropertyDescriptor( HTMLInputElement.prototype, 'checked', @@ -29,207 +26,204 @@ var setUntrackedValue = Object.getOwnPropertyDescriptor( 'value', ).set; -function getEvent(type, opts) { - var event = document.createEvent('Event'); - event.initEvent(type, true, true); - return event; -} +describe('ChangeEventPlugin', () => { + var container; -function dispatchEventOnNode(node, type) { - //console.log(`Dispatch ${type}`, node.checked); - var res = node.dispatchEvent( - new Event( type, {bubbles: true, cancelable: true} )); -} + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + }); -describe('ChangeEventPlugin', () => { - it('should fire click for checkbox input', () => { + afterEach(() => { + document.body.removeChild(container); + container = null; + }); + + fit('should fire change for checkbox input', () => { var called = 0; function cb(e) { - called = 1; - expect(e.type).toBe('click'); + called++; + expect(e.type).toBe('change'); } - var container = document.createElement('div'); - - var stub = ReactDOM.render( - , - container + var node = ReactDOM.render( + , + container, ); - document.body.appendChild(container); - - var node = ReactDOM.findDOMNode(stub); - - setUntrackedChecked.call(node, false); // track and set to false - dispatchEventOnNode(node, 'click'); // make it true - - expect(called).toBe(1); // onClick has been called - expect(node.checked).toBe(true); // tracked property is now true - - document.body.removeChild(container); - }); - - it('should catch setting the value programmatically', () => { - var container = document.createElement('div'); - var ref = ReactDOM.render( - , container - ); - var input = ReactDOM.findDOMNode(ref); - setUntrackedValue.call(input, 'bar'); + setUntrackedChecked.call(node, false); + node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + expect(called).toBe(1); + expect(node.checked).toBe(true); - expect(getTrackedValue(input)).toBe('bar'); + setUntrackedChecked.call(node, true); + node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + expect(called).toBe(2); + expect(node.checked).toBe(false); }); - it('should not fire change when setting the value programmatically', () => { + fit('should not fire change setting the value programmatically', () => { var called = 0; function cb(e) { - called += 1; + called++; expect(e.type).toBe('change'); } - var container = document.createElement('div'); - var ref = ReactDOM.render( - , container + var input = ReactDOM.render( + , + container, ); - var input = ReactDOM.findDOMNode(ref); input.value = 'bar'; - dispatchEventOnNode(input, 'change'); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); expect(called).toBe(0); setUntrackedValue.call(input, 'foo'); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + + expect(called).toBe(1); expect(input.value).toBe('foo'); - dispatchEventOnNode(input, 'change'); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); expect(called).toBe(1); }); - it('should not fire change when setting checked programmatically', () => { + fit('should not fire change when setting checked programmatically', () => { var called = 0; function cb(e) { - called += 1; + called++; expect(e.type).toBe('change'); } - var input = ReactTestUtils.renderIntoDocument( + var input = ReactDOM.render( , + container, ); + //set programmatically input.checked = true; - input.dispatchEvent(getEvent('click')); - expect(called).toBe(0); setUntrackedChecked.call(input, false); - input.dispatchEvent(getEvent('click')); + input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + expect(called).toBe(0); + + setUntrackedChecked.call(input, true); + input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); expect(called).toBe(1); }); - it('should unmount', () => { - var container = document.createElement('div'); + fit('should catch setting the value programmatically', () => { + var input = ReactDOM.render( + , + container, + ); + setUntrackedValue.call(input, 'bar'); + expect(getTrackedValue(input)).toBe('bar'); + }); + + fit('should unmount', () => { var input = ReactDOM.render(, container); ReactDOM.unmountComponentAtNode(container); }); - it('should only fire change for checked radio button once', () => { + fit('should only fire change for checked radio button once', () => { var called = 0; function cb(e) { - called += 1; + called++; + expect(e.type).toBe('change'); } - var input = ReactTestUtils.renderIntoDocument( + var input = ReactDOM.render( , + container, ); + setUntrackedChecked.call(input, true); - input.dispatchEvent(getEvent('click')); - input.dispatchEvent(getEvent('click')); + input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); expect(called).toBe(1); }); - it('should deduplicate input value change events', () => { - var input; + fit('should deduplicate input value change events', () => { var called = 0; function cb(e) { - called += 1; + called++; expect(e.type).toBe('change'); } - [ - , - , - , - ].forEach(function(element) { - called = 0; - input = ReactTestUtils.renderIntoDocument(element); - - setUntrackedValue.call(input, '40'); - input.dispatchEvent(getEvent('change')); - input.dispatchEvent(getEvent('change')); - expect(called).toBe(1); - - called = 0; - input = ReactTestUtils.renderIntoDocument(element); - setUntrackedValue.call(input, '40'); - input.dispatchEvent(getEvent('input')); - input.dispatchEvent(getEvent('input')); - expect(called).toBe(1); - - called = 0; - input = ReactTestUtils.renderIntoDocument(element); - setUntrackedValue.call(input, '40'); - input.dispatchEvent(getEvent('input')); - input.dispatchEvent(getEvent('change')); - expect(called).toBe(1); - }); + var input = ReactDOM.render(, container); + + setUntrackedValue.call(input, 'foo'); + + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); + + expect(called).toBe(1); + + setUntrackedValue.call(input, 'bar'); + + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + + expect(called).toBe(2); + + setUntrackedValue.call(input, 'foobar'); + + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); + + expect(called).toBe(3); }); - it('should listen for both change and input events when supported', () => { + fit('should listen for both change and input events when supported', () => { var called = 0; function cb(e) { - called += 1; + called++; expect(e.type).toBe('change'); } - var input = ReactTestUtils.renderIntoDocument( + var input = ReactDOM.render( , + container, ); - setUntrackedValue.call(input, 'bar'); - - input.dispatchEvent(getEvent('input')); - setUntrackedValue.call(input, 'foo'); + setUntrackedValue.call(input, '0'); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); - input.dispatchEvent(getEvent('change')); + setUntrackedValue.call(input, '1'); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); expect(called).toBe(2); }); - it('should only fire events when the value changes for range inputs', () => { + fit('should only fire events when the value changes for range inputs', () => { var called = 0; function cb(e) { - called += 1; + called++; expect(e.type).toBe('change'); } - var input = ReactTestUtils.renderIntoDocument( + var input = ReactDOM.render( , + container, ); setUntrackedValue.call(input, '40'); - input.dispatchEvent(getEvent('input')); - input.dispatchEvent(getEvent('change')); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); setUntrackedValue.call(input, 'foo'); + input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); + input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); - input.dispatchEvent(getEvent('input')); - input.dispatchEvent(getEvent('change')); expect(called).toBe(2); }); }); From 6238df36420a9f9ef97b8f9024131a45f311b272 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Fri, 24 Nov 2017 13:47:07 -0500 Subject: [PATCH 06/10] Fixed for revisions --- .../ChangeEventPlugin-test.internal.js | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) 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 e009f71a479d1..c90be635462cb 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js @@ -39,7 +39,7 @@ describe('ChangeEventPlugin', () => { container = null; }); - fit('should fire change for checkbox input', () => { + it('should fire change for checkbox input', () => { var called = 0; function cb(e) { @@ -52,6 +52,10 @@ describe('ChangeEventPlugin', () => { container, ); + + node.dispatchEvent(new Event('click')); + expect(called).toBe(0); + setUntrackedChecked.call(node, false); node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); expect(called).toBe(1); @@ -63,7 +67,7 @@ describe('ChangeEventPlugin', () => { expect(node.checked).toBe(false); }); - fit('should not fire change setting the value programmatically', () => { + it('should not fire change setting the value programmatically', () => { var called = 0; function cb(e) { @@ -90,7 +94,7 @@ describe('ChangeEventPlugin', () => { expect(called).toBe(1); }); - fit('should not fire change when setting checked programmatically', () => { + it('should not fire change when setting checked programmatically', () => { var called = 0; function cb(e) { @@ -116,22 +120,31 @@ describe('ChangeEventPlugin', () => { expect(called).toBe(1); }); - fit('should catch setting the value programmatically', () => { + it('should catch setting the value programmatically', () => { + var called = 0; + + function cb(e) { + called++; + expect(e.type).toBe('change'); + } + var input = ReactDOM.render( - , + , container, ); + setUntrackedValue.call(input, 'bar'); + expect(called).toBe(0); expect(getTrackedValue(input)).toBe('bar'); }); - fit('should unmount', () => { + it('should unmount', () => { var input = ReactDOM.render(, container); ReactDOM.unmountComponentAtNode(container); }); - fit('should only fire change for checked radio button once', () => { + it('should only fire change for checked radio button once', () => { var called = 0; function cb(e) { @@ -150,7 +163,7 @@ describe('ChangeEventPlugin', () => { expect(called).toBe(1); }); - fit('should deduplicate input value change events', () => { + it('should deduplicate input value change events', () => { var called = 0; function cb(e) { @@ -182,7 +195,7 @@ describe('ChangeEventPlugin', () => { expect(called).toBe(3); }); - fit('should listen for both change and input events when supported', () => { + it('should listen for both change and input events when supported', () => { var called = 0; function cb(e) { @@ -204,7 +217,7 @@ describe('ChangeEventPlugin', () => { expect(called).toBe(2); }); - fit('should only fire events when the value changes for range inputs', () => { + it('should only fire events when the value changes for range inputs', () => { var called = 0; function cb(e) { From 852e6638303400feae2befc71aa4c28d3ed15850 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Fri, 24 Nov 2017 14:03:06 -0500 Subject: [PATCH 07/10] Prettified --- .../src/events/__tests__/ChangeEventPlugin-test.internal.js | 1 - 1 file changed, 1 deletion(-) 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 c90be635462cb..e1ea5aaf62ea0 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js @@ -52,7 +52,6 @@ describe('ChangeEventPlugin', () => { container, ); - node.dispatchEvent(new Event('click')); expect(called).toBe(0); From 7bb212459b939716ea8c31bd67ff3ff7f853e6c6 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 24 Nov 2017 22:00:21 +0000 Subject: [PATCH 08/10] Add more details and fixes to tests --- .../ChangeEventPlugin-test.internal.js | 141 ++++++++++-------- 1 file changed, 82 insertions(+), 59 deletions(-) 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 e1ea5aaf62ea0..9fcf4e0dd1986 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js @@ -52,18 +52,21 @@ describe('ChangeEventPlugin', () => { container, ); - node.dispatchEvent(new Event('click')); - expect(called).toBe(0); - - setUntrackedChecked.call(node, false); - node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); - expect(called).toBe(1); + 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. expect(node.checked).toBe(true); + expect(called).toBe(1); - setUntrackedChecked.call(node, true); - node.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); - expect(called).toBe(2); + expect(node.checked).toBe(true); + node.dispatchEvent( + new MouseEvent('click', {bubbles: true, cancelable: true}), + ); expect(node.checked).toBe(false); + expect(called).toBe(2); }); it('should not fire change setting the value programmatically', () => { @@ -79,20 +82,36 @@ describe('ChangeEventPlugin', () => { container, ); + // We try to avoid firing "duplicate" React change events. + // However, to tell which events are duplicates and should be ignored, + // we are tracking the "current" input value, and only respect events + // that occur after it changes. In this test, we verify that we can + // keep track of the "current" value even if it is set programatically. + + // Set it programmatically. input.value = 'bar'; + // Even if a DOM input 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('input', {bubbles: true, cancelable: true})); + expect(input.value).toBe('bar'); + // In this case we don't expect to get a React event. expect(called).toBe(0); + // However, we can simulate user typing by calling the underlying setter. setUntrackedValue.call(input, 'foo'); + // Now, when the event fires, the real input value ('foo') differs from the + // "current" one we previously recorded ('bar'). input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); - - expect(called).toBe(1); expect(input.value).toBe('foo'); + // 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('input', {bubbles: true, cancelable: true})); expect(called).toBe(1); }); + // See a similar input test above for a detailed description of why. it('should not fire change when setting checked programmatically', () => { var called = 0; @@ -102,41 +121,29 @@ describe('ChangeEventPlugin', () => { } var input = ReactDOM.render( - , + , container, ); - //set programmatically + // Set the value, updating the "current" value that React tracks to true. input.checked = true; - + // Under the hood, uncheck the box so that the click will "check" it again. setUntrackedChecked.call(input, false); - input.dispatchEvent(new Event('click', {bubbles: true, cancelable: true})); + input.dispatchEvent( + new MouseEvent('click', {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). expect(called).toBe(0); - setUntrackedChecked.call(input, true); + // 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})); - expect(called).toBe(1); }); - it('should catch setting the value programmatically', () => { - var called = 0; - - function cb(e) { - called++; - expect(e.type).toBe('change'); - } - - var input = ReactDOM.render( - , - container, - ); - - setUntrackedValue.call(input, 'bar'); - expect(called).toBe(0); - expect(getTrackedValue(input)).toBe('bar'); - }); - it('should unmount', () => { var input = ReactDOM.render(, container); @@ -170,28 +177,44 @@ describe('ChangeEventPlugin', () => { expect(e.type).toBe('change'); } - var input = ReactDOM.render(, container); - - setUntrackedValue.call(input, 'foo'); - - input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); - input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); - - expect(called).toBe(1); - - setUntrackedValue.call(input, 'bar'); - - input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); - input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); - - expect(called).toBe(2); - - setUntrackedValue.call(input, 'foobar'); - - input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); - input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); - - expect(called).toBe(3); + var input; + ['text', 'number', 'range'].forEach(type => { + called = 0; + input = ReactDOM.render(, container); + setUntrackedValue.call(input, '42'); + input.dispatchEvent( + new Event('change', {bubbles: true, cancelable: true}), + ); + input.dispatchEvent( + new Event('change', {bubbles: true, cancelable: true}), + ); + expect(called).toBe(1); + ReactDOM.unmountComponentAtNode(container); + + called = 0; + input = ReactDOM.render(, container); + setUntrackedValue.call(input, '42'); + input.dispatchEvent( + new Event('input', {bubbles: true, cancelable: true}), + ); + input.dispatchEvent( + new Event('input', {bubbles: true, cancelable: true}), + ); + expect(called).toBe(1); + ReactDOM.unmountComponentAtNode(container); + + called = 0; + input = ReactDOM.render(, container); + setUntrackedValue.call(input, '42'); + input.dispatchEvent( + new Event('input', {bubbles: true, cancelable: true}), + ); + input.dispatchEvent( + new Event('change', {bubbles: true, cancelable: true}), + ); + expect(called).toBe(1); + ReactDOM.unmountComponentAtNode(container); + }); }); it('should listen for both change and input events when supported', () => { @@ -207,10 +230,10 @@ describe('ChangeEventPlugin', () => { container, ); - setUntrackedValue.call(input, '0'); + setUntrackedValue.call(input, 'foo'); input.dispatchEvent(new Event('input', {bubbles: true, cancelable: true})); - setUntrackedValue.call(input, '1'); + setUntrackedValue.call(input, 'bar'); input.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); expect(called).toBe(2); From 0570084a9c65ff128f8e493119fdb98546d3ba22 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 24 Nov 2017 22:04:41 +0000 Subject: [PATCH 09/10] Not internal anymore --- ...angeEventPlugin-test.internal.js => ChangeEventPlugin-test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/react-dom/src/events/__tests__/{ChangeEventPlugin-test.internal.js => ChangeEventPlugin-test.js} (100%) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js similarity index 100% rename from packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js rename to packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js From d34a205b46d465eeb1448c1fafbd2419fffbe704 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 24 Nov 2017 22:07:17 +0000 Subject: [PATCH 10/10] Remove unused code --- .../react-dom/src/events/__tests__/ChangeEventPlugin-test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js index 9fcf4e0dd1986..431ece116be89 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.js @@ -12,10 +12,6 @@ var React = require('react'); var ReactDOM = require('react-dom'); -function getTrackedValue(elem) { - return elem.value; -} - var setUntrackedChecked = Object.getOwnPropertyDescriptor( HTMLInputElement.prototype, 'checked',