Skip to content

Commit acff76c

Browse files
hayesgmhayesgm
committed
Have defaultValue reach DOM node for html input box for #4618 
1 parent 55bd203 commit acff76c

File tree

6 files changed

+151
-28
lines changed

6 files changed

+151
-28
lines changed

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

+9-10
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ var ReactDOMInput = {
7070
var nativeProps = assign({}, props, {
7171
defaultChecked: undefined,
7272
defaultValue: undefined,
73-
value: value != null ? value : inst._wrapperState.initialValue,
73+
value: value != null ? value : props.defaultValue,
7474
checked: checked != null ? checked : inst._wrapperState.initialChecked,
7575
onChange: inst._wrapperState.onChange,
7676
});
@@ -103,10 +103,8 @@ var ReactDOMInput = {
103103
warnIfValueIsNull(props);
104104
}
105105

106-
var defaultValue = props.defaultValue;
107106
inst._wrapperState = {
108107
initialChecked: props.defaultChecked || false,
109-
initialValue: defaultValue != null ? defaultValue : null,
110108
listeners: null,
111109
onChange: _handleChange.bind(inst),
112110
};
@@ -140,13 +138,14 @@ var ReactDOMInput = {
140138

141139
var value = LinkedValueUtils.getValue(props);
142140
if (value != null) {
143-
// Cast `value` to a string to ensure the value is set correctly. While
144-
// browsers typically do this as necessary, jsdom doesn't.
145-
DOMPropertyOperations.setValueForProperty(
146-
ReactDOMComponentTree.getNodeFromInstance(inst),
147-
'value',
148-
'' + value
149-
);
141+
var node = ReactDOMComponentTree.getNodeFromInstance(inst)
142+
143+
// To avoid side effects (such as losing text selection), only set value if changed
144+
if (value != node.value) {
145+
// Cast `value` to a string to ensure the value is set correctly. While
146+
// browsers typically do this as necessary, jsdom doesn't.
147+
node.value = '' + value;
148+
}
150149
}
151150
},
152151
};

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,14 @@ var ReactDOMTextarea = {
143143

144144
var value = LinkedValueUtils.getValue(props);
145145
if (value != null) {
146-
// Cast `value` to a string to ensure the value is set correctly. While
147-
// browsers typically do this as necessary, jsdom doesn't.
148-
DOMPropertyOperations.setValueForProperty(
149-
ReactDOMComponentTree.getNodeFromInstance(inst),
150-
'value',
151-
'' + value
152-
);
146+
var node = ReactDOMComponentTree.getNodeFromInstance(inst)
147+
148+
// To avoid side effects (such as losing text selection), only set value if changed
149+
if (value != node.value) {
150+
// Cast `value` to a string to ensure the value is set correctly. While
151+
// browsers typically do this as necessary, jsdom doesn't.
152+
node.value = '' + value;
153+
}
153154
}
154155
},
155156
};

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

+86
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('ReactDOMInput', function() {
3636
stub = ReactTestUtils.renderIntoDocument(stub);
3737
var node = ReactDOM.findDOMNode(stub);
3838

39+
expect(node.getAttribute('value')).toBe('0');
3940
expect(node.value).toBe('0');
4041
});
4142

@@ -55,6 +56,68 @@ describe('ReactDOMInput', function() {
5556
expect(node.value).toBe('false');
5657
});
5758

