Skip to content

Commit d7ca7f6

Browse files
hayesgmjim
hayesgm
authored and
jim
committed
Have defaultValue reach DOM node for html input box for #4618 
1 parent f7db143 commit d7ca7f6

File tree

6 files changed

+232
-114
lines changed

6 files changed

+232
-114
lines changed

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ var ReactDOMInput = {
7676
}, props, {
7777
defaultChecked: undefined,
7878
defaultValue: undefined,
79-
value: value != null ? value : inst._wrapperState.initialValue,
79+
value: value != null ? value : props.defaultValue,
8080
checked: checked != null ? checked : inst._wrapperState.initialChecked,
8181
onChange: inst._wrapperState.onChange,
8282
});
@@ -139,10 +139,8 @@ var ReactDOMInput = {
139139
warnIfValueIsNull(props);
140140
}
141141

142-
var defaultValue = props.defaultValue;
143142
inst._wrapperState = {
144143
initialChecked: props.defaultChecked || false,
145-
initialValue: defaultValue != null ? defaultValue : null,
146144
listeners: null,
147145
onChange: _handleChange.bind(inst),
148146
};
@@ -208,13 +206,16 @@ var ReactDOMInput = {
208206

209207
var value = LinkedValueUtils.getValue(props);
210208
if (value != null) {
209+
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
210+
211211
// Cast `value` to a string to ensure the value is set correctly. While
212212
// browsers typically do this as necessary, jsdom doesn't.
213-
DOMPropertyOperations.setValueForProperty(
214-
ReactDOMComponentTree.getNodeFromInstance(inst),
215-
'value',
216-
'' + value
217-
);
213+
var newValue = '' + value;
214+
215+
// To avoid side effects (such as losing text selection), only set value if changed
216+
if (newValue !== node.value) {
217+
node.value = newValue;
218+
}
218219
}
219220
},
220221
};

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

Lines changed: 46 additions & 44 deletions
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');
@@ -66,12 +65,46 @@ var ReactDOMTextarea = {
6665
'`dangerouslySetInnerHTML` does not make sense on <textarea>.'
6766
);
6867

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

@@ -110,41 +143,7 @@ var ReactDOMTextarea = {
110143
warnIfValueIsNull(props);
111144
}
112145

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)) {
129-
invariant(
130-
children.length <= 1,
131-
'<textarea> can only have at most one child.'
132-
);
133-
children = children[0];
134-
}
135-
136-
defaultValue = '' + children;
137-
}
138-
if (defaultValue == null) {
139-
defaultValue = '';
140-
}
141-
var value = LinkedValueUtils.getValue(props);
142146
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),
148147
listeners: null,
149148
onChange: _handleChange.bind(inst),
150149
};
@@ -159,13 +158,16 @@ var ReactDOMTextarea = {
159158

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

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

Lines changed: 51 additions & 6 deletions
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)