Skip to content

Commit 3cc5e30

Browse files
hayesgmjim
hayesgm
authored and
jim
committed
Have defaultValue reach DOM node for html input box for facebook#4618 
1 parent 516c1d8 commit 3cc5e30

File tree

6 files changed

+247
-136
lines changed

6 files changed

+247
-136
lines changed

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

+9-8
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ var ReactDOMInput = {
7575
}, props, {
7676
defaultChecked: undefined,
7777
defaultValue: undefined,
78-
value: value != null ? value : inst._wrapperState.initialValue,
78+
value: value != null ? value : props.defaultValue,
7979
checked: checked != null ? checked : inst._wrapperState.initialChecked,
8080
onChange: inst._wrapperState.onChange,
8181
});
@@ -146,10 +146,8 @@ var ReactDOMInput = {
146146
warnIfValueIsNull(props);
147147
}
148148

149-
var defaultValue = props.defaultValue;
150149
inst._wrapperState = {
151150
initialChecked: props.defaultChecked || false,
152-
initialValue: defaultValue != null ? defaultValue : null,
153151
listeners: null,
154152
onChange: _handleChange.bind(inst),
155153
};
@@ -215,13 +213,16 @@ var ReactDOMInput = {
215213

216214
var value = LinkedValueUtils.getValue(props);
217215
if (value != null) {
216+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
217+
218218
// Cast `value` to a string to ensure the value is set correctly. While
219219
// browsers typically do this as necessary, jsdom doesn't.
220-
DOMPropertyOperations.setValueForProperty(
221-
ReactDOMComponentTree.getNodeFromInstance(inst),
222-
'value',
223-
'' + value
224-
);
220+
var newValue = '' + value;
221+
222+
// To avoid side effects (such as losing text selection), only set value if changed
223+
if (newValue !== node.value) {
224+
node.value = newValue;
225+
}
225226
}
226227
},
227228
};

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

+46-44
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
'use strict';
1313

14-
var DOMPropertyOperations = require('DOMPropertyOperations');
1514
var LinkedValueUtils = require('LinkedValueUtils');
1615
var ReactDOMComponentTree = require('ReactDOMComponentTree');
1716
var ReactUpdates = require('ReactUpdates');
@@ -65,12 +64,46 @@ var ReactDOMTextarea = {
6564
'`dangerouslySetInnerHTML` does not make sense on <textarea>.'
6665
);
6766

68-
// Always set children to the same thing. In IE9, the selection range will
69-
// get reset if `textContent` is mutated.
67+
var value = LinkedValueUtils.getValue(props);
68+
69+
// only bother fetching default value if we're going to use it
70+
if (value == null) {
71+
var defaultValue = props.defaultValue;
72+
// TODO (yungsters): Remove support for children content in <textarea>.
73+
var children = props.children;
74+
if (children != null) {
75+
if (__DEV__) {
76+
warning(
77+
false,
78+
'Use the `defaultValue` or `value` props instead of setting ' +
79+
'children on <textarea>.'
80+
);
81+
}
82+
invariant(
83+
defaultValue == null,
84+
'If you supply `defaultValue` on a <textarea>, do not pass children.'
85+
);
86+
if (Array.isArray(children)) {
87+
invariant(
88+
children.length <= 1,
89+
'<textarea> can only have at most one child.'
90+
);
91+
children = children[0];
92+
}
93+
94+
defaultValue = '' + children;
95+
}
96+
if (defaultValue == null) {
97+
defaultValue = '';
98+
}
99+
}
100+
101+
// The value can be a boolean or object so that's why it's
102+
// forced to be a string.
70103
var nativeProps = Object.assign({}, props, {
71-
defaultValue: undefined,
104+
defaultValue: '' + (value != null ? value : defaultValue),
72105
value: undefined,
73-
children: inst._wrapperState.initialValue,
106+
children: undefined,
74107
onChange: inst._wrapperState.onChange,
75108
});
76109

@@ -109,41 +142,7 @@ var ReactDOMTextarea = {
109142
warnIfValueIsNull(props);
110143
}
111144

