Skip to content

Commit

Permalink
Properly set value and defaultValue for input and textarea (#6406)
Browse files Browse the repository at this point in the history 
* Have `defaultValue` reach DOM node for html input box for #4618

* Cleanup and bug fixes for merge.
  • Loading branch information
@jimfb
jimfb committed May 25, 2016
1 parent cb4a0af commit 4338c8d
Show file tree
Hide file tree
Showing 8 changed files with 367 additions and 115 deletions.
45 changes: 36 additions & 9 deletions src/renderers/dom/client/wrappers/ReactDOMInput.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ var ReactDOMInput = {

var defaultValue = props.defaultValue;
inst._wrapperState = {
initialChecked: props.defaultChecked || false,
initialValue: defaultValue != null ? defaultValue : null,
initialChecked: props.checked != null ? props.checked : props.defaultChecked,
initialValue: props.value != null ? props.value : defaultValue,
listeners: null,
onChange: _handleChange.bind(inst),
};
Expand All @@ -166,13 +166,12 @@ var ReactDOMInput = {
if (__DEV__) {
warnIfValueIsNull(props);

var initialValue = inst._wrapperState.initialChecked || inst._wrapperState.initialValue;
var defaultValue = props.defaultChecked || props.defaultValue;
var controlled = props.checked !== undefined || props.value !== undefined;
var owner = inst._currentElement._owner;

if (
(initialValue || !inst._wrapperState.controlled) &&
!inst._wrapperState.controlled &&
controlled && !didWarnUncontrolledToControlled
) {
warning(
Expand Down Expand Up @@ -214,17 +213,45 @@ var ReactDOMInput = {
);
}

var node = ReactDOMComponentTree.getNodeFromInstance(inst);
var value = LinkedValueUtils.getValue(props);
if (value != null) {

// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
DOMPropertyOperations.setValueForProperty(
ReactDOMComponentTree.getNodeFromInstance(inst),
'value',
'' + value
);
var newValue = '' + value;

// To avoid side effects (such as losing text selection), only set value if changed
if (newValue !== node.value) {
node.value = newValue;
}
} else {
if (props.value == null && props.defaultValue != null) {
node.defaultValue = '' + props.defaultValue;
}
if (props.checked == null && props.defaultChecked != null) {
node.defaultChecked = !!props.defaultChecked;
}
}
},

postMountWrapper: function(inst) {
// This is in postMount because we need access to the DOM node, which is not
// available until after the component has mounted.
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
node.value = node.value; // Detach value from defaultValue

// Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
// this is needed to work around a chrome bug where setting defaultChecked
// will sometimes influence the value of checked (even after detachment).
// Reference: https://bugs.chromium.org/p/chromium/issues/detail?id=608416
// We need to temporarily unset name to avoid disrupting radio button groups.
var name = node.name;
node.name = undefined;
node.defaultChecked = !node.defaultChecked;
node.defaultChecked = !node.defaultChecked;
node.name = name;
},
};

function _handleChange(event) {
Expand Down
99 changes: 60 additions & 39 deletions src/renderers/dom/client/wrappers/ReactDOMTextarea.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
'use strict';

var DisabledInputUtils = require('DisabledInputUtils');
var DOMPropertyOperations = require('DOMPropertyOperations');
var LinkedValueUtils = require('LinkedValueUtils');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactUpdates = require('ReactUpdates');
Expand Down Expand Up @@ -67,11 +66,14 @@ var ReactDOMTextarea = {
);

// Always set children to the same thing. In IE9, the selection range will
// get reset if `textContent` is mutated.
// get reset if `textContent` is mutated. We could add a check in setTextContent
// to only set the value if/when the value differs from the node value (which would
// completely solve this IE9 bug), but Sebastian+Ben seemed to like this solution.
// The value can be a boolean or object so that's why it's forced to be a string.
var hostProps = Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
defaultValue: undefined,
value: undefined,
children: inst._wrapperState.initialValue,
defaultValue: undefined,
children: '' + inst._wrapperState.initialValue,
onChange: inst._wrapperState.onChange,
});

Expand Down Expand Up @@ -110,41 +112,45 @@ var ReactDOMTextarea = {
warnIfValueIsNull(props);
}

var defaultValue = props.defaultValue;
// TODO (yungsters): Remove support for children content in <textarea>.
var children = props.children;
if (children != null) {
if (__DEV__) {
warning(
false,
'Use the `defaultValue` or `value` props instead of setting ' +
'children on <textarea>.'
);
}
invariant(
defaultValue == null,
'If you supply `defaultValue` on a <textarea>, do not pass children.'
);
if (Array.isArray(children)) {

var value = LinkedValueUtils.getValue(props);
var initialValue = value;

// Only bother fetching default value if we're going to use it
if (value == null) {
var defaultValue = props.defaultValue;
// TODO (yungsters): Remove support for children content in <textarea>.
var children = props.children;
if (children != null) {
if (__DEV__) {
warning(
false,
'Use the `defaultValue` or `value` props instead of setting ' +
'children on <textarea>.'
);
}
invariant(
children.length <= 1,
'<textarea> can only have at most one child.'
defaultValue == null,
'If you supply `defaultValue` on a <textarea>, do not pass children.'
);
children = children[0];
if (Array.isArray(children)) {
invariant(
children.length <= 1,
'<textarea> can only have at most one child.'
);
children = children[0];
}

defaultValue = '' + children;
}

defaultValue = '' + children;
}
if (defaultValue == null) {
defaultValue = '';
if (defaultValue == null) {
defaultValue = '';
}
initialValue = defaultValue;
}
var value = LinkedValueUtils.getValue(props);

inst._wrapperState = {
// We save the initial value so that `ReactDOMComponent` doesn't update
// `textContent` (unnecessary since we update value).
// The initial value can be a boolean or object so that's why it's
// forced to be a string.
initialValue: '' + (value != null ? value : defaultValue),
initialValue: '' + initialValue,
listeners: null,
onChange: _handleChange.bind(inst),
};
Expand All @@ -157,17 +163,32 @@ var ReactDOMTextarea = {
warnIfValueIsNull(props);
}

var node = ReactDOMComponentTree.getNodeFromInstance(inst);
var value = LinkedValueUtils.getValue(props);
if (value != null) {
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
DOMPropertyOperations.setValueForProperty(
ReactDOMComponentTree.getNodeFromInstance(inst),
'value',
'' + value
);
var newValue = '' + value;

// To avoid side effects (such as losing text selection), only set value if changed
if (newValue !== node.value) {
node.value = newValue;
}
if (props.defaultValue == null) {
node.defaultValue = newValue;
}
}
if (props.defaultValue != null) {
node.defaultValue = props.defaultValue;
}
},

postMountWrapper: function(inst) {
// This is in postMount because we need access to the DOM node, which is not
// available until after the component has mounted.
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
node.value = node.value; // Detach value from defaultValue
},
};

function _handleChange(event) {
Expand Down
89 changes: 83 additions & 6 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('ReactDOMInput', function() {
var EventConstants;
var React;
var ReactDOM;
var ReactDOMServer;
var ReactDOMFeatureFlags;
var ReactLink;
var ReactTestUtils;
Expand All @@ -35,6 +36,7 @@ describe('ReactDOMInput', function() {
EventConstants = require('EventConstants');
React = require('React');
ReactDOM = require('ReactDOM');
ReactDOMServer = require('ReactDOMServer');
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
ReactLink = require('ReactLink');
ReactTestUtils = require('ReactTestUtils');
Expand All @@ -47,6 +49,7 @@ describe('ReactDOMInput', function() {
stub = ReactTestUtils.renderIntoDocument(stub);
var node = ReactDOM.findDOMNode(stub);

expect(node.getAttribute('value')).toBe('0');
expect(node.value).toBe('0');
});

Expand All @@ -66,6 +69,48 @@ describe('ReactDOMInput', function() {
expect(node.value).toBe('false');
});

it('should update `defaultValue` for uncontrolled input', function() {
var container = document.createElement('div');

var node = ReactDOM.render(<input type="text" defaultValue="0" />, container);

expect(node.value).toBe('0');

ReactDOM.render(<input type="text" defaultValue="1" />, container);

expect(node.value).toBe('0');
expect(node.defaultValue).toBe('1');
});

it('should take `defaultValue` when changing to uncontrolled input', function() {
var container = document.createElement('div');

var node = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);

expect(node.value).toBe('0');

ReactDOM.render(<input type="text" defaultValue="1" />, container);

expect(node.value).toBe('0');
});

it('should render defaultValue for SSR', function() {
var markup = ReactDOMServer.renderToString(<input type="text" defaultValue="1" />);
var div = document.createElement('div');
div.innerHTML = markup;
expect(div.firstChild.getAttribute('value')).toBe('1');
expect(div.firstChild.getAttribute('defaultValue')).toBe(null);
});

it('should render value for SSR', function() {
var element = <input type="text" value="1" onChange={function() {}} />;
var markup = ReactDOMServer.renderToString(element);
var div = document.createElement('div');
div.innerHTML = markup;
expect(div.firstChild.getAttribute('value')).toBe('1');
expect(div.firstChild.getAttribute('defaultValue')).toBe(null);
});

it('should display "foobar" for `defaultValue` of `objToString`', function() {
var objToString = {
toString: function() {
Expand All @@ -91,8 +136,7 @@ describe('ReactDOMInput', function() {
it('should allow setting `value` to `true`', function() {
var container = document.createElement('div');
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
stub = ReactDOM.render(stub, container);
var node = ReactDOM.findDOMNode(stub);
var node = ReactDOM.render(stub, container);

expect(node.value).toBe('yolo');

Expand All @@ -106,8 +150,7 @@ describe('ReactDOMInput', function() {
it('should allow setting `value` to `false`', function() {
var container = document.createElement('div');
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
stub = ReactDOM.render(stub, container);
var node = ReactDOM.findDOMNode(stub);
var node = ReactDOM.render(stub, container);

expect(node.value).toBe('yolo');

Expand All @@ -121,8 +164,7 @@ describe('ReactDOMInput', function() {
it('should allow setting `value` to `objToString`', function() {
var container = document.createElement('div');
var stub = <input type="text" value="foo" onChange={emptyFunction} />;
stub = ReactDOM.render(stub, container);
var node = ReactDOM.findDOMNode(stub);
var node = ReactDOM.render(stub, container);

expect(node.value).toBe('foo');

Expand All @@ -138,6 +180,29 @@ describe('ReactDOMInput', function() {
expect(node.value).toEqual('foobar');
});

it('should not incur unnecessary DOM mutations', function() {
var container = document.createElement('div');
ReactDOM.render(<input value="a" />, container);

var node = container.firstChild;
var nodeValue = 'a';
var nodeValueSetter = jest.genMockFn();
Object.defineProperty(node, 'value', {
get: function() {
return nodeValue;
},
set: nodeValueSetter.mockImplementation(function(newValue) {
nodeValue = newValue;
}),
});

ReactDOM.render(<input value="a" />, container);
expect(nodeValueSetter.mock.calls.length).toBe(0);

ReactDOM.render(<input value="b"/>, container);
expect(nodeValueSetter.mock.calls.length).toBe(1);
});

it('should properly control a value of number `0`', function() {
var stub = <input type="text" value={0} onChange={emptyFunction} />;
stub = ReactTestUtils.renderIntoDocument(stub);
Expand Down Expand Up @@ -399,6 +464,13 @@ describe('ReactDOMInput', function() {

});

it('should update defaultValue to empty string', function() {
var container = document.createElement('div');
ReactDOM.render(<input type="text" defaultValue={'foo'} />, container);
ReactDOM.render(<input type="text" defaultValue={''} />, container);
expect(container.firstChild.defaultValue).toBe('');
});

it('should throw if both checkedLink and valueLink are provided', function() {
var node = document.createElement('div');
var link = new ReactLink(true, jest.fn());
Expand Down Expand Up @@ -618,6 +690,11 @@ describe('ReactDOMInput', function() {
'set data-reactroot',
'set type',
'set value',
'set value',
'set name',
'set checked',
'set checked',
'set name',
]);
});

Expand Down
Loading

0 comments on commit 4338c8d

Please sign in to comment.