isSameOrBefore(xxx, "day") not working for very low dates

[floriansimon1]

Description of the Issue and Steps to Reproduce:

Construct the minimal possible date

const m1 = moment(-8640000000000000);
const m2 = moment()

// This is false, but it should be true:
m1.isSameOrBefore(m2, "day")

Environment:

Problem experienced in NodeJS 11.X on Arch Linux

> new Date()).toString()
Fri Nov 09 2018 13:09:34 GMT+0100 (heure normale d’Europe centrale)
> (new Date()).toLocaleString()
09/11/2018 à 13:09:34
> (new Date()).getTimezoneOffset()
-60
> console.log( navigator.userAgent)
ReferenceError (because of running this in a NodeJS REPL.

Why I think this is happening: when having a comparison unit, the low date is being changed internally to align it to the beginning of the day, which transforms the date to an invalid date.

Why I think this should be fixed: generally, I think it does not make much sense that moment does not work fully across all of its value range. You might think it's a fringe use case, but actually moment-range relies on this value to represent infinite dates, so those values are quite common.

[ichernev]

@floriansimon1 I won't ask how did you stumble upon this problem.

What I will say though, is that moment uses internally the Date object. When you have such a date object and try to set the hours to 0:

d = new Date(-8640000000000000);
d.setHours(0)
// now d is Invalid Date

I don't think there is much we can do about that, other than rewrite moment not to use Date. This might happen, but this particular issue won't be the tipping point for sure.

[floriansimon1]

As I said, moment-range uses these values to represent infinite ranges. I mistakenly took the start value of an infinite range and tried to use startOf('month') for some computation.

I'm sure moment uses Date, and to be fair I expect it to, otherwise the performance would be horrible.

I know well why this happens, but I'd like to argue that this is incorrect behavior. We can't fix the native Date, but we sure can fix moment. There could be guards preventing this kind of error from happening, even if you use Date.

I could understand why you would not want to include those guards, but still, that is by definition a bug. Maybe a mention on the main README of this known bug would be in order? What do you think?

I was thinking something along the lines of "moment uses native dates internally, so it inherits its shortcomings".