fix: utcOffset when local in DST but date is not

[aloisklink]

Fixes a bug in the valueOf() function after setting a utcOffset, when the the local timezone offset changes.

I added a test for it to test/timezone.test.js, so it should be reproducible on other people's computers as well.

Without this fix, when trying to run npm test in my timezone (British Summer Time - BST), there are quite a lot of off-by-one errors, e.g.

  ● DST, a time that never existed Fall Back › 2012-11-04 02:00:00

    expect(received).toBe(expected) // Object.is equality
    
    Expected value to be:
      1352012400000
    Received:
      1352016000000

      206 |       expect(d.format()).toBe('2012-11-04T02:00:00-05:00')
      207 |       expect(d.utcOffset()).toBe(-300)
    > 208 |       expect(d.valueOf()).toBe(1352012400000)
      209 |     })
      210 |   })
      211 | })
      
      at forEach (test/plugin/timezone.test.js:208:27)
          at Array.forEach (<anonymous>)
      at Object.<anonymous> (test/plugin/timezone.test.js:204:21)

  ● keepLocalTime › keepLocalTime

    expect(received).toBe(expected) // Object.is equality
    
    Expected value to be:
      "2013-11-18T17:55:00+01:00"
    Received:
      "2013-11-18T18:55:00+02:00"

      263 |   const base = dayjs.tz('2013-11-18 11:55', 'America/Toronto')
      264 |   it('keepLocalTime', () => {
    > 265 |     expect(base.tz('Europe/Berlin').format()).toBe('2013-11-18T17:55:00+01:00')
      266 |     expect(base.tz('Europe/Berlin', true).format()).toBe('2013-11-18T11:55:00+01:00')
      267 |   })
      268 | })
      
      at Object.<anonymous> (test/plugin/timezone.test.js:265:47)

[iamkun]

I don't know why this PR shows "This branch has not been deployed".

Would you please close it and re-open a new one?

[codecov]

Codecov Report

Merging #1448 (c400f3c) into dev (9c20e77) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##              dev     #1448      +/-   ##
===========================================
+ Coverage   99.94%   100.00%   +0.05%     
===========================================
  Files         176       179       +3     
  Lines        1770      1996     +226     
  Branches      411       507      +96     
===========================================
+ Hits         1769      1996     +227     
+ Misses          1         0       -1     

Impacted Files	Coverage Δ
src/plugin/utc/index.js	100.00% <100.00%> (+1.07%)	⬆️
src/index.js	100.00% <0.00%> (ø)
src/utils.js	100.00% <0.00%> (ø)
src/locale/bg.js	100.00% <0.00%> (ø)
src/locale/br.js	100.00% <0.00%> (ø)
src/locale/ca.js	100.00% <0.00%> (ø)
src/locale/cs.js	100.00% <0.00%> (ø)
src/locale/de.js	100.00% <0.00%> (ø)
src/locale/et.js	100.00% <0.00%> (ø)
src/locale/fi.js	100.00% <0.00%> (ø)
... and 52 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c20e77...c400f3c. Read the comment docs.

[pszymanski95]

Hello 👋, any updates on this PR? Looks like it can fix a problem mentioned lately in that issue #1437, which I also encountered in my project.

[aloisklink]

I don't actually know if it will fix #1437, since I have only tested this for using utc offsets (not timezones), when you are running dayjs in a region that has daylight savings.

It does look like a similar issue though, so hopefully it does fix that issue, but unless somebody writes a test for #1437, we can't be sure.

[basuneko]

Thanks for the work on this @aloisklink.
We're in NZ +12/+13 timezone and we've also encountered #1437 in our project. This PR seems to resolve it (at the very least, the off-by-one test failures in dayjs seem to go away). Would be great to have it merged

[fenharel117]

This PR resolves off-by-one errors I've been encountering

[basuneko]

There have been a few issues that are probably related to this (#1635, #1622), and more are coming as we are nearing the DST change (last sunday of September here in NZ).
@iamkun could you please give an indication of when (or if?) this fix is going to be merged and released?
Thank you for the awesome work ❤️

[steponas]

On my machine (EEST, UTC+3) there are 7 test failures on vanilla dev/master branches.
When applying this change, all failures are fixed.

@iamkun is there any blocking reason why this isn't merged yet? If there is anything changeable - I can help.

[fredzv]

I actually applied this fix to utc.js in my node_modules folder. It seems to have fixed the issue when converting timezones .tz("some/timezone", true) when daylight savings is active. It now outputs the same result as moment.js (and the result that I would expect).

[jamieparkinson]

For those who are waiting on this PR, you can apply this fix in your application by means of a plugin, like we've done here https://github.com/wellcomecollection/wellcomecollection.org/blob/32af11a4169a0a91ff3e1efd2cf2bf919d61fb90/common/utils/dates.ts#L37-L45

[mr-short]

Bump for asking why this is not being resolved? Seems to be effecting a lot of timezone/utc issues people are having

[xvaara]

@iamkun any change to get this in?

[danielmalmros]

bump - @iamkun we are experience a similar issue in our production env regarding timezone/offset from UTC, so if the PR is ready, what is waiting?

Currently we experience that Europe/Copenhagen timezone shows wrong offset '$offset': 120, - it should be '$offset': 60 due to european winter time :)

Using version: 1.10.6

[Mike-Becatti]

@msal4 - Any idea when this can get pulled?

[xvaara]

bump - @iamkun we are experience a similar issue in our production env regarding timezone/offset from UTC, so if the PR is ready, what is waiting?

Currently we experience that Europe/Copenhagen timezone shows wrong offset '$offset': 120, - it should be '$offset': 60 due to european winter time :)

the fix few comments up works fine. Use that. Seems weird that there is no reaction to this, since dayjs has the wrong time for most of europe, maybe every DST region...

[dgrelaud]

This PR did not work in my case so I have created an alternative DaysJS plugin for timezone.
https://www.npmjs.com/package/dayjs-timezone-iana-plugin

cf. my comment: #1271 (comment)

[gl-aagostino]

I don't know how this is supposed to work, but if I do something like

const monthValue = date.get('month');
const month = dayjs
        .utc(date)
        .set('month', monthValue)
        .tz("America/Los_Angeles")
        .startOf('month')
        .utc();

and move through the months with a date picker, in March, start of month becomes -6:00GMT instead of -7:00GMT, and once I hit December it goes back to -8:00 GMT. Why does it change by 2 hours?

[iamkun]

🎉 This PR is included in version 1.11.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