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

_.isNaN returns true for whatever created with new Number() instead of just NaN and new Number(NaN) #2257

Closed
debutt opened this issue Aug 2, 2015 · 5 comments · Fixed by #2259
Labels

Comments

@debutt
Copy link

debutt commented Aug 2, 2015

// All => true
_.isNaN(new Number(3))
_.isNaN(new Number(Infinity))
_.isNaN(new Number('donut'))

If _.isNaN is intended to return true for only NaN and new Number(NaN), I think in line 1304, obj !== +obj should be replaced with obj != +obj.

Correct me if I'm wrong : )

@michaelficarra
Copy link
Collaborator

I don't understand what you're trying to say.

@debutt debutt changed the title _.isNaN returns true for whatever created with new Number() instead of just NaN and new Nubmer(NaN) _.isNaN returns true for whatever created with new Number() instead of just NaN and new Number(NaN) Aug 2, 2015
@bjmiller
Copy link

bjmiller commented Aug 2, 2015

I don't know if his proposed solution will work. But, if _.isNaN(new Number(3)) returns true, that's probably a bug.

@megawac
Copy link
Collaborator

megawac commented Aug 2, 2015

Feel free to send a PR with the change to obj != +obj

@megawac megawac added the bug label Aug 2, 2015
@debutt
Copy link
Author

debutt commented Aug 3, 2015

If we change the code to obj != +obj, _.isNaN still returns true for things like new Number('dice'), but I guess that's effectively the same as NaN in operations : )

@debutt
Copy link
Author

debutt commented Aug 3, 2015

Since != won't make it through the build, I suggest we can use the good old isNaN with _.isNumber, thus

_.isNumber(obj) && isNaN(obj)

_.isNumber makes sure obj is a number, primitive or wrapped, and then isNaN makes sure only NaN and new Number(NaN) pass the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants