Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn when an input switches between controlled and uncontrolled #5864

Merged
merged 1 commit into from
Feb 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 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 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
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