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

v.number should reject NaN #29

Closed
davidtheclark opened this issue May 3, 2019 · 4 comments · Fixed by #44
Closed

v.number should reject NaN #29

davidtheclark opened this issue May 3, 2019 · 4 comments · Fixed by #44

Comments

@davidtheclark
Copy link
Contributor

Although typeof NaN === 'number', I'd expect that you always want to reject NaN if you're expecting a real number.

@kepta
Copy link
Contributor

kepta commented May 3, 2019

I'd expect that you always want to reject NaN if you're expecting a real number.

I don't think we should bifurcate fusspots behaviour from standard Javascript. If NaN is a number, fusspot should treat it as a number, along with other non standard numbers like (Infinity, -Infinity). How about creating something like v.finite? Along the lines of Number.isFinite.

@davidtheclark
Copy link
Contributor Author

I don't know ... I don't know of any situation in which I'd want to accept NaN in a place where I'm validating for numbers. I guess we could make a different primitive validator — and I'd just always use that one, never use v.number.

@kepta
Copy link
Contributor

kepta commented May 3, 2019

I guess we could make a different primitive validator — and I'd just always use that one, never use v.number.

Seems reasonable to me. We already have a v.nonEmptyString dealing similar case for strings. Let us add a v.finite validator, which works similar to Number.isFinite.

@t3h2mas
Copy link
Contributor

t3h2mas commented Oct 22, 2020

Opened #44 to address this

@kepta @davidtheclark

@kepta kepta closed this as completed in #44 Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants