properly validate date, including when instance = Date but is Invalid…

[netpoetica]

… Date

Fixes bug reported by @dagstuan

IDK what you people are doing for tests, it's very complicated to figure out where to put a test. meteor-test, typescript-test, grunt file test task, qtest, karma w/ qunit... meanwhile no test directory, no test folder anywhere? No spec files?

But if you could just add a test like

expect(isDate(new Date('asdf')).to.be.false

This will prove this code is working.

Cheers

[maggiepint]

IMO fundamentally an invalid date is still a date. I haven't dug though the code but I feel like this could potentially break a ton of logic. Five tests are already failing due to this change. I think this is a no-go.

[netpoetica]

Hmmm...

Maybe you could compromise by keeping your isDate function the way it is, but making the isValid() function properly check whether both isDate() && is actually a valid date?

[mattjohnsonpint]

We're using qunit. Tests go in /src/test. You can see existing validity tests here.

Which bug in particular are you referring to? Please reference by issue number. A search for issues with author:dagstuan returned nothing.

[netpoetica]

@mj1856 It was reported on Gitter, there is no existing issue to describe it. The issue is:

moment().year("asdf").isValid()
=> true
moment().year("asdf").toString()
=> "Invalid date"

Usually, in JS when we're checking for valid dates, a date is only "valid" if it also is actually not an Invalid Date.

var d = new Date('asdf')
> undefined
d
> Invalid Date
d instanceof Date
> true
Object.prototype.toString.call(d)
> "[object Date]"

The method from this PR is an old tried and true way of making sure that the date is not an instanceof Date that is also an Invalid Date

[mattjohnsonpint]

It's an input validation bug with the year function. It has nothing to do with isValid. Please log it separately as an issue if you would.

[netpoetica]

See #3481

[mattjohnsonpint]

Now lives as #3483.