Skip to content

Commit 86b8bac

Browse files
hayesgmjim
hayesgm
authored and
jim
committed
Have defaultValue reach DOM node for html input box for facebook#4618 
1 parent 44f8463 commit 86b8bac

File tree

6 files changed

+248
-125
lines changed

6 files changed

+248
-125
lines changed

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

+9-8
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ var ReactDOMInput = {
7676
}, DisabledInputUtils.getNativeProps(inst, 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
});
@@ -147,10 +147,8 @@ var ReactDOMInput = {
147147
warnIfValueIsNull(props);
148148
}
149149

150-
var defaultValue = props.defaultValue;
151150
inst._wrapperState = {
152151
initialChecked: props.defaultChecked || false,
153-
initialValue: defaultValue != null ? defaultValue : null,
154152
listeners: null,
155153
onChange: _handleChange.bind(inst),
156154
};
@@ -216,13 +214,16 @@ var ReactDOMInput = {
216214

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

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

+46-44
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');
@@ -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 = Object.assign({}, DisabledInputUtils.getNativeProps(inst, 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

+51-6
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('ReactDOMInput', function() {
4747
stub = ReactTestUtils.renderIntoDocument(stub);
4848
var node = ReactDOM.findDOMNode(stub);
4949

50+
expect(node.getAttribute('value')).toBe('0');
5051
expect(node.value).toBe('0');
5152
});
5253

@@ -66,6 +67,30 @@ describe('ReactDOMInput', function() {
6667
expect(node.value).toBe('false');
6768
});
6869

70+
it('should update `defaultValue` for uncontrolled input', function() {
71+
var container = document.createElement('div');
72+
73+
var node = ReactDOM.render(<input type="text" defaultValue="0" />, container);
74+
75+
expect(node.value).toBe('0');
76+
77+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
78+
79+
expect(node.value).toBe('1');
80+
});
81+
82+
it('should take `defaultValue` when changing to uncontrolled input', function() {
83+
var container = document.createElement('div');
84+
85+
var node = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);
86+
87+
expect(node.value).toBe('0');
88+
89+
ReactDOM.render(<input type="text" defaultValue="1" />, container);
90+
91+
expect(node.value).toBe('1');
92+
});
93+
6994
it('should display "foobar" for `defaultValue` of `objToString`', function() {
7095
var objToString = {
7196
toString: function() {
@@ -91,8 +116,7 @@ describe('ReactDOMInput', function() {
91116
it('should allow setting `value` to `true`', function() {
92117
var container = document.createElement('div');
93118
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
94-
stub = ReactDOM.render(stub, container);
95-
var node = ReactDOM.findDOMNode(stub);
119+
var node = ReactDOM.render(stub, container);
96120

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

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

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

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

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

@@ -138,6 +160,29 @@ describe('ReactDOMInput', function() {
138160
expect(node.value).toEqual('foobar');
139161
});
140162

163+
it('should not incur unnecessary DOM mutations', function() {
164+
var container = document.createElement('div');
165+
ReactDOM.render(<input value="a" />, container);
166+
167+
var node = container.firstChild;
168+
var nodeValue = 'a'; // node.value always returns undefined
169+
var nodeValueSetter = jest.genMockFn();
170+
Object.defineProperty(node, 'value', {
171+
get: function() {
172+
return nodeValue;
173+
},
174+
set: nodeValueSetter.mockImplementation(function(newValue) {
175+
nodeValue = newValue;
176+
}),
177+
});
178+
179+
ReactDOM.render(<input value="a" />, container);
180+
expect(nodeValueSetter.mock.calls.length).toBe(0);
181+
182+
ReactDOM.render(<input value="b"/>, container);
183+
expect(nodeValueSetter.mock.calls.length).toBe(1);
184+
});
185+
141186
it('should properly control a value of number `0`', function() {
142187
var stub = <input type="text" value={0} onChange={emptyFunction} />;
143188
stub = ReactTestUtils.renderIntoDocument(stub);

0 commit comments

Comments
 (0)