Skip to content

Commit

Permalink
Have defaultValue reach DOM node for html input box for facebook#4618
Browse files Browse the repository at this point in the history
  • Loading branch information
@hayesgm
hayesgm authored and jim committed May 16, 2016
1 parent 3703b63 commit 510cd85
Show file tree
Hide file tree
Showing 6 changed files with 249 additions and 126 deletions.
17 changes: 9 additions & 8 deletions src/renderers/dom/client/wrappers/ReactDOMInput.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ var ReactDOMInput = {
}, DisabledInputUtils.getHostProps(inst, props), {
defaultChecked: undefined,
defaultValue: undefined,
value: value != null ? value : inst._wrapperState.initialValue,
value: value != null ? value : props.defaultValue,
checked: checked != null ? checked : inst._wrapperState.initialChecked,
onChange: inst._wrapperState.onChange,
});
Expand Down Expand Up @@ -147,10 +147,8 @@ var ReactDOMInput = {
warnIfValueIsNull(props);
}

var defaultValue = props.defaultValue;
inst._wrapperState = {
initialChecked: props.defaultChecked || false,
initialValue: defaultValue != null ? defaultValue : null,
listeners: null,
onChange: _handleChange.bind(inst),
};
Expand Down Expand Up @@ -216,13 +214,16 @@ var ReactDOMInput = {

var value = LinkedValueUtils.getValue(props);
if (value != null) {
var node = ReactDOMComponentTree.getNodeFromInstance(inst);

// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
DOMPropertyOperations.setValueForProperty(
ReactDOMComponentTree.getNodeFromInstance(inst),
'value',
'' + value
);
var newValue = '' + value;

// To avoid side effects (such as losing text selection), only set value if changed
if (newValue !== node.value) {
node.value = newValue;
}
}
},
};
Expand Down
92 changes: 47 additions & 45 deletions src/renderers/dom/client/wrappers/ReactDOMTextarea.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
'use strict';

var DisabledInputUtils = require('DisabledInputUtils');
var DOMPropertyOperations = require('DOMPropertyOperations');
var LinkedValueUtils = require('LinkedValueUtils');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactUpdates = require('ReactUpdates');
Expand Down Expand Up @@ -66,12 +65,46 @@ var ReactDOMTextarea = {
'`dangerouslySetInnerHTML` does not make sense on <textarea>.'
);

// Always set children to the same thing. In IE9, the selection range will
// get reset if `textContent` is mutated.
var hostProps = Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
defaultValue: undefined,
var value = LinkedValueUtils.getValue(props);

// only bother fetching default value if we're going to use it
if (value == null) {
var defaultValue = props.defaultValue;
// TODO (yungsters): Remove support for children content in <textarea>.
var children = props.children;
if (children != null) {
if (__DEV__) {
warning(
false,
'Use the `defaultValue` or `value` props instead of setting ' +
'children on <textarea>.'
);
}
invariant(
defaultValue == null,
'If you supply `defaultValue` on a <textarea>, do not pass children.'
);
if (Array.isArray(children)) {
invariant(
children.length <= 1,
'<textarea> can only have at most one child.'
);
children = children[0];
}

defaultValue = '' + children;
}
if (defaultValue == null) {
defaultValue = '';
}
}

// The value can be a boolean or object so that's why it's
// forced to be a string.
var nativeProps = Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
defaultValue: '' + (value != null ? value : defaultValue),
value: undefined,
children: inst._wrapperState.initialValue,
children: undefined,
onChange: inst._wrapperState.onChange,
});

Expand Down Expand Up @@ -110,41 +143,7 @@ var ReactDOMTextarea = {
warnIfValueIsNull(props);
}

var defaultValue = props.defaultValue;
// TODO (yungsters): Remove support for children content in <textarea>.
var children = props.children;
if (children != null) {
if (__DEV__) {
warning(
false,
'Use the `defaultValue` or `value` props instead of setting ' +
'children on <textarea>.'
);
}
invariant(
defaultValue == null,
'If you supply `defaultValue` on a <textarea>, do not pass children.'
);
if (Array.isArray(children)) {
invariant(
children.length <= 1,
'<textarea> can only have at most one child.'
);
children = children[0];
}

defaultValue = '' + children;
}
if (defaultValue == null) {
defaultValue = '';
}
var value = LinkedValueUtils.getValue(props);
inst._wrapperState = {
// We save the initial value so that `ReactDOMComponent` doesn't update
// `textContent` (unnecessary since we update value).
// The initial value can be a boolean or object so that's why it's
// forced to be a string.
initialValue: '' + (value != null ? value : defaultValue),
listeners: null,
onChange: _handleChange.bind(inst),
};
Expand All @@ -159,13 +158,16 @@ var ReactDOMTextarea = {

var value = LinkedValueUtils.getValue(props);
if (value != null) {
var node = ReactDOMComponentTree.getNodeFromInstance(inst);

// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
DOMPropertyOperations.setValueForProperty(
ReactDOMComponentTree.getNodeFromInstance(inst),
'value',
'' + value
);
var newValue = '' + value;

// To avoid side effects (such as losing text selection), only set value if changed
if (newValue !== node.value) {
node.value = newValue;
}
}
},
};
Expand Down
57 changes: 51 additions & 6 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('ReactDOMInput', function() {
stub = ReactTestUtils.renderIntoDocument(stub);
var node = ReactDOM.findDOMNode(stub);

expect(node.getAttribute('value')).toBe('0');
expect(node.value).toBe('0');
});

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

it('should update `defaultValue` for uncontrolled input', function() {
var container = document.createElement('div');

var node = ReactDOM.render(<input type="text" defaultValue="0" />, container);

expect(node.value).toBe('0');

ReactDOM.render(<input type="text" defaultValue="1" />, container);

expect(node.value).toBe('1');
});

it('should take `defaultValue` when changing to uncontrolled input', function() {
var container = document.createElement('div');

var node = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);

expect(node.value).toBe('0');

ReactDOM.render(<input type="text" defaultValue="1" />, container);

expect(node.value).toBe('1');
});

it('should display "foobar" for `defaultValue` of `objToString`', function() {
var objToString = {
toString: function() {
Expand All @@ -91,8 +116,7 @@ describe('ReactDOMInput', function() {
it('should allow setting `value` to `true`', function() {
var container = document.createElement('div');
var stub = <input type="text" value="yolo" onChange={emptyFunction} />;
stub = ReactDOM.render(stub, container);
var node = ReactDOM.findDOMNode(stub);
var node = ReactDOM.render(stub, container);

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

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

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

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

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

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

it('should not incur unnecessary DOM mutations', function() {
var container = document.createElement('div');
ReactDOM.render(<input value="a" />, container);

var node = container.firstChild;
var nodeValue = 'a'; // node.value always returns undefined
var nodeValueSetter = jest.genMockFn();
Object.defineProperty(node, 'value', {
get: function() {
return nodeValue;
},
set: nodeValueSetter.mockImplementation(function(newValue) {
nodeValue = newValue;
}),
});

ReactDOM.render(<input value="a" />, container);
expect(nodeValueSetter.mock.calls.length).toBe(0);

ReactDOM.render(<input value="b"/>, container);
expect(nodeValueSetter.mock.calls.length).toBe(1);
});

it('should properly control a value of number `0`', function() {
var stub = <input type="text" value={0} onChange={emptyFunction} />;
stub = ReactTestUtils.renderIntoDocument(stub);
Expand Down
Loading

0 comments on commit 510cd85

Please sign in to comment.