112-
var defaultValue = props.defaultValue;
113-
// TODO (yungsters): Remove support for children content in <textarea>.
114-
var children = props.children;
115-
if (children != null) {
116-
if (__DEV__) {
117-
warning(
118-
false,
119-
'Use the `defaultValue` or `value` props instead of setting ' +
120-
'children on <textarea>.'
121-
);
122-
}
123-
invariant(
124-
defaultValue == null,
125-
'If you supply `defaultValue` on a <textarea>, do not pass children.'
126-
);
127-
if (Array.isArray(children)) {
128-
invariant(
129-
children.length <= 1,
130-
'<textarea> can only have at most one child.'
131-
);
132-
children = children[0];
133-
}
134-
135-
defaultValue = '' + children;
136-
}
137-
if (defaultValue == null) {
138-
defaultValue = '';
139-
}
140-
var value = LinkedValueUtils.getValue(props);
141145
inst._wrapperState = {
142-
// We save the initial value so that `ReactDOMComponent` doesn't update
143-
// `textContent` (unnecessary since we update value).
144-
// The initial value can be a boolean or object so that's why it's
145-
// forced to be a string.
146-
initialValue: '' + (value != null ? value : defaultValue),
147146
listeners: null,
148147
onChange: _handleChange.bind(inst),
149148
};
@@ -158,13 +157,16 @@ var ReactDOMTextarea = {
158157

159158
var value = LinkedValueUtils.getValue(props);
160159
if (value != null) {
160+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
161+
161162
// Cast `value` to a string to ensure the value is set correctly. While
162163
// browsers typically do this as necessary, jsdom doesn't.
163-
DOMPropertyOperations.setValueForProperty(
164-
ReactDOMComponentTree.getNodeFromInstance(inst),
165-
'value',
166-
'' + value
167-
);
164+
var newValue = '' + value;
165+
166+
// To avoid side effects (such as losing text selection), only set value if changed
167+
if (newValue !== node.value) {
168+
node.value = newValue;
169+
}
168170
}
169171
},
170172
};

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

+51-6
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ describe('ReactDOMInput', function() {
3838
stub = ReactTestUtils.renderIntoDocument(stub);
3939
var node = ReactDOM.findDOMNode(stub);
4040

41+
expect(node.getAttribute('value')).toBe('0');
4142
expect(node.value).toBe('0');
4243
});
4344

@@ -57,6 +58,30 @@ describe('ReactDOMInput', function() {
5758
expect(node.value).toBe('false');
5859
});
5960

61+
it('should update `defaultValue` for uncontrolled input', function() {
62+
var container = document.createElement('div');
63+
64+
var node = ReactDOM.render(<input type="text" defaultValue="0" />, container);
65+
66+
expect(node.value).toBe('0');
67+
68+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
69+
70+
expect(node.value).toBe('1');
71+
});
72+
73+
it('should take `defaultValue` when changing to uncontrolled input', function() {
74+
var container = document.createElement('div');
75+
76+
var node = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);
77+
78+
expect(node.value).toBe('0');
79+
80+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
81+
82+
expect(node.value).toBe('1');
83+
});
84+
6085
it('should display "foobar" for `defaultValue` of `objToString`', function() {
6186
var objToString = {
6287
toString: function() {
@@ -82,8 +107,7 @@ describe('ReactDOMInput', function() {
82107
it('should allow setting `value` to `true`', function() {
83108
var container = document.createElement('div');
84109
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
85-
stub = ReactDOM.render(stub, container);
86-
var node = ReactDOM.findDOMNode(stub);
110+
var node = ReactDOM.render(stub, container);
87111

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

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

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

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

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

@@ -129,6 +151,29 @@ describe('ReactDOMInput', function() {
129151
expect(node.value).toEqual('foobar');
130152
});
131153

154+
it('should not incur unnecessary DOM mutations', function() {
155+
var container = document.createElement('div');
156+
ReactDOM.render(<input value="a" />, container);
157+
158+
var node = container.firstChild;
159+
var nodeValue = 'a'; // node.value always returns undefined
160+
var nodeValueSetter = jest.genMockFn();
161+
Object.defineProperty(node, 'value', {
162+
get: function() {
163+
return nodeValue;
164+
},
165+
set: nodeValueSetter.mockImplementation(function(newValue) {
166+
nodeValue = newValue;
167+
}),
168+
});
169+
170+
ReactDOM.render(<input value="a" />, container);
171+
expect(nodeValueSetter.mock.calls.length).toBe(0);
172+
173+
ReactDOM.render(<input value="b"/>, container);
174+
expect(nodeValueSetter.mock.calls.length).toBe(1);
175+
});
176+
132177
it('should properly control a value of number `0`', function() {
133178
var stub = <input type="text" value={0} onChange={emptyFunction} />;
134179
stub = ReactTestUtils.renderIntoDocument(stub);

0 commit comments

Comments
 (0)