Skip to content

Commit

Permalink
Merge pull request #5823 from mgmcdermott/master
Browse files Browse the repository at this point in the history
Warn when value and defaultValue props are both specified on input or textarea.
  • Loading branch information
jimfb committed Jan 11, 2016
2 parents f7850dd + f2b62e9 commit 171305f
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 0 deletions.
24 changes: 24 additions & 0 deletions src/renderers/dom/client/wrappers/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ var instancesByReactID = {};
var didWarnValueLink = false;
var didWarnCheckedLink = false;
var didWarnValueNull = false;
var didWarnValueDefaultValue = false;
var didWarnCheckedDefaultChecked = false;

function forceUpdateIfMounted() {
if (this._rootNodeID) {
Expand Down Expand Up @@ -104,6 +106,28 @@ var ReactDOMInput = {
);
didWarnCheckedLink = true;
}
if (props.checked !== undefined && props.defaultChecked !== undefined &&
!didWarnCheckedDefaultChecked) {
warning(
false,
'Input elements must be either controlled or uncontrolled (specify either the ' +
'checked prop, or the defaultChecked prop, but not both). Decide between using a ' +
'controlled or uncontrolled input and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);
didWarnCheckedDefaultChecked = true;
}
if (props.value !== undefined && props.defaultValue !== undefined &&
!didWarnValueDefaultValue) {
warning(
false,
'Input elements must be either controlled or uncontrolled (specify either the value ' +
'prop, or the defaultValue prop, but not both). Decide between using a controlled ' +
'or uncontrolled input and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);
didWarnValueDefaultValue = true;
}
warnIfValueIsNull(props);
}

Expand Down
13 changes: 13 additions & 0 deletions src/renderers/dom/client/wrappers/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var warning = require('warning');

var didWarnValueLink = false;
var didWarnValueNull = false;
var didWarnValueDefaultValue = false;

function updateOptionsIfPendingUpdateAndMounted() {
if (this._rootNodeID && this._wrapperState.pendingUpdate) {
Expand Down Expand Up @@ -178,6 +179,18 @@ var ReactDOMSelect = {
onChange: _handleChange.bind(inst),
wasMultiple: Boolean(props.multiple),
};

if (props.value !== undefined && props.defaultValue !== undefined &&
!didWarnValueDefaultValue) {
warning(
false,
'Select elements must be either controlled or uncontrolled (specify either the ' +
'value prop, or the defaultValue prop, but not both). Decide between using a ' +
'controlled or uncontrolled select element and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);
didWarnValueDefaultValue = true;
}
},

getSelectValueContext: function(inst) {
Expand Down
11 changes: 11 additions & 0 deletions src/renderers/dom/client/wrappers/ReactDOMTextarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var warning = require('warning');

var didWarnValueLink = false;
var didWarnValueNull = false;
var didWarnValDefaultVal = false;

function forceUpdateIfMounted() {
if (this._rootNodeID) {
Expand Down Expand Up @@ -91,6 +92,16 @@ var ReactDOMTextarea = {
);
didWarnValueLink = true;
}
if (props.value !== undefined && props.defaultValue !== undefined && !didWarnValDefaultVal) {
warning(
false,
'Textarea elements must be either controlled or uncontrolled (specify either the value ' +
'prop, or the defaultValue prop, but not both). Decide between using a controlled or ' +
'uncontrolled input and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);
didWarnValDefaultVal = true;
}
warnIfValueIsNull(props);
}

Expand Down
34 changes: 34 additions & 0 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,40 @@ describe('ReactDOMInput', function() {
expect(console.error.argsForCall.length).toBe(1);
});

it('should throw warning message if checked and defaultChecked props are specified', function() {
ReactTestUtils.renderIntoDocument(
<input type="radio" checked={true} defaultChecked={true} readOnly={true} />
);
expect(console.error.argsForCall[0][0]).toContain(
'Input elements must be either controlled or uncontrolled (specify either the ' +
'checked prop, or the defaultChecked prop, but not both). Decide between using a ' +
'controlled or uncontrolled input and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);

ReactTestUtils.renderIntoDocument(
<input type="radio" checked={true} defaultChecked={true} readOnly={true} />
);
expect(console.error.argsForCall.length).toBe(1);
});

it('should throw warning message if value and defaultValue props are specified', function() {
ReactTestUtils.renderIntoDocument(
<input type="text" value="foo" defaultValue="bar" readOnly={true} />
);
expect(console.error.argsForCall[0][0]).toContain(
'Input elements must be either controlled or uncontrolled (specify either the value ' +
'prop, or the defaultValue prop, but not both). Decide between using a controlled or ' +
'uncontrolled input and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);

ReactTestUtils.renderIntoDocument(
<input type="text" value="foo" defaultValue="bar" readOnly={true} />
);
expect(console.error.argsForCall.length).toBe(1);
});

it('sets type before value always', function() {
var log = [];
var originalCreateElement = document.createElement;
Expand Down
26 changes: 26 additions & 0 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMSelect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -490,4 +490,30 @@ describe('ReactDOMSelect', function() {

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

it('should throw warning message if value and defaultValue props are specified', function() {
spyOn(console, 'error');
ReactTestUtils.renderIntoDocument(
<select value="giraffe" defaultValue="giraffe" readOnly={true}>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
<option value="gorilla">A gorilla!</option>
</select>
);
expect(console.error.argsForCall[0][0]).toContain(
'Select elements must be either controlled or uncontrolled (specify either the ' +
'value prop, or the defaultValue prop, but not both). Decide between using a ' +
'controlled or uncontrolled select element and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);

ReactTestUtils.renderIntoDocument(
<select value="giraffe" defaultValue="giraffe" readOnly={true}>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
<option value="gorilla">A gorilla!</option>
</select>
);
expect(console.error.argsForCall.length).toBe(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -278,4 +278,22 @@ describe('ReactDOMTextarea', function() {
ReactTestUtils.renderIntoDocument(<textarea value={null} />);
expect(console.error.argsForCall.length).toBe(1);
});

it('should throw warning message if value and defaultValue are specified', function() {
spyOn(console, 'error');
ReactTestUtils.renderIntoDocument(
<textarea value="foo" defaultValue="bar" readOnly={true} />
);
expect(console.error.argsForCall[0][0]).toContain(
'Textarea elements must be either controlled or uncontrolled (specify either the value ' +
'prop, or the defaultValue prop, but not both). Decide between using a controlled or ' +
'uncontrolled input and remove one of these props. More info: ' +
'https://fb.me/react-controlled-components'
);

ReactTestUtils.renderIntoDocument(
<textarea value="foo" defaultValue="bar" readOnly={true} />
);
expect(console.error.argsForCall.length).toBe(1);
});
});

0 comments on commit 171305f

Please sign in to comment.