From a61f8baed940d97df8cb4544f715e8128f3463cb Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 7 Nov 2016 16:42:42 -0800 Subject: [PATCH] Ensure that we're listening to all onChange dependencies for controlled inputs 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. --- .../wrappers/__tests__/ReactDOMInput-test.js | 27 ++++++++++++++++++ .../dom/stack/client/ReactDOMComponent.js | 28 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index d9ffa5ba3e9b2..d61b5cca2be0d 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -40,6 +40,32 @@ describe('ReactDOMInput', () => { spyOn(console, 'error'); }); + it('should properly control a value even if no event listener exists', () => { + var container = document.createElement('div'); + var stub = ReactDOM.render( + , + container + ); + + document.body.appendChild(container); + + var node = ReactDOM.findDOMNode(stub); + expect(console.error.calls.count()).toBe(1); + + // Simulate a native change event + setUntrackedValue(node, 'giraffe'); + + // This must use the native event dispatching. If we simulate, we will + // bypass the lazy event attachment system so we won't actually test this. + var nativeEvent = document.createEvent('Event'); + nativeEvent.initEvent('change', true, true); + node.dispatchEvent(nativeEvent); + + expect(node.value).toBe('lion'); + + document.body.removeChild(container); + }); + it('should display `defaultValue` of number 0', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub); @@ -455,6 +481,7 @@ describe('ReactDOMInput', () => { expect(console.error.calls.count()).toBe(1); }); + it('should have a this value of undefined if bind is not used', () => { var unboundInputOnChange = function() { expect(this).toBe(undefined); diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index bc23bc144fddf..e0fd44e83da37 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -228,6 +228,25 @@ function enqueuePutListener(inst, registrationName, listener, transaction) { }); } +// TODO: This is coming from future #8192. Dedupe this and enqueuePutListener. +function ensureListeningTo(inst, registrationName, transaction) { + if (transaction instanceof ReactServerRenderingTransaction) { + return; + } + if (__DEV__) { + // IE8 has no API for event capturing and the `onScroll` event doesn't + // bubble. + warning( + registrationName !== 'onScroll' || isEventSupported('scroll', true), + 'This browser doesn\'t support the `onScroll` event' + ); + } + var containerInfo = inst._hostContainerInfo; + var isDocumentFragment = containerInfo._node && containerInfo._node.nodeType === DOC_FRAGMENT_TYPE; + var doc = isDocumentFragment ? containerInfo._node : containerInfo._ownerDocument; + listenTo(registrationName, doc); +} + function putListener() { var listenerToPut = this; EventPluginHub.putListener( @@ -561,6 +580,9 @@ ReactDOMComponent.Mixin = { props = ReactDOMInput.getHostProps(this, props); transaction.getReactMountReady().enqueue(trackInputValue, this); transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this); + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(this, 'onChange', transaction); break; case 'option': ReactDOMOption.mountWrapper(this, props, hostParent); @@ -570,12 +592,18 @@ ReactDOMComponent.Mixin = { ReactDOMSelect.mountWrapper(this, props, hostParent); props = ReactDOMSelect.getHostProps(this, props); transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this); + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(this, 'onChange', transaction); break; case 'textarea': ReactDOMTextarea.mountWrapper(this, props, hostParent); props = ReactDOMTextarea.getHostProps(this, props); transaction.getReactMountReady().enqueue(trackInputValue, this); transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this); + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(this, 'onChange', transaction); break; }