Leap year and general date validation for isDate() #418
Conversation
| @@ -450,7 +450,26 @@ | |||
| }; | |||
|
|
|||
| validator.isDate = function (str) { | |||
| return !isNaN(Date.parse(str)); | |||
| var rawDate = new Date(str); | |||
| var normalizedDate = new Date(rawDate.getTime() + rawDate.getTimezoneOffset()*60000); | |||
There was a problem hiding this comment.
Why add rawDate.getTimezoneOffset() * 60000 here?
There was a problem hiding this comment.
A valid Date object like new Date('2011-09-30') could be interpreted differently depending on the javascript engine.
In webkit browsers and node.js, this returns Thu Sep 29 2011 20:00:00 GMT-0400 (EDT)
whereas in Firefox, this returns 2011-09-30 T00:00:00.000Z. I.e., different days are interpreted. Adding getTimezoneOffset() * 60000 standardizes the date.
There was a problem hiding this comment.
Fair enough. I was wondering more about the 60000 but I see now that getTimezoneOffset() returns an offset in minutes.
|
Added more tests and slightly modified the date matching regex. |
| , '2011-08-04 12:00' | ||
| , '2/29/24' |
There was a problem hiding this comment.
This test fails for me in UTC+1. rawDate is equal to Thu Feb 29 2024 00:00:00 GMT+0100 (CET) and normalizedDate is equal to Wed Feb 28 2024 23:00:00 GMT+0100 (CET).
There was a problem hiding this comment.
So I realized I've been doing date normalization the wrong way. There's built-in functions that use the UTC date. I've switch to using those. Tests should pass now.
Also, let me know if you'd like me to squash my commits.
There was a problem hiding this comment.
Awesome, thanks. Yeah that'd be great if you could squash them.
There was a problem hiding this comment.
The test is still failing for me. normalizedDate is equal to Thu Feb 29 2024 00:00:00 GMT+0100 (CET) and so getUTCDate() returns 28. All tests pass if I change getUTCDate() to getDate().
There was a problem hiding this comment.
Interesting. Tests fail for me if I switch to getDate(). I'll look at this later today.
There was a problem hiding this comment.
I'm still looking at this. I'll send out a diff soon.
There was a problem hiding this comment.
@chriso Just updated the diff. It should work now. I tested it in over 15 time zones.
Leap year and general date validation for isDate()
|
@ravestack I had to revert this one as it what causing invalid results when the date string had hours, minutes or seconds that started with a 2 or 3. If you can fix up those issues I'd be happy to accept another PR. |
|
@chriso really sorry about that. I'll be sure to make an extensive test suite this time. |
Let me know if you'd like me to add comments for anything in particular or if I should break this out into a separate function.