Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getTimezoneOffset appears to incorrect calculate daylight savings time #227

Open
douglas-linder-hireup opened this issue Feb 15, 2023 · 4 comments

Comments

@douglas-linder-hireup
Copy link

There are so many tickets on this issue, I've tried to consolidate my findings into the 'tldr' section at the bottom.

However, in detail... the documentation on this function says:

For time zones where daylight savings time is applicable a Date should be passed on the second parameter to ensure the offset correctly accounts for DST at that time of year.

However, and this is critically important, what it doesn't say is what the date should be.

What should the date be?

It turns out some time zones do strange things like put the transition between DST and non-DST on odd hours in the morning, like 2am in the hopes that won't impact too many people.

So, given that a specific day has the clocks turned back to 2am when you hit 3am, and you want to know what your offset is at 2am, what date should you pass to this function?

Local time? You can't. It's ambiguous. There is no UTC moment that matches '2am' local time. There are two moments, with two different offsets for that local time.

We therefore conclude that is it incorrect to pass a local time date into getTimezoneOffset

The implementation assumes the opposite. This seems a) wrong, and b) trivial to fix.

Let's move on...

Tests

Specifically, these tests: https://github.com/marnusw/date-fns-tz/blob/master/src/getTimezoneOffset/test.js#L49

The code:

var date = new Date('2020-10-04T00:45:00.000Z') 

Returns the UTC timestamp 1601775900000, which is Sunday, October 4, 2020 1:45:00 AM GMT <---

However, this test is supposed to be asserting that when the local time ticks over, the daylight savings is applied.

As previously discussed, using the local time for this cannot be correct. These tests should be using UTC time.

If we substitute the utterly unambiguous exact UTC moment:

    it('15 minutes before', function () {
      var date = new Date(1601739900000) 
      assert.equal(getTimezoneOffset('Australia/Melbourne', date), 10 * hours)
    })

    it('15 minutes after', function () {
      var date = new Date(1601741700000)
      assert.equal(getTimezoneOffset('Australia/Melbourne', date), 11 * hours)
    })

Then the test suite fails.

By adding additional tests, you can see that the conversion is in fact running at the GMT time equivalent.

In fact, this test passes:

   it('does DST at GMT time incorrectly', function () {
      var date = new Date(1601776800000) // Sunday, October 4, 2020 2:00:00 AM GMT. Wrong!
      assert.equal(getTimezoneOffset('Australia/Melbourne', date), 11 * hours)
    })

What?

So what's happening?

1601776800000 -> Sunday, October 4, 2020 2:00:00 AM GMT <--- DST is applied at this moment

1601740800000 -> Sunday, October 4, 2020 2:00:00 AM Australia/Melbourne <-- is should be applied here

The DST is being applied to local times.

Why?

It's pretty straight forward:

https://github.com/marnusw/date-fns-tz/blob/master/src/getTimezoneOffset/index.js#L31

return -tzParseTimezone(timeZone, date)

but:

https://github.com/marnusw/date-fns-tz/blob/master/src/_lib/tzParseTimezone/index.js#L15

export default function tzParseTimezone(timezoneString, date, isUtcDate) { <---- !!!!

tzParseTimezone is being invoked with isUtcDate undefined, and therefore false.

Therefore, it converts the incoming date object to UTC.

To be fair, in when this was added (dd83ed0), that parameter didn't exist; it was added to fix a different DST issue in e97b76c

TLDR

There's a bug.

It's here: https://github.com/marnusw/date-fns-tz/blob/master/src/getTimezoneOffset/index.js#L31

It should read:

return -tzParseTimezone(timeZone, date, true)

The question is: should it be fixed? Or is that cool, or, because it fundamentally changes the API behaviour, a breaking change?

Either way, I just want to call out, again, that this isn't a documentation issue.

Passing a local time into this function is fundamentally wrong and cannot produce unambiguous results.

@douglas-linder-hireup douglas-linder-hireup changed the title getTimezoneOffset appears to incorrect calculate daylight savings time (deliberately?) getTimezoneOffset appears to incorrect calculate daylight savings time Feb 15, 2023
@TranVanTinhUIT
Copy link

I had a similar problem and I solved it as follows:

function getTzOffset(timeZone: string, strISODate: string) {
  const zoneDate = utcToZonedTime(new Date(strISODate), timeZone);
  const zoneDateStr = format(zoneDate, 'yyyy-MM-dd HH:mm:ss');
  const utc = new Date(strISODate);
  const msOffset = utc.getTime() - new Date(zoneDateStr + 'Z').getTime();
  return msOffset / (1000 * 60);
}

@tomfa
Copy link

tomfa commented Aug 14, 2023

Here's a test we have that highlights this bug (if I read this issue right):

// 3:30 AM in CEST (UTC+2)
const immediatelyAfterDstChange = new Date('2022-03-27T01:30:00.000Z'); 

// Fails, returns 3600 * 1000
expect(getTimezoneOffset('Europe/Oslo', immediatelyAfterDstChange)).toBe(7200 * 1000)

@bferreira
Copy link

bferreira commented Dec 8, 2023

We are also experience this issues when using getTimezoneOffset together with Highcharts:
image
This should be twice 2:00

@dereekb
Copy link

dereekb commented Mar 11, 2024

I've run into this right now where getTimezoneOffset() returns a -5 hour offset for 12AM for America/Chicago instead of -6 up until 2/3AM when it actually becomes Central Daylight Time.

Thanks @TranVanTinhUIT for your workaround since that does return the proper offset for time before 2AM.

I did have to change it a little bit to match the same tzOffset return value (GMT-5 should be -5 hours in milliseconds):

const tzOffset = getTimezoneOffset(timezone, date);

is now:

const zoneDate = utcToZonedTime(date, timezone);
  const zoneDateStr = formatDate(zoneDate, 'yyyy-MM-dd HH:mm');   // ignore seconds, etc.
  const zoneDateTime = new Date(zoneDateStr + 'Z').getTime();
  const inputTime = setDate(date, { seconds: 0, milliseconds: 0 }).getTime(); // use date-fns set() function to clear ms/seconds since offset should only be hours/minutes
  const tzOffset = zoneDateTime - inputTime;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants