iamkun / dayjs

⏰ Day.js 2kB immutable date-time library alternative to Moment.js with the same modern API
https://day.js.org
MIT License
46.71k stars 2.28k forks source link

diff malfunctions when dates go across the ST/DST change date #2120

Open andre-moves opened 1 year ago

andre-moves commented 1 year ago

Describe the bug example of issue: https://replit.com/@AndreLowy/dayjs-diff-DST-issue?v=1

When using the diff method, if the first date is in Standard Time (ST) and the second date is in Daylight Savings Time (DST) or vice versa, the diff result will be incorrect. In the above repl, we have Nov 5 and Nov 11 at the same time of day. I run the diff method against those dates in UTC (doesn't use DST) and in America/Toronto zones (uses ST and DST). In UTC, the diff in days is 6, and in America/Toronto, it's only 5, but they should both be 6.

It seems like diff should take the ST/DST switch into consideration.

Expected behavior I expect the diff method to be aware of the ST/DST change between two dates and take that into consideration when making the comparison. In my repl example, both scenarios should return 6 days.

Information

BePo65 commented 1 year ago

Sorry, but IMHO replit is not the ideal solution to run such tests. So I made just a few simple Jest tests and they showed what was expected. BTW to switch timezones in dayjs, you have to use the timezone plugin. Adding an offset to a date string is not the same as changing the timezone (but of course you now that already).

  it('dayjs code in timezone "Europe/Berlin" - end of DST was 2022-10-30', () => {
    const dateStringThen = '2021-11-05T13:44:15.728Z'
    const dateStringNow = '2021-11-11T13:45:15.728Z'
    const then = dayjs(dateStringThen)
    const now = dayjs(dateStringNow)

    expect(then.utcOffset()).toBe(60)
    expect(then.utcOffset()).toBe(then.utcOffset())
    expect(now.diff(then, 'day')).toBe(6)
    expect(now.diff(then, 'hour')).toBe(144) // 6 * 24h
  })

  it('dayjs code with tz in "America/Toronto" with Offset', () => {
    const dateStringThen = '2021-11-05T13:44:15.728Z'
    const dateStringNow = '2021-11-11T13:45:15.728Z'
    const then = dayjs.tz(dateStringThen, TO)
    const now = dayjs.tz(dateStringNow, TO)

    expect(then.utcOffset()).toBe(-240) // -4 * 60min
    expect(then.utcOffset()).toBe(then.utcOffset())
    expect(now.diff(then, 'day')).toBe(6)
    expect(now.diff(then, 'hour')).toBe(145) // 6 * 24h + 1
  })

  it('dayjs code with tz in "America/Toronto"', () => {
    const dateStringThen = '2021-11-05T13:44:15.728'
    const dateStringNow = '2021-11-11T13:45:15.728'
    const then = dayjs.tz(dateStringThen, TO)
    const now = dayjs.tz(dateStringNow, TO)

    expect(then.utcOffset()).toBe(-240) // -4 * 60min
    expect(then.utcOffset()).toBe(then.utcOffset())
    expect(now.diff(then, 'day')).toBe(6)
    expect(now.diff(then, 'hour')).toBe(145) // 6 * 24h + 1
  })

  it('dayjs code with tz in "UTC" with offset', () => {
    const dateStringThen = '2021-11-05T13:44:15.728Z'
    const dateStringNow = '2021-11-11T13:45:15.728Z'
    const then = dayjs.tz(dateStringThen, UTC)
    const now = dayjs.tz(dateStringNow, UTC)

    expect(then.utcOffset()).toBe(0)
    expect(then.utcOffset()).toBe(then.utcOffset())
    expect(now.diff(then, 'day')).toBe(6)
    expect(now.diff(then, 'hour')).toBe(144) // 6 * 24h
  })
andre-moves commented 1 year ago

Thank you very much for taking the time to reply and write out some tests. This was very helpful. (btw To make sure it's clear, I'm changing the TZ env var in the bash shell, not changing the offset on the date string)

It's unfortunate that without using the timezone plugin, dayjs.diff doesn't work across the ST/DST change dates. IMHO I don't see what the benefit of this is since it results in arguably unexpected behaviour from dayjs. The fact that there's a solution makes this a much less pressing change.

Given all this, it seems that using the timezone plugin is really necessary for consistent behaviour.