Skip to content

Commit 262f85d

Browse files
jimfbzpao
jimfb
authored and
zpao
committed
Properly set value and defaultValue for input and textarea (facebook#6406)
* Have `defaultValue` reach DOM node for html input box for facebook#4618 * Cleanup and bug fixes for merge. (cherry picked from commit 4338c8d)
1 parent 23d2540 commit 262f85d

File tree

8 files changed

+369
-117
lines changed

8 files changed

+369
-117
lines changed

‎src/renderers/dom/client/wrappers/ReactDOMInput.js

+36-9
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ var ReactDOMInput = {
149149

150150
var defaultValue = props.defaultValue;
151151
inst._wrapperState = {
152-
initialChecked: props.defaultChecked || false,
153-
initialValue: defaultValue != null ? defaultValue : null,
152+
initialChecked: props.checked != null ? props.checked : props.defaultChecked,
153+
initialValue: props.value != null ? props.value : defaultValue,
154154
listeners: null,
155155
onChange: _handleChange.bind(inst),
156156
};
@@ -166,13 +166,12 @@ var ReactDOMInput = {
166166
if (__DEV__) {
167167
warnIfValueIsNull(props);
168168

169-
var initialValue = inst._wrapperState.initialChecked || inst._wrapperState.initialValue;
170169
var defaultValue = props.defaultChecked || props.defaultValue;
171170
var controlled = props.checked !== undefined || props.value !== undefined;
172171
var owner = inst._currentElement._owner;
173172

174173
if (
175-
(initialValue || !inst._wrapperState.controlled) &&
174+
!inst._wrapperState.controlled &&
176175
controlled && !didWarnUncontrolledToControlled
177176
) {
178177
warning(
@@ -214,17 +213,45 @@ var ReactDOMInput = {
214213
);
215214
}
216215

216+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
217217
var value = LinkedValueUtils.getValue(props);
218218
if (value != null) {
219+
219220
// Cast `value` to a string to ensure the value is set correctly. While
220221
// browsers typically do this as necessary, jsdom doesn't.
221-
DOMPropertyOperations.setValueForProperty(
222-
ReactDOMComponentTree.getNodeFromInstance(inst),
223-
'value',
224-
'' + value
225-
);
222+
var newValue = '' + value;
223+
224+
// To avoid side effects (such as losing text selection), only set value if changed
225+
if (newValue !== node.value) {
226+
node.value = newValue;
227+
}
228+
} else {
229+
if (props.value == null && props.defaultValue != null) {
230+
node.defaultValue = '' + props.defaultValue;
231+
}
232+
if (props.checked == null && props.defaultChecked != null) {
233+
node.defaultChecked = !!props.defaultChecked;
234+
}
226235
}
227236
},
237+
238+
postMountWrapper: function(inst) {
239+
// This is in postMount because we need access to the DOM node, which is not
240+
// available until after the component has mounted.
241+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
242+
node.value = node.value; // Detach value from defaultValue
243+
244+
// Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
245+
// this is needed to work around a chrome bug where setting defaultChecked
246+
// will sometimes influence the value of checked (even after detachment).
247+
// Reference: https://bugs.chromium.org/p/chromium/issues/detail?id=608416
248+
// We need to temporarily unset name to avoid disrupting radio button groups.
249+
var name = node.name;
250+
node.name = undefined;
251+
node.defaultChecked = !node.defaultChecked;
252+
node.defaultChecked = !node.defaultChecked;
253+
node.name = name;
254+
},
228255
};
229256

230257
function _handleChange(event) {

‎src/renderers/dom/client/wrappers/ReactDOMTextarea.js

+60-39
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
'use strict';
1313

1414
var DisabledInputUtils = require('DisabledInputUtils');
15-
var DOMPropertyOperations = require('DOMPropertyOperations');
1615
var LinkedValueUtils = require('LinkedValueUtils');
1716
var ReactDOMComponentTree = require('ReactDOMComponentTree');
1817
var ReactUpdates = require('ReactUpdates');
@@ -67,11 +66,14 @@ var ReactDOMTextarea = {
6766
);
6867

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

@@ -110,41 +112,45 @@ var ReactDOMTextarea = {
110112
warnIfValueIsNull(props);
111113
}
112114

113-
var defaultValue = props.defaultValue;
114-
// TODO (yungsters): Remove support for children content in <textarea>.
115-
var children = props.children;
116-
if (children != null) {
117-
if (__DEV__) {
118-
warning(
119-
false,
120-
'Use the `defaultValue` or `value` props instead of setting ' +
121-
'children on <textarea>.'
122-
);
123-
}
124-
invariant(
125-
defaultValue == null,
126-
'If you supply `defaultValue` on a <textarea>, do not pass children.'
127-
);
128-
if (Array.isArray(children)) {
115+
116+
var value = LinkedValueUtils.getValue(props);
117+
var initialValue = value;
118+
119+
// Only bother fetching default value if we're going to use it
120+
if (value == null) {
121+
var defaultValue = props.defaultValue;
122+
// TODO (yungsters): Remove support for children content in <textarea>.
123+
var children = props.children;
124+
if (children != null) {
125+
if (__DEV__) {
126+
warning(
127+
false,
128+
'Use the `defaultValue` or `value` props instead of setting ' +
129+
'children on <textarea>.'
130+
);
131+
}
129132
invariant(
130-
children.length <= 1,
131-
'<textarea> can only have at most one child.'
133+
defaultValue == null,
134+
'If you supply `defaultValue` on a <textarea>, do not pass children.'
132135
);
133-
children = children[0];
136+
if (Array.isArray(children)) {
137+
invariant(
138+
children.length <= 1,
139+
'<textarea> can only have at most one child.'
140+
);
141+
children = children[0];
142+
}
143+
144+
defaultValue = '' + children;
134145
}
135-
136-
defaultValue = '' + children;
137-
}
138-
if (defaultValue == null) {
139-
defaultValue = '';
146+
if (defaultValue == null) {
147+
defaultValue = '';
148+
}
149+
initialValue = defaultValue;
140150
}
141-
var value = LinkedValueUtils.getValue(props);
151+
142152
inst._wrapperState = {
143-
// We save the initial value so that `ReactDOMComponent` doesn't update
144-
// `textContent` (unnecessary since we update value).
145-
// The initial value can be a boolean or object so that's why it's
146-
// forced to be a string.
147-
initialValue: '' + (value != null ? value : defaultValue),
153+
initialValue: '' + initialValue,
148154
listeners: null,
149155
onChange: _handleChange.bind(inst),
150156
};
@@ -157,17 +163,32 @@ var ReactDOMTextarea = {
157163
warnIfValueIsNull(props);
158164
}
159165

166+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
160167
var value = LinkedValueUtils.getValue(props);
161168
if (value != null) {
162169
// Cast `value` to a string to ensure the value is set correctly. While
163170
// browsers typically do this as necessary, jsdom doesn't.
164-
DOMPropertyOperations.setValueForProperty(
165-
ReactDOMComponentTree.getNodeFromInstance(inst),
166-
'value',
167-
'' + value
168-
);
171+
var newValue = '' + value;
172+
173+
// To avoid side effects (such as losing text selection), only set value if changed
174+
if (newValue !== node.value) {
175+
node.value = newValue;
176+
}
177+
if (props.defaultValue == null) {
178+
node.defaultValue = newValue;
179+
}
180+
}
181+
if (props.defaultValue != null) {
182+
node.defaultValue = props.defaultValue;
169183
}
170184
},
185+
186+
postMountWrapper: function(inst) {
187+
// This is in postMount because we need access to the DOM node, which is not
188+
// available until after the component has mounted.
189+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
190+
node.value = node.value; // Detach value from defaultValue
191+
},
171192
};
172193

173194
function _handleChange(event) {

‎src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js

+83-6
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ describe('ReactDOMInput', function() {
1818
var EventConstants;
1919
var React;
2020
var ReactDOM;
21+
var ReactDOMServer;
2122
var ReactDOMFeatureFlags;
2223
var ReactLink;
2324
var ReactTestUtils;
@@ -27,6 +28,7 @@ describe('ReactDOMInput', function() {
2728
EventConstants = require('EventConstants');
2829
React = require('React');
2930
ReactDOM = require('ReactDOM');
31+
ReactDOMServer = require('ReactDOMServer');
3032
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
3133
ReactLink = require('ReactLink');
3234
ReactTestUtils = require('ReactTestUtils');
@@ -38,6 +40,7 @@ describe('ReactDOMInput', function() {
3840
stub = ReactTestUtils.renderIntoDocument(stub);
3941
var node = ReactDOM.findDOMNode(stub);
4042

43+
expect(node.getAttribute('value')).toBe('0');
4144
expect(node.value).toBe('0');
4245
});
4346

@@ -57,6 +60,48 @@ describe('ReactDOMInput', function() {
5760
expect(node.value).toBe('false');
5861
});
5962

63+
it('should update `defaultValue` for uncontrolled input', function() {
64+
var container = document.createElement('div');
65+
66+
var node = ReactDOM.render(<input type="text" defaultValue="0" />, container);
67+
68+
expect(node.value).toBe('0');
69+
70+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
71+
72+
expect(node.value).toBe('0');
73+
expect(node.defaultValue).toBe('1');
74+
});
75+
76+
it('should take `defaultValue` when changing to uncontrolled input', function() {
77+
var container = document.createElement('div');
78+
79+
var node = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);
80+
81+
expect(node.value).toBe('0');
82+
83+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
84+
85+
expect(node.value).toBe('0');
86+
});
87+
88+
it('should render defaultValue for SSR', function() {
89+
var markup = ReactDOMServer.renderToString(<input type="text" defaultValue="1" />);
90+
var div = document.createElement('div');
91+
div.innerHTML = markup;
92+
expect(div.firstChild.getAttribute('value')).toBe('1');
93+
expect(div.firstChild.getAttribute('defaultValue')).toBe(null);
94+
});
95+
96+
it('should render value for SSR', function() {
97+
var element = <input type="text" value="1" onChange={function() {}} />;
98+
var markup = ReactDOMServer.renderToString(element);
99+
var div = document.createElement('div');
100+
div.innerHTML = markup;
101+
expect(div.firstChild.getAttribute('value')).toBe('1');
102+
expect(div.firstChild.getAttribute('defaultValue')).toBe(null);
103+
});
104+
60105
it('should display "foobar" for `defaultValue` of `objToString`', function() {
61106
var objToString = {
62107
toString: function() {
@@ -82,8 +127,7 @@ describe('ReactDOMInput', function() {
82127
it('should allow setting `value` to `true`', function() {
83128
var container = document.createElement('div');
84129
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
85-
stub = ReactDOM.render(stub, container);
86-
var node = ReactDOM.findDOMNode(stub);
130+
var node = ReactDOM.render(stub, container);
87131

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

@@ -97,8 +141,7 @@ describe('ReactDOMInput', function() {
97141
it('should allow setting `value` to `false`', function() {
98142
var container = document.createElement('div');
99143
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
100-
stub = ReactDOM.render(stub, container);
101-
var node = ReactDOM.findDOMNode(stub);
144+
var node = ReactDOM.render(stub, container);
102145

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

@@ -112,8 +155,7 @@ describe('ReactDOMInput', function() {
112155
it('should allow setting `value` to `objToString`', function() {
113156
var container = document.createElement('div');
114157
var stub = <input type="text" value="foo" onChange={emptyFunction} />;
115-
stub = ReactDOM.render(stub, container);
116-
var node = ReactDOM.findDOMNode(stub);
158+
var node = ReactDOM.render(stub, container);
117159

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

@@ -129,6 +171,29 @@ describe('ReactDOMInput', function() {
129171
expect(node.value).toEqual('foobar');
130172
});
131173

174+
it('should not incur unnecessary DOM mutations', function() {
175+
var container = document.createElement('div');
176+
ReactDOM.render(<input value="a" />, container);
177+
178+
var node = container.firstChild;
179+
var nodeValue = 'a';
180+
var nodeValueSetter = jest.genMockFn();
181+
Object.defineProperty(node, 'value', {
182+
get: function() {
183+
return nodeValue;
184+
},
185+
set: nodeValueSetter.mockImplementation(function(newValue) {
186+
nodeValue = newValue;
187+
}),
188+
});
189+
190+
ReactDOM.render(<input value="a" />, container);
191+
expect(nodeValueSetter.mock.calls.length).toBe(0);
192+
193+
ReactDOM.render(<input value="b"/>, container);
194+
expect(nodeValueSetter.mock.calls.length).toBe(1);
195+
});
196+
132197
it('should properly control a value of number `0`', function() {
133198
var stub = <input type="text" value={0} onChange={emptyFunction} />;
134199
stub = ReactTestUtils.renderIntoDocument(stub);
@@ -390,6 +455,13 @@ describe('ReactDOMInput', function() {
390455

391456
});
392457

458+
it('should update defaultValue to empty string', function() {
459+
var container = document.createElement('div');
460+
ReactDOM.render(<input type="text" defaultValue={'foo'} />, container);
461+
ReactDOM.render(<input type="text" defaultValue={''} />, container);
462+
expect(container.firstChild.defaultValue).toBe('');
463+
});
464+
393465
it('should throw if both checkedLink and valueLink are provided', function() {
394466
var node = document.createElement('div');
395467
var link = new ReactLink(true, jest.fn());
@@ -609,6 +681,11 @@ describe('ReactDOMInput', function() {
609681
'set data-reactroot',
610682
'set type',
611683
'set value',
684+
'set value',
685+
'set name',
686+
'set checked',
687+
'set checked',
688+
'set name',
612689
]);
613690
});
614691

0 commit comments

Comments
 (0)