Invalid Date's are Still Reported as isValid() = true

[netpoetica]

Description of the Issue and Steps to Reproduce:
As reported on Gitter, 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]"

@mj1856 claimed in associated PR:

It's an input validation bug with the year function. It has nothing to do with isValid.

But I'm not sure how so.

Environment:

All

Other information that may be helpful:

(new Date()).toString()
> "Mon Oct 03 2016 13:36:52 GMT-0400 (EDT)"
(new Date()).toLocaleString()
> "10/3/2016, 1:37:00 PM"
(new Date()).getTimezoneOffset()
> 240
navigator.userAgent
> "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1"

[mattjohnsonpint]

This reads too much like a duplicate of the original #3436. Re-wrote it as #3483.

[netpoetica]

@mj1856 Sorry bud tried my best there :) I took a look at the new ticket, but I am having trouble seeing the relation. Regardless of how you handle .year() input, no matter how you get to it, shouldn't the isValid() function do a check to make sure a Date that is an "Invalid Date" does not return true on an isValid() check?

I understand this could break a lot of current code, and probably should be saved for a major version bump, but just on a note of expected, intuitive, and sensible behavior, I would expect that when

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

then

moment().year("asdf").isValid()
=> false

[maggiepint]

@netpoetica, ultimately what will happen is that the first case will NOT occur. The result of that will be the current date, and valid will be true. Basically:

moment().year("asdf").toString()
"Tue Oct 04 2016 08:03:33 GMT-0700"

moment().year("asdf").isValid()
true

[netpoetica]

Thanks! I understand that, I think it makes sense to fix the issue with year input for sure.

However, I still feel like there may be a separate issue:

> moment().year('asdf').isValid()
true

> moment(new Date('asdf')).isValid()
false

Both of those have an internal prop from moment:

_d: Invalid Date

But for some reason, that isValid() function considers the first one to be valid.

Not sure why isValid() would report true for one "Invalid Date" and false for another "Invalid Date", but it seems like a potential error with isValid().

Not trying to be a pain in the butt here, just trying to help make sure momentjs stays awesome :)

[mattjohnsonpint]

I think you are confusing moment(input) with moment().someFunction(input). They are two different things. Calling moment() creates a moment object, in local mode, with the time set to now. Anything done after that is mutating the moment object.

Consider:

moment().year(2000).format()  // "2000-10-04T09:47:16-07:00"
moment([2000]).format()       // "2000-01-01T00:00:00-08:00"

The first one is following your pattern, which keeps today's month, day, and time values. The second one constructs a moment using only a year, initializing the remaining fields to their earliest possible values.

[mattjohnsonpint]

Also, the reason isValid is returning true is because it is validating the initial state of the moment object, not the current state. Once a moment is determined to be a valid moment, that is cached in the _isValid field, and it is never re-evaluated. It is assumed that any mutation you might perform would either succeed, or fail gracefully - either way, leaving the moment object in a valid state. That is why I opened the other issue.