dayjs icon indicating copy to clipboard operation
dayjs copied to clipboard

`dayjs.tz` is not idempotent with dates in DST when it's not currently DST

Open scottschup opened this issue 3 years ago • 12 comments

Describe the bug Given:

  • it is currently Standard Time (i.e. not Daylight Saving Time)
  • the date being manipulated is in Daylight Saving Time

When Dayjs#tz is called more than once on a Dayjs object, the time moves back 1 hour each time Dayjs#tz is called.

> dayjs('2020-08-08T00:00:00.000Z').toString()
//=> 'Sat, 08 Aug 2020 00:00:00 GMT'

> dayjs('2020-08-08T00:00:00.000Z').tz('America/Chicago').toString()
//=> 'Fri, 07 Aug 2020 23:00:00 GMT'

> dayjs('2020-08-08T00:00:00.000Z').tz('America/Chicago').tz('America/Chicago').toString()
//=> 'Fri, 07 Aug 2020 22:00:00 GMT'

> dayjs('2020-08-08T00:00:00.000Z').tz('America/Chicago').tz('America/Chicago').tz('America/Chicago').toString()
//=> 'Fri, 07 Aug 2020 21:00:00 GMT'

The result is the same regardless of what timezones are used, or whether a timezone arg is passed in at all:

> dayjs('2020-08-08T00:00:00.001Z').tz('UTC').tz().tz('UTC').toString()
//=> 'Fri, 07 Aug 2020 21:00:00 GMT'

> dayjs('2020-08-08T00:00:00.001Z').tz('America/Chicago').tz('UTC').tz().toString()
//=> 'Fri, 07 Aug 2020 21:00:00 GMT'

> dayjs('2020-08-08T00:00:00.001Z').tz().tz().tz().toString()
//=> 'Fri, 07 Aug 2020 21:00:00 GMT'

It's February and I'm in Chicago, so I've only been able to confirm this issue occurs during CST with a date in CDT. I suspect that the opposite will be true in a few weeks (that calling Dayjs#tz on a CST time during CDT will result in a one-hour shift the other way).

And I saw several open issue tickets that are likely related or the result of the same underlying problem:

  • #1804
  • #1803
  • #1801
  • #1795
  • #1791
  • #1690
  • #1664
  • #1657
  • #1635
  • #1622
  • #1260
  • #1001

Expected behavior Applying a timezone should not change the underlying time.

Information

  • Day.js v1.10.7
  • OS: iOS 12.1
  • Time zone: GMT-06:00 (Central Standard Time)

scottschup avatar Feb 12 '22 00:02 scottschup

I filed a ticket (#1801) some days ago on the same topic and am wondering if I should try to PR a fix. The blocker I have against doing that is, I read the timezone plugin and I'm not 100% sure I understand the logic as it's implemented. Wonder if the right way to get started is to PR additions to https://github.com/iamkun/dayjs/blob/dev/test/timezone.test.js and add tests that fail.

sulkaharo avatar Feb 14 '22 07:02 sulkaharo

Reading the code of the UTC and Timezone plugins, the UTC plugin doesn't set the time zone of the date object (just flags $u=true), while the Timezone plugin uses $x.$timezone to determine the time zone, so there is no interaction between the two that I can see. If you initialise a date using dayjs.utc(number), dayjs.isUTC() return true for the object and the date string is in UTC format, but assigning a new time zone using dayjs.tz() will ignore the UTC flag and not detect the time zone format and thus consider the time is in local time of the computer running the code.

sulkaharo avatar Feb 15 '22 07:02 sulkaharo

This bug is also related: https://github.com/iamkun/dayjs/issues/1803

SkaceKamen avatar Feb 15 '22 08:02 SkaceKamen

+1. The tests I wrote to catch this started failing when the west coast switched off daylight savings time. Specifically, if you install the plugins utc, and tz, and define these functions:

const toUtcTime = time => dayjs.utc(time).tz('UTC')
const toLocalTime = time => dayjs.utc(time).tz()

then, you have the following behavior:

> const timestampUtc = "2021-09-30T20:08:35+00:00"
> toUtcTime(toLocalTime(timestampUtc)).format() === timestampUtc
false
> toUtcTime(toLocalTime(timestampUtc).format()).format() === timestampUtc
true
> 

The internal state of the dayjs object does not match its format string, as evidenced by the second line. If you don't re-use the dayjs objects and format them between each step, it works as expected.

The really lame part is, I don't even need the timezone plugin in my application. The only reason I included it is so that I could use dayjs.tz.setDefault() in my tests to (theoretically) get consistent results and assertions. Without including it, I have no idea how I would force the timezone that is considered "local time" for testing purposes.

snoozbuster avatar Feb 16 '22 18:02 snoozbuster

@iamkun can you confirm is the library is still being maintained? Based on the commit history you've touched dayjs logic last time 7 months ago and there doesn't seem to be another developer on board maintaining dayjs, resulting in a long backlog of pull requests and issues. If you've burned out or don't have time for this library, let's find a solution to getting the development back on track.

sulkaharo avatar Feb 21 '22 08:02 sulkaharo

@sulkaharo I ended up switching to Luxon in my project since it looks like this one is no longer active (and luxon’s docs are much better to boot). I don’t know if that’s an option for you but it may be worth considering.

snoozbuster avatar Feb 21 '22 16:02 snoozbuster

@sulkaharo I ended up switching to Luxon in my project since it looks like this one is no longer active (and luxon’s docs are much better to boot). I don’t know if that’s an option for you but it may be worth considering.

I'm about to go that route too. I found a workaround for this issue, but now anytime I enter a date that's not in the same Daylight Saving period as the current date, I get a "Too many re-renders" error from the MaterialUI DatePicker component, but no indication of what's causing it other than it's the DatePicker that's re-rendering too many times.

scottschup avatar Feb 21 '22 18:02 scottschup

I did end up switching over to Luxon and life is so much better now. Using it, it's clear that the creator built it with timezones in mind from the start rather than adding support for them later as an after-thought.

https://moment.github.io/luxon/#/why

scottschup avatar Mar 02 '22 22:03 scottschup

Faced the same problem. @iamkun I see you returned, please check that ASAP, or at least provide any workaround.

Serg-Mois avatar Mar 15 '22 12:03 Serg-Mois

+1 to what @sulkaharo said

@iamkun if all of the PRs and issues and noise around this project is too much, we should find a way to keep the development of this project going, since there are a lot of capable and willing people and it's a widely used (and awesome) library.

justingolden21 avatar Mar 26 '22 18:03 justingolden21

For those who need reliable behavior and cannot switch off of DayJS, thankfully this issue is easy to work around. This approach relies on the fact that multiple .tz() calls to the same timezone will stack up the offset. All we do here is measure the offset over multiple calls, and then add that offset back to the original.

function tz (date, timezone) {
  const once = date.tz(timezone)
  const twice = once.tz(timezone)
  const offset = once.diff(twice, 'minute')
  return once.add(offset, 'minute')
}

Travis-Enright avatar Mar 11 '23 17:03 Travis-Enright

This issue fixed in 1.11.2

Qiming-Liu avatar Mar 12 '25 01:03 Qiming-Liu