Jimbly / timezone-mock

A JavaScript library to mock the local timezone
MIT License
105 stars 34 forks source link

setDate() returns wrong value in a DST scenario #56

Open dogusyildiz opened 2 years ago

dogusyildiz commented 2 years ago

The result of the setDate() method returns the wrong value in the following Daylight Saving Time (DST) scenario.

For Eastern Time Zone, DST happens at 2022-03-13 02:00 and the time immediately becomes 2022-03-13 03:00 https://www.timeanddate.com/time/change/usa

In this case, we have a Date instance of '2022-03-12T02:30:00.000-05:00' and add 1 day to the date. The expected result is '2022-03-13T03:30:00.000-04:00' because there is no 02:30 AM on the 13th. But instead, the result is '2022-03-13T01:30:00.000-05:00'

Please see below the outputs from the native Date class and from timezone-mock

new Date(new Date('2022-03-12T02:30:00.000-05:00').setDate(13));
// > Sun Mar 13 2022 03:30:00 GMT-0400 (Eastern Daylight Time)

timezoneMock.register('US/Eastern');
new Date(new Date('2022-03-12T02:30:00.000-05:00').setDate(13));
// > MockDate {d: Sun Mar 13 2022 01:30:00 GMT-0500 (Eastern Standard Time)}
timezoneMock.unregister();
Jimbly commented 2 years ago

Hi, that's equivalent to new Date('2022-03-13T02:30:00.000-05:00') which is a time that doesn't exist, so that's not too surprising, and I think Node's behavior on these strings has changed over the years.

See the (somewhat relevant) note in the README:

With DST timezones, it may sometimes behave slightly differently when given an ambiguous date string (e.g. "2014-11-02 01:00:00" in "US/Pacific", is treated as 1AM PDT instead of 1AM PST - same clock time, utc timestamp off by an hour).

For that particular case you mentioned, the "expected result" actually seems like the bug to me (calling .setDate(13) -> .setDate(14) changes the time to 3:30, but just .setDate(14) leaves the time at the expected 2:30)! That being said, it is apparently how Node currently behaves (though probably not all browsers, and possibly not old Node versions), so it wouldn't be too unreasonable to change it if possible, so a PR is welcome.

dogusyildiz commented 2 years ago

Hi,

Hi, that's equivalent to new Date('2022-03-13T02:30:00.000-05:00') which is a time that doesn't exist, so that's not too surprising, and I think Node's behavior on these strings has changed over the years.

Yes, I agree that the computed date is equivalent to new Date('2022-03-13T02:30:00.000-05:00').

For that particular case you mentioned, the "expected result" actually seems like the bug to me

I guess this is where are not on the same page. But we can agree to disagree here. As you already mentioned, it makes sense to be consistent with the latest Node version.

I'll try to come up with a solution for this in the following days.

Regards, Doğuş