Include string holding a number in examples
bakert opened this issue ยท 8 comments
Wonderful library, thank you.
I had a poke around but didn't find anything for this. A common use case for this library is probably validating user input that comes in the form of strings.
v8n().number().test('23')
v8n().integer().test('23')
are both rightly false. But what's the appropriate way to test if a string contains a number/integer? It feels wrong to add a pre-validation validation step where we attempt to convert to an int and handle as a validation failure if it doesn't. But perhaps that's the intended usage. Or maybe I just missed it.
Thanks!
Thank you man for your question!
So, @bakert unfortunately, we don't have a good implementation for this kind of situation yet. Actually, I don't even know how would be a good implementation for that.
Today, I use to use something like this:
v8n()
.number(false)
.test(parseInt('123')); // true
The number
rule has this flag where you can inform if it should consider NaN
and infinite values as valid numbers. Defaults to true
.
So, in this example, if parseInt
returns NaN
, the validation will return false
, for example:
v8n()
.number(false)
.test(parseInt('hello')); // false
I don't know if it's a good solution for this problem, but that's something that will work.
Feel free to suggest other approaches or even ideas of how we could implement a good feature for this case. ๐
Ah ok. That's roughly where I ended up.
validDay = v8n().integer().between(1, 31).test(Number(day))
I didn't use parseInt because of parseInt("1jfkdsalfja") === 1
which is just wonky to me.
If I wanted to generate some kind of helpful error message here (I don't) I would have to do part of my validation "outside" v8n to know "hey they string you sent me is not the string of a number" and then go into the v8n stuff. So I think it would make sense to include.
I guess my suggestion is something like:
validDay = v8n().acceptString.integer().between(1, 31).test(day)
??
I don't like the name acceptString
much. Maybe it's a param to integer
?
Yes, @bakert it looks like a good idea. We just need to pay close attention to the way v8n
works with its modifiers
and rules
.
In your example, acceptString
is a modifier that is being applied to the integer
rule. But this makes sense for others rules
, too? Is it going to be ok with mixed rules? ๐ฌ
Modifiers cannot be exclusive for just a rule. If this only makes sense with integer
, maybe it should be part of the integer parameters. Or, if it's something that is expected for the whole validation, maybe it should be in v8n
constructor parameters. ๐ค
Anyway it's a tricky one, and we must be carefull. But solving tricky problems is really fun. ๐
I can believe that validDay = v8n().integer(stringOk=true).between(1, 31).test(day)
is the best API for it. But of course both number and integer (and others?) will want to be able to do this. And for string this could be a noop. So maybe it does belong in the chain. I bow to your wisdom :D
A parameter for integer
is probably easiest. The issue with JavaScript is that any added parameters make code less readable, as the name of the parameter is not included in the code (unlike Python for example). I don't like this idea, it adds clutter.
We currently have the number
and integer
rules. Instead of adding yet another parameter to integer we could add a rule for "string-numbers". Laravel, as an example, does not have a number
rule but rather a numeric
rule. This rule will check for any number or string that is a number. Ideally the number
or numeric
rule would do all the validation for numbers that integer does not do. This would include stringed numbers.
v8n().number().test(12); // true
v8n().number().test('12'); // true
v8n().number().test(12.01); // true
v8n().number().test('12.01'); // true
The parameter for Infinity
and NaN
could stay.
I like this a lot.
number
already exists but numeric
works.
I'd add:
v8n().numeric().test('12hello'); // false
to the test suite just to be explicit that we are avoiding the horror that is parseInt
's behavior :D
Very good @sebastianbarfurth
Woo!