Skip to content

Commit

Permalink
Ensure that we're listening to all onChange dependencies for controll…
Browse files Browse the repository at this point in the history
…ed 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.
  • Loading branch information
sebmarkbage committed Nov 8, 2016
1 parent a92b830 commit a61f8ba
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 0 deletions.
27 changes: 27 additions & 0 deletions src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<input type="text" value="lion" />,
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 = <input type="text" defaultValue={0} />;
stub = ReactTestUtils.renderIntoDocument(stub);
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions src/renderers/dom/stack/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down

0 comments on commit a61f8ba

Please sign in to comment.