59+
it('should update `defaultValue` for uncontrolled input', function() {
60+
var container = document.createElement('div');
61+
62+
var el = ReactDOM.render(<input type="text" defaultValue="0" />, container);
63+
var node = ReactDOM.findDOMNode(el);
64+
65+
expect(node.value).toBe('0');
66+
67+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
68+
69+
expect(node.value).toBe('1');
70+
});
71+
72+
it('should take `defaultValue` for a controlled input', function() {
73+
var container = document.createElement('div');
74+
75+
var el = ReactDOM.render(<input type="text" value={null} defaultValue="0" />, container);
76+
var node = ReactDOM.findDOMNode(el);
77+
78+
expect(node.value).toBe('0');
79+
80+
ReactDOM.render(<input type="text" value={null} defaultValue="1" />, container);
81+
82+
expect(node.value).toBe('1');
83+
84+
ReactDOM.render(<input type="text" value={3} defaultValue="2" />, container);
85+
86+
expect(node.value).toBe('3');
87+
88+
ReactDOM.render(<input type="text" value={5} defaultValue="4" />, container);
89+
90+
expect(node.value).toBe('5');
91+
expect(node.getAttribute('value')).toBe('5');
92+
expect(node.defaultValue).toBe('5');
93+
});
94+
95+
it('should update `value` when changing to controlled input', function() {
96+
var container = document.createElement('div');
97+
98+
var el = ReactDOM.render(<input type="text" defaultValue="0" />, container);
99+
var node = ReactDOM.findDOMNode(el);
100+
101+
expect(node.value).toBe('0');
102+
103+
ReactDOM.render(<input type="text" value="1" defaultValue="0" />, container);
104+
105+
expect(node.value).toBe('1');
106+
});
107+
108+
it('should take `defaultValue` when changing to uncontrolled input', function() {
109+
var container = document.createElement('div');
110+
111+
var el = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);
112+
var node = ReactDOM.findDOMNode(el);
113+
114+
expect(node.value).toBe('0');
115+
116+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
117+
118+
expect(node.value).toBe('1');
119+
});
120+
58121
it('should display "foobar" for `defaultValue` of `objToString`', function() {
59122
var objToString = {
60123
toString: function() {
@@ -127,6 +190,29 @@ describe('ReactDOMInput', function() {
127190
expect(node.value).toEqual('foobar');
128191
});
129192

193+
it('should not incur unnecessary DOM mutations', function() {
194+
var container = document.createElement('div');
195+
ReactDOM.render(<input value="a" />, container);
196+
197+
var node = container.firstChild;
198+
var nodeValue = 'a'; // node.value always returns undefined
199+
var nodeValueSetter = jest.genMockFn();
200+
Object.defineProperty(node, 'value', {
201+
get: function() {
202+
return nodeValue;
203+
},
204+
set: nodeValueSetter.mockImplementation(function(newValue) {
205+
nodeValue = newValue;
206+
}),
207+
});
208+
209+
ReactDOM.render(<input value="a" />, container);
210+
expect(nodeValueSetter.mock.calls.length).toBe(0);
211+
212+
ReactDOM.render(<input value="b"/>, container);
213+
expect(nodeValueSetter.mock.calls.length).toBe(1);
214+
});
215+
130216
it('should properly control a value of number `0`', function() {
131217
var stub = <input type="text" value={0} onChange={emptyFunction} />;
132218
stub = ReactTestUtils.renderIntoDocument(stub);

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

+37
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,42 @@ describe('ReactDOMTextarea', function() {
163163
expect(node.value).toEqual('foo');
164164
});
165165

166+
it('should not take updates to `defaultValue` for uncontrolled textarea', function() {
167+
var container = document.createElement('div');
168+
169+
var el = ReactDOM.render(<textarea type="text" defaultValue="0" />, container);
170+
var node = ReactDOM.findDOMNode(el);
171+
172+
expect(node.value).toBe('0');
173+
174+
ReactDOM.render(<textarea type="text" defaultValue="1" />, container);
175+
176+
expect(node.value).toBe('0');
177+
});
178+
179+
it('should not incur unnecessary DOM mutations', function() {
180+
var container = document.createElement('div');
181+
ReactDOM.render(<textarea value="a" onChange={emptyFunction} />, container);
182+
183+
var node = container.firstChild;
184+
var nodeValue = 'a'; // node.value always returns undefined
185+
var nodeValueSetter = jest.genMockFn();
186+
Object.defineProperty(node, 'value', {
187+
get: function() {
188+
return nodeValue;
189+
},
190+
set: nodeValueSetter.mockImplementation(function(newValue) {
191+
nodeValue = newValue;
192+
}),
193+
});
194+
195+
ReactDOM.render(<textarea value="a" onChange={emptyFunction} />, container);
196+
expect(nodeValueSetter.mock.calls.length).toBe(0);
197+
198+
ReactDOM.render(<textarea value="b" onChange={emptyFunction} />, container);
199+
expect(nodeValueSetter.mock.calls.length).toBe(1);
200+
});
201+
166202
it('should properly control a value of number `0`', function() {
167203
var stub = <textarea value={0} onChange={emptyFunction} />;
168204
stub = renderTextarea(stub);
@@ -278,4 +314,5 @@ describe('ReactDOMTextarea', function() {
278314
ReactTestUtils.renderIntoDocument(<textarea value={null} />);
279315
expect(console.error.argsForCall.length).toBe(1);
280316
});
317+
281318
});

‎src/renderers/dom/shared/HTMLDOMPropertyConfig.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ var HTMLDOMPropertyConfig = {
169169
// Setting .type throws on non-<input> tags
170170
type: MUST_USE_ATTRIBUTE,
171171
useMap: null,
172-
value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS,
172+
value: MUST_USE_ATTRIBUTE,
173173
width: MUST_USE_ATTRIBUTE,
174174
wmode: MUST_USE_ATTRIBUTE,
175175
wrap: MUST_USE_ATTRIBUTE,

‎src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

+10-10
Original file line numberDiff line numberDiff line change
@@ -413,25 +413,25 @@ describe('ReactDOMComponent', function() {
413413

414414
it('should not incur unnecessary DOM mutations', function() {
415415
var container = document.createElement('div');
416-
ReactDOM.render(<div value="" />, container);
416+
ReactDOM.render(<div id="" />, container);
417417

418418
var node = container.firstChild;
419-
var nodeValue = ''; // node.value always returns undefined
420-
var nodeValueSetter = jest.genMockFn();
421-
Object.defineProperty(node, 'value', {
419+
var nodeId = ''; // node.value always returns undefined
420+
var nodeIdSetter = jest.genMockFn();
421+
Object.defineProperty(node, 'id', {
422422
get: function() {
423-
return nodeValue;
423+
return nodeId;
424424
},
425-
set: nodeValueSetter.mockImplementation(function(newValue) {
426-
nodeValue = newValue;
425+
set: nodeIdSetter.mockImplementation(function(newValue) {
426+
nodeId = newValue;
427427
}),
428428
});
429429

430-
ReactDOM.render(<div value="" />, container);
431-
expect(nodeValueSetter.mock.calls.length).toBe(0);
430+
ReactDOM.render(<div id="" />, container);
431+
expect(nodeIdSetter.mock.calls.length).toBe(0);
432432

433433
ReactDOM.render(<div />, container);
434-
expect(nodeValueSetter.mock.calls.length).toBe(1);
434+
expect(nodeIdSetter.mock.calls.length).toBe(1);
435435
});
436436

437437
it('should ignore attribute whitelist for elements with the "is: attribute', function() {

0 commit comments

Comments
 (0)