Calling moment(undefined) should not return a valid moment date

[hvoecking]

Description of the Issue and Steps to Reproduce:

Calling moment with a single argument that is undefined returns a valid moment object with the current time. I would expect it to not silently ignore the argument passed as undefined. For example new Date does return an Invalid Date. I would expect that moment behaves similar. For example calling moment with null returns an NaN date:

$ node
> var moment = require('moment')
> moment(undefined)
moment("2017-02-14T11:22:57.411")
> new Date(undefined)
Invalid Date
> moment(null)
moment.invalid(/* NaN */)

My particular scenario that took me quite some time to figure out looked basically like this:

$ node
> var moment = require('moment')
> var obj = {timestamp: "2017-02-14T11:11:48.088"};
> moment(obj.timsetamp); // Note the mistyped field name
moment("2017-02-14T11:13:48.015") // Note the that the timestamp of the moment object is set to the current time and therefore differs to the timestamp of `obj`.

Environment:

$ node -v
v6.9.2
$ uname -a
Linux vagrant.development 3.2.0-121-virtual #164-Ubuntu SMP Wed Jan 11 15:43:33 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
$ node
> var moment = require('moment')
> console.log( (new Date()).toString())
Tue Feb 14 2017 11:28:12 GMT+0000 (UTC)
> console.log((new Date()).toLocaleString())
2/14/2017, 11:28:12 AM
> console.log( (new Date()).getTimezoneOffset())
0
> console.log( navigator.userAgent)
ReferenceError: navigator is not defined
…
> console.log(moment.version)
2.17.1

[icambron]

See #2729 and #3180. That new Date(undefined) is different from new Date() is the weird thing here, not Moment's behavior.

[dohse]

That new Date(undefined) produces invalid dates is standard Javascript behavior:

> new Date().getTime()
1487150538775
> new Date(undefined).getTime()
NaN

Why is moment.js not consistent with this?

[icambron]

Because that is not standard JS behavior; it's a weird legacy decision inconsistent with the purpose of undefined, most built-in JS APIs, and most JS libraries. Reading through the spec, the logic of it is that the Date constructor first checks the number of arguments without regard to their contents to determine which set of rules to apply and then applies those rules, with the strange result that an actually-passed-in undefined gets interpreted as NaN before constructing a date out of it. That's emphatically not the expected behavior of a JS library, as I explain in the tickets I linked to above.

[dohse]

This is indeed the behavior of ES6 default arguments:

> function getValue(value = Date.now()) { return value }
undefined
> getValue()
1487165737680
> getValue(undefined)
1487165740723
> getValue(null)
null

Unfortunately the current behavior of moment leads to hard to spot bugs like:

const object = {
  behaviourUpdate: new Date(),
};
console.log(moment(object.behaviorUpdate));

How can this be mitigated? Is there a way of constructing moments that does not accept undefined?

[icambron]

If you want to mitigate it, which I don't recommend, you can do this:

var protectiveMoment = function(...args){
  if (args.length > 0 && typeof(args[0]) == "undefined"){
    throw "Don't pass undefined to moment"
  }
  else {
     return moment.apply(args);
  }
}

And then use protectiveMoment() where you'd use moment()

[dohse]

Yeah, that is basically patching moment to behave safer. I thought more about adding a helper like moment.safe() that does neither accept undefined, nor no arguments.