@samitouri / QOS-React / commits / a93ab6b1af

Ensure that we're listening to all onChange dependencies for controlled inputs (#8229)

When there is no onChange event handler, we should still listen to it to ensure controlled values get reset. There is warning associated with this but the prod behavior should still be respected.

Sebastian Markbåge committed Nov 7, 2016 at 16:47 UTC a93ab6b1afe5652e3cea1fc24bc433c25f6653e5
2 files changed +55
src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
+27
@@ -40,6 +40,32 @@ describe('ReactDOMInput', () => {
40 spyOn(console, 'error');
41 });
42
43 + it('should properly control a value even if no event listener exists', () => {
44 + var container = document.createElement('div');
45 + var stub = ReactDOM.render(
46 + <input type="text" value="lion" />,
47 + container
48 + );
49 +
50 + document.body.appendChild(container);
51 +
52 + var node = ReactDOM.findDOMNode(stub);
53 + expect(console.error.calls.count()).toBe(1);
54 +
55 + // Simulate a native change event
56 + setUntrackedValue(node, 'giraffe');
57 +
58 + // This must use the native event dispatching. If we simulate, we will
59 + // bypass the lazy event attachment system so we won't actually test this.
60 + var nativeEvent = document.createEvent('Event');
61 + nativeEvent.initEvent('change', true, true);
62 + node.dispatchEvent(nativeEvent);
63 +
64 + expect(node.value).toBe('lion');
65 +
66 + document.body.removeChild(container);
67 + });
68 +
69 it('should display `defaultValue` of number 0', () => {
70 var stub = <input type="text" defaultValue={0} />;
71 stub = ReactTestUtils.renderIntoDocument(stub);
@@ -455,6 +481,7 @@ describe('ReactDOMInput', () => {
481 expect(console.error.calls.count()).toBe(1);
482 });
483
484 +
485 it('should have a this value of undefined if bind is not used', () => {
486 var unboundInputOnChange = function() {
487 expect(this).toBe(undefined);
src/renderers/dom/stack/client/ReactDOMComponent.js
+28
@@ -228,6 +228,25 @@ function enqueuePutListener(inst, registrationName, listener, transaction) {
228 });
229 }
230
231 +// TODO: This is coming from future #8192. Dedupe this and enqueuePutListener.
232 +function ensureListeningTo(inst, registrationName, transaction) {
233 + if (transaction instanceof ReactServerRenderingTransaction) {
234 + return;
235 + }
236 + if (__DEV__) {
237 + // IE8 has no API for event capturing and the `onScroll` event doesn't
238 + // bubble.
239 + warning(
240 + registrationName !== 'onScroll' || isEventSupported('scroll', true),
241 + 'This browser doesn\'t support the `onScroll` event'
242 + );
243 + }
244 + var containerInfo = inst._hostContainerInfo;
245 + var isDocumentFragment = containerInfo._node && containerInfo._node.nodeType === DOC_FRAGMENT_TYPE;
246 + var doc = isDocumentFragment ? containerInfo._node : containerInfo._ownerDocument;
247 + listenTo(registrationName, doc);
248 +}
249 +
250 function putListener() {
251 var listenerToPut = this;
252 EventPluginHub.putListener(
@@ -561,6 +580,9 @@ ReactDOMComponent.Mixin = {
580 props = ReactDOMInput.getHostProps(this, props);
581 transaction.getReactMountReady().enqueue(trackInputValue, this);
582 transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this);
583 + // For controlled components we always need to ensure we're listening
584 + // to onChange. Even if there is no listener.
585 + ensureListeningTo(this, 'onChange', transaction);
586 break;
587 case 'option':
588 ReactDOMOption.mountWrapper(this, props, hostParent);
@@ -570,12 +592,18 @@ ReactDOMComponent.Mixin = {
592 ReactDOMSelect.mountWrapper(this, props, hostParent);
593 props = ReactDOMSelect.getHostProps(this, props);
594 transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this);
595 + // For controlled components we always need to ensure we're listening
596 + // to onChange. Even if there is no listener.
597 + ensureListeningTo(this, 'onChange', transaction);
598 break;
599 case 'textarea':
600 ReactDOMTextarea.mountWrapper(this, props, hostParent);
601 props = ReactDOMTextarea.getHostProps(this, props);
602 transaction.getReactMountReady().enqueue(trackInputValue, this);
603 transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this);
604 + // For controlled components we always need to ensure we're listening
605 + // to onChange. Even if there is no listener.
606 + ensureListeningTo(this, 'onChange', transaction);
607 break;
608 }
609