-
Notifications
You must be signed in to change notification settings - Fork 132
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
Fixes issue with filtering with property values of number 0 #47
Conversation
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.
Thanks! Just one simple change.
src/index.js
Outdated
@@ -387,7 +387,7 @@ function getItemValues(item, key) { | |||
value = item[key] | |||
} | |||
// concat because `value` can be a string or an array | |||
return value ? [].concat(value) : null | |||
return value || value === 0 ? [].concat(value) : null |
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'd rather change this to:
return value == null ? [].concat(value) : null
That way we cover null and undefined and all other cases will be concatenate.
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.
Makes sense. Will change it.
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.
Hmmm... Somehow this is breaking the build, it looks like it's the test:ts
script (runs as part of the validate
script). Does the validate
script work for you locally?
Nope, validate script fails locally : |
Hmmm... Looks like your changes are not related to this. It's breaking for me locally without your changes. I'll look into it. |
I'm not sure what happened, but things were very out of date. I've updated everything and it should work now. |
Thanks a bunch! |
🎉 This PR is included in version 2.2.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
😁 |
What:
#46
The filter does not return data if the value of the key being filtered is the number 0
Why:
It is possible that the data may contain properties with values which are numbers (e.g. age) and these properties could have the value 0 as well. Hence, filtering on 0 should be possible.
How:
By adding an additional check to make sure the number 0 is not incorrectly rejected by the if statement.
Checklist: