Add option for strictNumbers. Resolves #1128. #1219
Conversation
issacgerges
force-pushed
the
strict-numbers
branch
from
577ff49
to
6727a79
Compare
|
@issacgerges thank you - added some comments. |
issacgerges
force-pushed
the
strict-numbers
branch
from
6727a79
to
3f25187
Compare
|
Hey @epoberezkin made the changes you suggested. I ended up leaving |
|
@epoberezkin any other changes you'd like me to make? |
spec/options/strictNumbers.spec.js
Outdated
| var validate = ajv.compile({type: 'integer'}); | ||
| validate(1).should.equal(true); | ||
| validate(NaN).should.equal(false); | ||
| validate(Infinity).should.equal(true); |
There was a problem hiding this comment.
Just noticed it, sorry - I think integer should also not allow Infinity with strictNumbers.
There was a problem hiding this comment.
I don't think anybody would want to have a separate option and integers are also numbers (i.e. they would not fail validation with type: 'number'). The idea of type: 'integer' is to impose stricter constraint than number, but in this case it becomes somewhat overlapping rather than stricter.
There was a problem hiding this comment.
And I would also add non-integer examples to the tests, to make sure nothing was broken with the new option (e.g. 1.1 would pass number but fail integer).
made one small comment about integer with strict numbers - sorry, missed it before. Almost there. |
issacgerges
force-pushed
the
strict-numbers
branch
from
3f25187
to
edd8c92
Compare
no worries @epoberezkin, I believe I should have met all your requests this time. let me know! |
|
Thank you! |
What issue does this pull request resolve?
#1128
What changes did you make?
Added an option for
strictNumberswhich is used incheckDataTypeto conditionally checkisFinite(date). Added documentationIs there anything that requires more attention while reviewing?
The current behavior of
ajvis thatintegerwill always failNaNand passInfinitybutnumbercurrently doesn't. To respect semver, this will continue to behave the same way, and this new optionstrictNumbersis off by default.I'm unfamiliar with some of the internals of
ajvand believe that my changes tocheckDataTypeare sufficient. Also was unsure why y'all were usingdata === datavsisFinite(data)in integer (one will only failNaN, the other will failNaN and pos/negInfinity`)