maxLength fails if observable happens to be a number rather than a string

[grofit]

I can see the next comment being "It is not meant for numbers", which is 100% correct and I understand that. However I have a scenario where a user can enter up to 20 characters into an observable, however more often than not they would put in a number like "20".

Now in most cases this is fine and is processed as text, however it seems that upon rehydration of data I assume knockout is treating it as a number 20 not the text 20 which causes validation to fail.

Now the failure is because in this case when maxLength check does:

val.length >= minLength it will return false as val.length is undefined.

Although I am raising this issue here I am not sure if it is this libraries job to handle this use case, however I cannot tell how it is getting converted from a string to a number as it just an ajax request down from the server with the data, so it just populating like any other means myObservable(someValueFromServer).

I only raise it because in JS we do not enforce type so it could be possible for you to have something that you want to have a maxLength on if it is string but if it is not then dont apply it, so I was wondering if maybe it should be checked to see if length is not undefined first then try that check, if it is undefined then maxLength should not apply... however again I am not sure if that is the right stance...

I wanted to raise it anyway as it has been giving me issues for while but it was only today I realised what the issue was with it reading the text as a number on rehydration. I can work around it but wanted to know your thoughts.

[stevegreatrex]

I'm not sure that it should be the responsibility of this library to cover
this scenario but it would also be a low-risk change to add ''+ to the
check.

If you open a PR with tests etc then I'll get it merged in.
On 9 Oct 2014 16:50, "Lee Prosser" notifications@github.com wrote:

I can see the next comment being "It is not meant for numbers", which is
100% correct and I understand that. However I have a scenario where a user
can enter up to 20 characters into an observable, however more often than
not they would put in a number like "20".

Now in most cases this is fine and is processed as text, however it seems
that upon rehydration of data I assume knockout is treating it as a number
20 not the text 20 which causes validation to fail.

Now the failure is because in this case when maxLength check does:

val.length >= minLength it will return false as val.length is undefined.

Although I am raising this issue here I am not sure if it is this
libraries job to handle this use case, however I cannot tell how it is
getting converted from a string to a number as it just an ajax request down
from the server with the data, so it just populating like any other means
myObservable(someValueFromServer).

I only raise it because in JS we do not enforce type so it could be
possible for you to have something that you want to have a maxLength on if
it is string but if it is not then dont apply it, so I was wondering if
maybe it should be checked to see if length is not undefined first then try
that check, if it is undefined then maxLength should not apply... however
again I am not sure if that is the right stance...

I wanted to raise it anyway as it has been giving me issues for while but
it was only today I realised what the issue was with it reading the text as
a number on rehydration. I can work around it but wanted to know your
thoughts.

—
Reply to this email directly or view it on GitHub
#457.

[grofit]

it is not quite as simple as adding an empty string/char to the front of the object as you accept arrays and other possible objects, so I think the safest approach is to check if length is undefined after the isEmpty check then if it doesnt have a length try adding the string, or adding some form of isNumber to the utils by doing an inverse NaN check or something, then if it is prefix the string.

[grofit]

I have made proposed changes on c9 and run tests:

https://ide.c9.io/grofit/knockout-validation

If you connect up and just run grunt you should see all tests pass, 2 new ones were added to min and max length specifically to check for numbers with the issue number. To reduce risk I have added an isNumber method to utils which is not the best but it at least isolates this specific use case.

Take a look and if you are happy I will commit it up to my fork and push.

[stevegreatrex]

Sorry, I don't have a c9 account; if you push it to github I'll take a look

On 9 October 2014 18:53, Lee Prosser notifications@github.com wrote:

I have made proposed changes on c9 and run tests:

https://ide.c9.io/grofit/knockout-validation

If you connect up and just run grunt you should see all tests pass, 2 new
ones were added specifically to check for numbers with the issue number. To
reduce risk I have added an isNumber method to utils which is not the
best but it at least isolates this specific use case.

Take a look and if you are happy I will commit it up to my fork and push.

—
Reply to this email directly or view it on GitHub
#457 (comment)
.