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

dateISO could be more flexible #265

Closed
jods4 opened this issue Apr 15, 2013 · 11 comments
Closed

dateISO could be more flexible #265

jods4 opened this issue Apr 15, 2013 · 11 comments
Assignees

Comments

@jods4
Copy link

jods4 commented Apr 15, 2013

The dateISO binding is a bit too restrictive, in that it doesn't accept a time part, although it's part of the ISO specification.

Case where this means trouble: if you receive a date from ASP.NET Web API in JSON format, you get this by default: 2010-01-01T00:00:00. Unfortunately it's refused by the validation plugin because dateISO only accepts yyyy-MM-dd.

A second issue is that dateISO only uses a regexp, hence it accepts things such as 3000-99-99.

I suggest the following fix: only check the mandatory ISO part by modifying the regexp: /^\d{4}[\/\-]\d{1,2}[\/\-]\d{1,2}/ (notice I removed the trailing $); and then delegate the work of figuring out if it's a correct date to rules['date']. This step also validates that the remaining of the expression is correct.

New rule:

function(value, validate) {
    if (!validate) return true;
    return utils.isEmptyVal(value) ||
    (/^\d{4}[\/\-]\d{1,2}[\/\-]\d{1,2}/.test(value) && 
    ko.validation.rules.date.validator(value, validate));
  };
@crissdev
Copy link
Member

@stevegreatrex I'm thinking on using the same regex that jquery.validate uses: https://github.com/jzaefferer/jquery-validation/blob/master/src/core.js#L1136 as it seems more complete.

@stevegreatrex
Copy link
Contributor

Sounds like a sensible idea to me

@crissdev
Copy link
Member

Othwerwise I can add some restrictions on month the least - 01-12. Currently for month can be anything from 0 to 99. I'll think more on this.

@stevegreatrex
Copy link
Contributor

Use the regex to pull the groups, parseInt and check?

@crissdev
Copy link
Member

Pulling out groups and parseInt works but it can be done with regex too. So that months like 0 won't be accepted.

@crissdev crissdev self-assigned this Nov 29, 2014
@crissdev
Copy link
Member

crissdev commented Dec 4, 2014

IMO date rule should not accept time to be specified. Moreover a date input doesn't accept time in the value. The rule will be changed to not allow months and days outside the valid range instead.

@crissdev crissdev removed the todo label Dec 4, 2014
@jods4
Copy link
Author

jods4 commented Dec 6, 2014

Fair enough for the time part, but regex only is hardly an acceptable solution. This validator still accepts things such as 2014-02-31, which is not a valid value. What's the point of having a dateISO validation rule if it does no better than a regex validator? It's even more confusing (for users) because the date validator doesn't have this bug.

@crissdev
Copy link
Member

crissdev commented Dec 7, 2014

@jods4 The assumptions you make are not correct.

  • 2014-02-31 may not be considered a valid date according to the Gregorian calendar but the JavaScript engine seems to ignore this. When creating a Date object with such a value the unexpected result is returned, ie. the date in the next month. A similar example is constructing a Date with new Date('2014-11-31') - which returns 1st of Dcember 2014.
  • date validator tries to create an instance of Date with the value being validated. So, if a value passes dateISO then it will also pass date rule validation (with the current API).

I'm not aware of any validation framework that will attempt to check the value against a specific calendar, although it may exist. More over, if you use a date input in a browser that supports this input type then you won't be able to enter such value in the first place - so it doesn't make sense to make the validation too strict.

Some tests to clarify this >> http://jsfiddle.net/dmwc84u7/

@jods4
Copy link
Author

jods4 commented Dec 8, 2014

@crissdev Thanks for the information. I wasn't aware that the Date object does parse and 'fix' invalid dates! This is surprising and certainly sloppy. I understand that ko-validation isn't a date manipulation library and can't decently do more than the browser actually supports.

FWIW, I currently use my own date validator, based upon momentjs. I just checked and momentjs returns 'Invalid date' for 2014-11-31, as it should be.

@crissdev
Copy link
Member

crissdev commented Dec 8, 2014

@jods4 You may post the validator to User Contributed Rule wiki page if possible as we don't have a rule based on moment.js. Thanks.

@jods4
Copy link
Author

jods4 commented Feb 11, 2015

The validator contains additional bits of logic that are specific to my project, but to be honest it really comes down to: moment(value).isValid().

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

No branches or pull requests

3 participants