-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
handle options with value 0 properly #1310
Conversation
@@ -553,7 +553,7 @@ const Select = React.createClass({ | |||
* @param {Object} nextProps - optionally specify the nextProps so the returned array uses the latest configuration | |||
* @returns {Array} the value of the select represented in an array | |||
*/ | |||
getValueArray (value, nextProps) { | |||
getValueArray (value, nextProps = undefined) { |
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.
Why is this needed? is not the default behaviour?
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.
Yes you're right, but my IDE was warning me that this function was sometimes called with only 1 argument. I thought making it explicit wouldn't hurt.
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 param being optional should be indicated in the jsdoc http://usejsdoc.org/tags-param.html#optional-parameters-and-default-values, which should also stop the IDE warning
Nice it looks good to me, I agree that 0 it a possible value should not be consider as empty 👍 |
Is this going to be merged any time soon? |
No merged PRs since mid-november? Would really like this one to land :/ |
It would be great to get this merged 👍 |
fixes #1284