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

isDate() validation #22

Closed
ekorn opened this issue Jul 27, 2011 · 6 comments
Closed

isDate() validation #22

ekorn opened this issue Jul 27, 2011 · 6 comments

Comments

@ekorn
Copy link

ekorn commented Jul 27, 2011

Hi Chris,
had some trouble with the isDate() check. I had a look at the regex and it themes like it would expect something like MM/DD/YYYY but when the regex part is successful the next part fails .
Could you have a look at it and maybe include it in your test file.

Cheers,
Nico

@foxbunny
Copy link
Contributor

foxbunny commented Aug 4, 2011

I just got burned by this. In my case, I don't really care about the date format, as long as it can be parsed by Date.parse(), so I do this:

var someDate = Date.parse(someDate); // This never throws
if (!isNaN(someDate)) {
    someDate = new Date(someDate); // This won't throw as long as someDate is not NaN
}

I'm not sure how robust Date.parse is, but unless you want a whole custom date parser with format strings, or depend on an existing date parser library (I'm not aware of anything except maybe Date.js), I think it's best to just let the value run through Date.parse, and make sure it's a non-NaN value.

foxbunny pushed a commit to foxbunny/node-validator that referenced this issue Aug 4, 2011
@foxbunny
Copy link
Contributor

foxbunny commented Aug 4, 2011

So, I added the test that sort of addresses this problem. You can see in my next commit (b6a2950) passes the test (but is broken in other ways). I didn't do the pull request because this way of validating dates is ridiculous. First of all, it is date and time validation (you can say "11:00pm" to make it pass). The only reason it is silghtly 'better' (well, debatable, for sure) is that it doesn't enforce a date format. In all other respects, it's crap. It even passes stuff like "11" (which translates to "11pm 31st of Oct 2001", if it makes any sense).

An alternative solution is to pass format parameter using YYYY, YY, MMMM, MMM, MM, M, DD, and D wildcards. The MMMM and MM wildcards would be full month name, and short month name respectively, but then we have a locale problem.

@chriso
Copy link
Collaborator

chriso commented Aug 6, 2011

Yeah isDate() was a user-contributed validator. Date validation is a nightmare - I'll have to think of the best way of moving forward with this one..

@chriso
Copy link
Collaborator

chriso commented Aug 6, 2011

Actually @foxbunny I've added the changes you mentioned in this commit. Can you let me know how this goes? It (impressively) passes the tests.. (TIL about Date.parse!)

@chriso chriso closed this as completed Aug 6, 2011
@foxbunny
Copy link
Contributor

foxbunny commented Aug 6, 2011

I haven't been able to find a better solution. All parsers I've looked at will parse just about anything. If you throw a cow at them, they'll parse it into a Date object. So they can't be used reliably to validate dates. I think it should be noted in the docs, that isDate() should be used only if you want to make sure it's a 'pareseable string', and that one should use regex for more strict validation.

@chriso
Copy link
Collaborator

chriso commented Aug 6, 2011

I've added a note in the README, thanks

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