Skip to content

improves documentation for isDate()#480

Closed
tacomanator wants to merge 1 commit into
validatorjs:masterfrom
tacomanator:master
Closed

improves documentation for isDate()#480
tacomanator wants to merge 1 commit into
validatorjs:masterfrom
tacomanator:master

Conversation

@tacomanator

Copy link
Copy Markdown

Was confused by the docs which said isDate takes a string. Took some time to figure out correct way. Hoping this helps others avoid that.

@chriso

chriso commented Jan 26, 2016

Copy link
Copy Markdown
Collaborator

It does take a string! The library validates strings only, and your dates are being coerced to a string before validation:

> new Date(2015, 1, 26).toString()
'Thu Feb 26 2015 00:00:00 GMT+0900 (JST)'

The date validator is fairly lenient which may be the cause of confusion (#476). Can you provide some examples that you found confusing?

@tacomanator

Copy link
Copy Markdown
Author

@chriso I see that I didn't investigate enough. My expectation was that isDate would accept any string that works with new Date(). However I ran into the following problem:

 > new Date('2011-12-21')
<· Tue Dec 20 2011 16:00:00 GMT-0800 (PST)

 > isDate('2011-12-21')
<· false

 > isDate(new Date('2011-12-21'))
<· true

While looking into it I found #476, which says to use toDate on strings before passing them to isDate. This worked for the above date, so I made a bad assumption.

 > isDate(toDate('2011-12-21'))
<· true

Is it supposed to be that isDate only accepts strings in the format output by date.toString()?

@chriso

chriso commented Jan 27, 2016

Copy link
Copy Markdown
Collaborator

As noted in #476, the isDate validator uses the built-in Date.parse() function which is very lenient. If Date.parse(str) returns something other than NaN then the validator attempts to filter out invalid dates such as 2015-02-29:

> validator.isDate('2011-12-21')
true
> validator.isDate('2015-02-29')
false

The fact that your validator.isDate('2011-12-21') is returning false is a bug, probably timezone related (#472, #431). What version are you using and what's the result of new Date().getTimezoneOffset() for you? Do any of the unit tests fail (npm test)?

@tacomanator

Copy link
Copy Markdown
Author

@chriso Version is 4.5.1

> new Date().getTimezoneOffset()
480

One test fails:

  77 passing (256ms)
  1 failing

  1) Validators should validate dates:
     Error: validator.isDate(2011-09-30) failed but should have passed
      at test/validators.js:19:23
      at Array.forEach (native)
      at test (test/validators.js:14:23)
      at Context.<anonymous> (test/validators.js:892:9)

The same input fails in the browser. Here's a screenshot of the inspector:

2016-01-26_21-38-49

@chriso

chriso commented Jan 27, 2016

Copy link
Copy Markdown
Collaborator

Thanks. Would you mind applying the following diff and the re-running the tests?

diff --git i/validator.js w/validator.js
index 900e718..cd3b777 100644
--- i/validator.js
+++ w/validator.js
@@ -522,7 +522,8 @@
         } else {
             timezone = iso8601Parts[21];
             if (!timezone) {
-                return null;
+                // if no hour/minute was provided, the date is GMT
+                return !iso8601Parts[12] ? 0 : null;
             }
             if (timezone === 'z' || timezone === 'Z') {
                 return 0;

The following two dates are both valid ISO8601 dates, but Date.parse() parses the first in the user's timezone and the second in GMT:

> new Date('2015-01-01 12:00')
Thu Jan 01 2015 12:00:00 GMT+0900 (JST)
> new Date('2015-01-01')
Thu Jan 01 2015 09:00:00 GMT+0900 (JST)

@tacomanator

Copy link
Copy Markdown
Author

@chriso tests pass with the patch

chriso added a commit that referenced this pull request Jan 28, 2016
The validator accepts ISO 8601 dates which include "%Y-%m-%d" and
"%Y-%m-%d %H:%M". The validator makes use of Date.parse() to parse
the date before checking if the date is invalid (e.g. 2015-02-29).
The built-in Date.parse() function interprets "%Y-%m-%d" dates in
GMT but "%Y-%m-%d %H:%M" in the user's timezone. This caused the
validator to apply an incorrect timezone offset which then caused
it to get out of sync with the built-in Date.parse() function.
Because of this mismatch, certain dates such as 2011-12-21 were
being marked as invalid when the user had a timezone offset < 0,
e.g. in PST, since the date would become 2011-12-20 after the
incorrect timezone offset was applied.

This fixes the function that determines the timezone in the date.
It now checks for the special "%Y-%m-%d" case and returns a
timezone offset of 0 (GMT).

cc #480
@chriso

chriso commented Jan 28, 2016

Copy link
Copy Markdown
Collaborator

Thanks. I've published the fix in 4.5.2. Did you have any other issues/inconsistencies with the validator?

@tacomanator

Copy link
Copy Markdown
Author

Thanks I'll grab the new version and let you know if I run into any other issues :)

@chriso

chriso commented Jan 28, 2016

Copy link
Copy Markdown
Collaborator

Awesome, thanks!

@chriso chriso closed this Jan 28, 2016
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

Successfully merging this pull request may close these issues.

2 participants