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

fix: tz with DST date while DST is not active (#1340) #1352

Merged
merged 2 commits into from Jan 28, 2021

Conversation

hansdaniels
Copy link
Contributor

@addisonElliott, your last suggestion made my new tests work and also no longer break the existing tests.
No complains about keepLocalTime.

@addisonElliott
Copy link

Glad to hear it. This should resolve most DST issues people have experienced.

I would recommend adding tests for the bug that this fixes. Using that specific date you had in the issue should work for a good test.

@codecov
Copy link

codecov bot commented Jan 22, 2021

Codecov Report

Merging #1352 (0170f24) into dev (e93e6b8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1352   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          176       176           
  Lines         1737      1737           
  Branches       394       394           
=========================================
  Hits          1737      1737           
Impacted Files Coverage Δ
src/plugin/timezone/index.js 100.00% <100.00%> (ø)

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 e93e6b8...0170f24. Read the comment docs.

@hansdaniels
Copy link
Contributor Author

I've added 2 expects to an already existing test case for DST in test/plugin/timezone.test.js. One date with DST, one without, exactly what I needed. Before I have applied your suggested changes, the DST expect failed, afterwards both passed.

The exact date minutes before and after a DST change for 'Europe/Berlin' aren't any more special than dates in December and June for 'America/Los_Angeles' as used in the "DST" testcase.

@hansdaniels
Copy link
Contributor Author

Side-Problems:

  • Commiting from Windows is hard, eslint complains when having files checked out with CRLF (even if I have told Git to commit with LF).
  • The pre-commit hook failed because eslint wasn't a known command. I've circumvented this by running node node_modules/eslint/bin/eslint.js src/* test/* build/* manually and commiting with -n which shouldn't be necessary to my eyes.

@alex-w0
Copy link

alex-w0 commented Jan 26, 2021

Is this pull request related to this issue?

@addisonElliott
Copy link

addisonElliott commented Jan 26, 2021

@alex-w0 I think so, but I'm not entirely sure. This bug fixes UTC offsets for DST-related timezones (pretty vague I know). Best way is going to be trying the failing tests from the issue after applying this change.

@iamkun
Copy link
Owner

iamkun commented Jan 27, 2021

nice catch

@addisonElliott
Copy link

@iamkun Do you need anything in order to merge this?

@ashconnell
Copy link

ashconnell commented Mar 31, 2021

I think this commit has just made things worse.

Edit: plot twist, it didn't. My brain is just fried from looking at dates all day. Kindly ignore me.

I'm specifically seeing this issue: #1437

So I checked this commit out on dev and added a test for my real world scenario:

it('DST startOf(day)', () => {
  const beforeDST = dayjs
    .tz('2021-04-01 08:00', 'Australia/Melbourne')
    .startOf('day')
    .format()
  expect(beforeDST).toBe('2021-04-01T00:00:00+11:00')
  const afterDST = dayjs
    .tz('2021-04-15 08:00', 'Australia/Melbourne')
    .startOf('day')
    .format()
  expect(afterDST).toBe('2021-04-15T00:00:00+11:00')
})

And the output of the second expect comes out like this:

2021-04-15T00:00:00+10:00

Note that the offset is +10:00 and not +11:00

iamkun pushed a commit that referenced this pull request May 26, 2021
## [1.10.5](v1.10.4...v1.10.5) (2021-05-26)

### Bug Fixes

* add meridiem in ar locales ([#1375](#1375)) ([319f616](319f616))
* Added Zulu support to customParseFormat ([#1359](#1359)) ([1138a3f](1138a3f))
* fix Bengali [bn] locale monthsShort error ([a0e6c0c](a0e6c0c))
* fix missing types for ArraySupport plugin ([#1401](#1401)) ([b1abdc4](b1abdc4))
* fix Ukrainian [uk] locale ([#1463](#1463)) ([0fdac93](0fdac93))
* hotfix for `Duration` types ([#1357](#1357)) ([855b7b3](855b7b3)), closes [#1354](#1354)
* timezone plugin DST error ([#1352](#1352)) ([71bed15](71bed15))
* Update duration plugin change string to number ([#1394](#1394)) ([e1546d1](e1546d1))
* update Duration plugin to support no-argument ([#1400](#1400)) ([8d9a5ae](8d9a5ae))
* Update Finnish [fi] locale to set yearStart  ([#1378](#1378)) ([f3370bd](f3370bd))
* update Russian [ru] locale meridiem and unit tests ([#1403](#1403)) ([f10f39d](f10f39d))
* update Russian [ru] locale yearStart config  ([#1372](#1372)) ([5052515](5052515))
* update Slovenian [sl] locale to set correct ordinal  ([#1386](#1386)) ([cb4f746](cb4f746))
* update Spanish [es] locale to change month names to lowercase ([#1414](#1414)) ([9c20e77](9c20e77))
* update Swedish [sv] locale to set correct yearStart ([#1385](#1385)) ([66c5935](66c5935))
* update UTC plugin to support string argument like +HH:mm ([#1395](#1395)) ([656127c](656127c))
@iamkun
Copy link
Owner

iamkun commented May 26, 2021

🎉 This PR is included in version 1.10.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@henhal
Copy link

henhal commented Feb 4, 2022

This commit seems to be the reason for quite a few problems, including #1795 , which seems to be similar to what @ashconnell reported almost a year ago. This is really critical.

@kevinsimper
Copy link

I think this also gives problems
Here you can see a dayjs object

    M {
      '$L': 'en',
      '$offset': 60,
      '$d': 2021-01-04T08:00:00.000Z,
      '$x': { '$timezone': 'Europe/Stockholm' },
      '$y': 2021,
      '$M': 0,
      '$D': 4,
      '$W': 1,
      '$H': 9,
      '$m': 0,
      '$s': 0,
      '$ms': 0
    } 

and then running toISOString() gives this output now

2021-01-04T09:00:00.000Z

I upgraded from 1.10.4 to 1.11.0

This is the result in 1.10.4

      d {
        '$L': 'en',
        '$offset': 120,
        '$d': 2021-01-04T08:00:00.000Z,
        '$x': { '$timezone': 'Europe/Stockholm' },
        '$y': 2021,
        '$M': 0,
        '$D': 4,
        '$W': 1,
        '$H': 9,
        '$m': 0,
        '$s': 0,
        '$ms': 0
      } 
2021-01-04T08:00:00.000Z

andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.10.5](iamkun/dayjs@v1.10.4...v1.10.5) (2021-05-26)

### Bug Fixes

* add meridiem in ar locales ([#1375](iamkun/dayjs#1375)) ([319f616](iamkun/dayjs@319f616))
* Added Zulu support to customParseFormat ([#1359](iamkun/dayjs#1359)) ([1138a3f](iamkun/dayjs@1138a3f))
* fix Bengali [bn] locale monthsShort error ([a0e6c0c](iamkun/dayjs@a0e6c0c))
* fix missing types for ArraySupport plugin ([#1401](iamkun/dayjs#1401)) ([b1abdc4](iamkun/dayjs@b1abdc4))
* fix Ukrainian [uk] locale ([#1463](iamkun/dayjs#1463)) ([0fdac93](iamkun/dayjs@0fdac93))
* hotfix for `Duration` types ([#1357](iamkun/dayjs#1357)) ([855b7b3](iamkun/dayjs@855b7b3)), closes [#1354](iamkun/dayjs#1354)
* timezone plugin DST error ([#1352](iamkun/dayjs#1352)) ([71bed15](iamkun/dayjs@71bed15))
* Update duration plugin change string to number ([#1394](iamkun/dayjs#1394)) ([e1546d1](iamkun/dayjs@e1546d1))
* update Duration plugin to support no-argument ([#1400](iamkun/dayjs#1400)) ([8d9a5ae](iamkun/dayjs@8d9a5ae))
* Update Finnish [fi] locale to set yearStart  ([#1378](iamkun/dayjs#1378)) ([f3370bd](iamkun/dayjs@f3370bd))
* update Russian [ru] locale meridiem and unit tests ([#1403](iamkun/dayjs#1403)) ([f10f39d](iamkun/dayjs@f10f39d))
* update Russian [ru] locale yearStart config  ([#1372](iamkun/dayjs#1372)) ([5052515](iamkun/dayjs@5052515))
* update Slovenian [sl] locale to set correct ordinal  ([#1386](iamkun/dayjs#1386)) ([cb4f746](iamkun/dayjs@cb4f746))
* update Spanish [es] locale to change month names to lowercase ([#1414](iamkun/dayjs#1414)) ([9c20e77](iamkun/dayjs@9c20e77))
* update Swedish [sv] locale to set correct yearStart ([#1385](iamkun/dayjs#1385)) ([66c5935](iamkun/dayjs@66c5935))
* update UTC plugin to support string argument like +HH:mm ([#1395](iamkun/dayjs#1395)) ([656127c](iamkun/dayjs@656127c))
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.10.5](iamkun/dayjs@v1.10.4...v1.10.5) (2021-05-26)

### Bug Fixes

* add meridiem in ar locales ([#1375](iamkun/dayjs#1375)) ([319f616](iamkun/dayjs@319f616))
* Added Zulu support to customParseFormat ([#1359](iamkun/dayjs#1359)) ([1138a3f](iamkun/dayjs@1138a3f))
* fix Bengali [bn] locale monthsShort error ([a0e6c0c](iamkun/dayjs@a0e6c0c))
* fix missing types for ArraySupport plugin ([#1401](iamkun/dayjs#1401)) ([b1abdc4](iamkun/dayjs@b1abdc4))
* fix Ukrainian [uk] locale ([#1463](iamkun/dayjs#1463)) ([0fdac93](iamkun/dayjs@0fdac93))
* hotfix for `Duration` types ([#1357](iamkun/dayjs#1357)) ([855b7b3](iamkun/dayjs@855b7b3)), closes [#1354](iamkun/dayjs#1354)
* timezone plugin DST error ([#1352](iamkun/dayjs#1352)) ([71bed15](iamkun/dayjs@71bed15))
* Update duration plugin change string to number ([#1394](iamkun/dayjs#1394)) ([e1546d1](iamkun/dayjs@e1546d1))
* update Duration plugin to support no-argument ([#1400](iamkun/dayjs#1400)) ([8d9a5ae](iamkun/dayjs@8d9a5ae))
* Update Finnish [fi] locale to set yearStart  ([#1378](iamkun/dayjs#1378)) ([f3370bd](iamkun/dayjs@f3370bd))
* update Russian [ru] locale meridiem and unit tests ([#1403](iamkun/dayjs#1403)) ([f10f39d](iamkun/dayjs@f10f39d))
* update Russian [ru] locale yearStart config  ([#1372](iamkun/dayjs#1372)) ([5052515](iamkun/dayjs@5052515))
* update Slovenian [sl] locale to set correct ordinal  ([#1386](iamkun/dayjs#1386)) ([cb4f746](iamkun/dayjs@cb4f746))
* update Spanish [es] locale to change month names to lowercase ([#1414](iamkun/dayjs#1414)) ([9c20e77](iamkun/dayjs@9c20e77))
* update Swedish [sv] locale to set correct yearStart ([#1385](iamkun/dayjs#1385)) ([66c5935](iamkun/dayjs@66c5935))
* update UTC plugin to support string argument like +HH:mm ([#1395](iamkun/dayjs#1395)) ([656127c](iamkun/dayjs@656127c))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants