Skip to content

Commit

Permalink
Merge pull request #5864 from TheBlasfem/master
Browse files Browse the repository at this point in the history 
Warn when an input switches between controlled and uncontrolled
  • Loading branch information
@jimfb
jimfb committed Feb 13, 2016
2 parents b3eaab9 + b38b39a commit d684b15
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 0 deletions.
43 changes: 43 additions & 0 deletions src/renderers/dom/client/wrappers/ReactDOMInput.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ var didWarnCheckedLink = false;
var didWarnValueNull = false;
var didWarnValueDefaultValue = false;
var didWarnCheckedDefaultChecked = false;
var didWarnControlledToUncontrolled = false;
var didWarnUncontrolledToControlled = false;

function forceUpdateIfMounted() {
if (this._rootNodeID) {
Expand Down Expand Up @@ -144,13 +146,54 @@ var ReactDOMInput = {
listeners: null,
onChange: _handleChange.bind(inst),
};

if (__DEV__) {
inst._wrapperState.controlled = props.checked !== undefined || props.value !== undefined;
}
},

updateWrapper: function(inst) {
var props = inst._currentElement.props;

if (__DEV__) {
warnIfValueIsNull(props);

var initialValue = inst._wrapperState.initialChecked || inst._wrapperState.initialValue;
var defaultValue = props.defaultChecked || props.defaultValue;
var controlled = props.checked !== undefined || props.value !== undefined;
var owner = inst._currentElement._owner;

if (
(initialValue || !inst._wrapperState.controlled) &&
controlled && !didWarnUncontrolledToControlled
) {
warning(
false,
'%s is changing a uncontrolled input of type %s to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components',
owner && owner.getName() || 'A component',
props.type
);
didWarnUncontrolledToControlled = true;
}
if (
inst._wrapperState.controlled &&
(defaultValue || !controlled) &&
!didWarnControlledToUncontrolled
) {
warning(
false,
'%s is changing a controlled input of type %s to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components',
owner && owner.getName() || 'A component',
props.type
);
didWarnControlledToUncontrolled = true;
}
}

// TODO: Shouldn't this be getChecked(props)?
Expand Down
126 changes: 126 additions & 0 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,132 @@ describe('ReactDOMInput', function() {
expect(console.error.argsForCall.length).toBe(1);
});

it('should warn if controlled input switches to uncontrolled', function() {
var stub = <input type="text" value="controlled" onChange={emptyFunction} />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="text" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a controlled input of type text to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if controlled input switches to uncontrolled with defaultValue', function() {
var stub = <input type="text" value="controlled" onChange={emptyFunction} />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="text" defaultValue="uncontrolled" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a controlled input of type text to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if uncontrolled input switches to controlled', function() {
var stub = <input type="text" />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="text" value="controlled" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a uncontrolled input of type text to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if controlled checkbox switches to uncontrolled', function() {
var stub = <input type="checkbox" checked={true} onChange={emptyFunction} />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="checkbox" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a controlled input of type checkbox to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if controlled checkbox switches to uncontrolled with defaultChecked', function() {
var stub = <input type="checkbox" checked={true} onChange={emptyFunction} />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="checkbox" defaultChecked={true} />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a controlled input of type checkbox to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if uncontrolled checkbox switches to controlled', function() {
var stub = <input type="checkbox" />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="checkbox" checked={true} />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a uncontrolled input of type checkbox to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if controlled radio switches to uncontrolled', function() {
var stub = <input type="radio" checked={true} onChange={emptyFunction} />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="radio" />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a controlled input of type radio to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if controlled radio switches to uncontrolled with defaultChecked', function() {
var stub = <input type="radio" checked={true} onChange={emptyFunction} />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="radio" defaultChecked={true} />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a controlled input of type radio to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('should warn if uncontrolled radio switches to controlled', function() {
var stub = <input type="radio" />;
var container = document.createElement('div');
ReactDOM.render(stub, container);
ReactDOM.render(<input type="radio" checked={true} />, container);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'A component is changing a uncontrolled input of type radio to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or viceversa). ' +
'Decide between using a controlled or uncontrolled input ' +
'element for the lifetime of the component. More info: https://fb.me/react-controlled-components'
);
});

it('sets type before value always', function() {
var log = [];
var originalCreateElement = document.createElement;
Expand Down

0 comments on commit d684b15

Please sign in to comment.