-
Notifications
You must be signed in to change notification settings - Fork 47.6k
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
Fix “no onChange handler” warning to fire on falsy values ("", 0, false) too #12628
Changes from 12 commits
49851ff
32547ff
9e0be3d
15302d6
d8fbf04
d4a0e3c
f2fcf3c
41f4311
b17fab1
b91c2b0
484b0cb
e68c16a
3804ecd
70235ae
08bab46
c802ca5
6a03a7a
731e3e0
8bf0648
1e2d2a3
9cfbed6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,30 @@ describe('ReactDOMInput', () => { | |
ReactTestUtils = require('react-dom/test-utils'); | ||
}); | ||
|
||
it('should warn of no event listener with a falsey value of 0', () => { | ||
expect(() => { | ||
ReactTestUtils.renderIntoDocument(<input type="text" value={0} />); | ||
}).toWarnDev( | ||
'Failed prop type: You provided a `value` prop to a form field without an `onChange` handler.', | ||
); | ||
}); | ||
|
||
it('should warn of no event listener with a falsey value of ""', () => { | ||
expect(() => { | ||
ReactTestUtils.renderIntoDocument(<input type="text" value="" />); | ||
}).toWarnDev( | ||
'Failed prop type: You provided a `value` prop to a form field without an `onChange` handler.', | ||
); | ||
}); | ||
|
||
it('should warn of no event listener with a value of "0"', () => { | ||
expect(() => { | ||
ReactTestUtils.renderIntoDocument(<input type="text" value="0" />); | ||
}).toWarnDev( | ||
'Failed prop type: You provided a `value` prop to a form field without an `onChange` handler.', | ||
); | ||
}); | ||
|
||
it('should properly control a value even if no event listener exists', () => { | ||
const container = document.createElement('div'); | ||
let stub; | ||
|
@@ -475,7 +499,7 @@ describe('ReactDOMInput', () => { | |
}); | ||
|
||
it('should display `value` of number 0', () => { | ||
let stub = <input type="text" value={0} />; | ||
let stub = <input type="text" value={0} onChange={emptyFunction} />; | ||
stub = ReactTestUtils.renderIntoDocument(stub); | ||
const node = ReactDOM.findDOMNode(stub); | ||
|
||
|
@@ -625,8 +649,14 @@ describe('ReactDOMInput', () => { | |
it('should properly transition from an empty value to 0', function() { | ||
const container = document.createElement('div'); | ||
|
||
ReactDOM.render(<input type="text" value="" />, container); | ||
ReactDOM.render(<input type="text" value={0} />, container); | ||
ReactDOM.render( | ||
<input type="text" value="" onChange={emptyFunction} />, | ||
container, | ||
); | ||
ReactDOM.render( | ||
<input type="text" value={0} onChange={emptyFunction} />, | ||
container, | ||
); | ||
|
||
const node = container.firstChild; | ||
|
||
|
@@ -637,8 +667,14 @@ describe('ReactDOMInput', () => { | |
it('should properly transition from 0 to an empty value', function() { | ||
const container = document.createElement('div'); | ||
|
||
ReactDOM.render(<input type="text" value={0} />, container); | ||
ReactDOM.render(<input type="text" value="" />, container); | ||
ReactDOM.render( | ||
<input type="text" value={0} onChange={emptyFunction} />, | ||
container, | ||
); | ||
ReactDOM.render( | ||
<input type="text" value="" onChange={emptyFunction} />, | ||
container, | ||
); | ||
|
||
const node = container.firstChild; | ||
|
||
|
@@ -649,8 +685,14 @@ describe('ReactDOMInput', () => { | |
it('should properly transition a text input from 0 to an empty 0.0', function() { | ||
const container = document.createElement('div'); | ||
|
||
ReactDOM.render(<input type="text" value={0} />, container); | ||
ReactDOM.render(<input type="text" value="0.0" />, container); | ||
ReactDOM.render( | ||
<input type="text" value={0} onChange={emptyFunction} />, | ||
container, | ||
); | ||
ReactDOM.render( | ||
<input type="text" value="0.0" onChange={emptyFunction} />, | ||
container, | ||
); | ||
|
||
const node = container.firstChild; | ||
|
||
|
@@ -661,8 +703,14 @@ describe('ReactDOMInput', () => { | |
it('should properly transition a number input from "" to 0', function() { | ||
const container = document.createElement('div'); | ||
|
||
ReactDOM.render(<input type="number" value="" />, container); | ||
ReactDOM.render(<input type="number" value={0} />, container); | ||
ReactDOM.render( | ||
<input type="number" value="" onChange={emptyFunction} />, | ||
container, | ||
); | ||
ReactDOM.render( | ||
<input type="number" value={0} onChange={emptyFunction} />, | ||
container, | ||
); | ||
|
||
const node = container.firstChild; | ||
|
||
|
@@ -673,8 +721,14 @@ describe('ReactDOMInput', () => { | |
it('should properly transition a number input from "" to "0"', function() { | ||
const container = document.createElement('div'); | ||
|
||
ReactDOM.render(<input type="number" value="" />, container); | ||
ReactDOM.render(<input type="number" value="0" />, container); | ||
ReactDOM.render( | ||
<input type="number" value="" onChange={emptyFunction} />, | ||
container, | ||
); | ||
ReactDOM.render( | ||
<input type="number" value="0" onChange={emptyFunction} />, | ||
container, | ||
); | ||
|
||
const node = container.firstChild; | ||
|
||
|
@@ -911,6 +965,16 @@ describe('ReactDOMInput', () => { | |
ReactTestUtils.Simulate.change(instance); | ||
}); | ||
|
||
it('should warn of no event listener with a falsey checked prop', () => { | ||
expect(() => | ||
ReactTestUtils.renderIntoDocument( | ||
<input type="checkbox" checked="false" />, | ||
), | ||
).toWarnDev( | ||
'Failed prop type: You provided a `checked` prop to a form field without an `onChange` handler.', | ||
); | ||
}); | ||
|
||
it('should warn with checked and no onChange handler with readOnly specified', () => { | ||
ReactTestUtils.renderIntoDocument( | ||
<input type="checkbox" checked="false" readOnly={true} />, | ||
|
@@ -936,14 +1000,18 @@ describe('ReactDOMInput', () => { | |
|
||
it('should warn if value is null', () => { | ||
expect(() => | ||
ReactTestUtils.renderIntoDocument(<input type="text" value={null} />), | ||
ReactTestUtils.renderIntoDocument( | ||
<input type="text" value={null} onChange={emptyFunction} />, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like these changes are now unnecessary and can be reverted? We still expect one warning with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes gotcha 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ping :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good now! |
||
), | ||
).toWarnDev( | ||
'`value` prop on `input` should not be null. ' + | ||
'Consider using an empty string to clear the component or `undefined` ' + | ||
'for uncontrolled components.', | ||
); | ||
|
||
ReactTestUtils.renderIntoDocument(<input type="text" value={null} />); | ||
ReactTestUtils.renderIntoDocument( | ||
<input type="text" value={null} onChange={emptyFunction} />, | ||
); | ||
}); | ||
|
||
it('should warn if checked and defaultChecked props are specified', () => { | ||
|
@@ -1053,7 +1121,10 @@ describe('ReactDOMInput', () => { | |
const container = document.createElement('div'); | ||
ReactDOM.render(stub, container); | ||
expect(() => | ||
ReactDOM.render(<input type="text" value="controlled" />, container), | ||
ReactDOM.render( | ||
<input type="text" value="controlled" onChange={emptyFunction} />, | ||
container, | ||
), | ||
).toWarnDev( | ||
'Warning: A component is changing an uncontrolled input of type text to be controlled. ' + | ||
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' + | ||
|
@@ -1064,7 +1135,7 @@ describe('ReactDOMInput', () => { | |
}); | ||
|
||
it('should warn if uncontrolled input (value is null) switches to controlled', () => { | ||
const stub = <input type="text" value={null} />; | ||
const stub = <input type="text" value={null} onChange={emptyFunction} />; | ||
const container = document.createElement('div'); | ||
expect(() => ReactDOM.render(stub, container)).toWarnDev( | ||
'`value` prop on `input` should not be null. ' + | ||
|
@@ -1140,7 +1211,10 @@ describe('ReactDOMInput', () => { | |
const container = document.createElement('div'); | ||
ReactDOM.render(stub, container); | ||
expect(() => | ||
ReactDOM.render(<input type="checkbox" checked={true} />, container), | ||
ReactDOM.render( | ||
<input type="checkbox" checked={true} onChange={emptyFunction} />, | ||
container, | ||
), | ||
).toWarnDev( | ||
'Warning: A component is changing an uncontrolled input of type checkbox to be controlled. ' + | ||
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' + | ||
|
@@ -1151,11 +1225,16 @@ describe('ReactDOMInput', () => { | |
}); | ||
|
||
it('should warn if uncontrolled checkbox (checked is null) switches to controlled', () => { | ||
const stub = <input type="checkbox" checked={null} />; | ||
const stub = ( | ||
<input type="checkbox" checked={null} onChange={emptyFunction} /> | ||
); | ||
const container = document.createElement('div'); | ||
ReactDOM.render(stub, container); | ||
expect(() => | ||
ReactDOM.render(<input type="checkbox" checked={true} />, container), | ||
ReactDOM.render( | ||
<input type="checkbox" checked={true} onChange={emptyFunction} />, | ||
container, | ||
), | ||
).toWarnDev( | ||
'Warning: A component is changing an uncontrolled input of type checkbox to be controlled. ' + | ||
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' + | ||
|
@@ -1183,7 +1262,10 @@ describe('ReactDOMInput', () => { | |
const container = document.createElement('div'); | ||
ReactDOM.render(stub, container); | ||
expect(() => | ||
ReactDOM.render(<input type="radio" checked={null} />, container), | ||
ReactDOM.render( | ||
<input type="radio" checked={null} onChange={emptyFunction} />, | ||
container, | ||
), | ||
).toWarnDev( | ||
'Warning: A component is changing a controlled input of type radio to be uncontrolled. ' + | ||
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' + | ||
|
@@ -1224,11 +1306,14 @@ describe('ReactDOMInput', () => { | |
}); | ||
|
||
it('should warn if uncontrolled radio (checked is null) switches to controlled', () => { | ||
const stub = <input type="radio" checked={null} />; | ||
const stub = <input type="radio" checked={null} onChange={emptyFunction} />; | ||
const container = document.createElement('div'); | ||
ReactDOM.render(stub, container); | ||
expect(() => | ||
ReactDOM.render(<input type="radio" checked={true} />, container), | ||
ReactDOM.render( | ||
<input type="radio" checked={true} onChange={emptyFunction} />, | ||
container, | ||
), | ||
).toWarnDev( | ||
'Warning: A component is changing an uncontrolled input of type radio to be controlled. ' + | ||
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' + | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems wrong.
"false"
is not falsy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the behavior is right but the test title is confusing.