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

utc.valueOf doesn't account for DST correctly #1804

Closed
janzipek opened this issue Feb 11, 2022 · 2 comments
Closed

utc.valueOf doesn't account for DST correctly #1804

janzipek opened this issue Feb 11, 2022 · 2 comments

Comments

@janzipek
Copy link

Describe the bug
The utc.valueOf method doesn't correctly account for DST when $x.$localOffset is not set:

const addedOffset = !this.$utils().u(this.$offset)
? this.$offset + (this.$x.$localOffset || (new Date()).getTimezoneOffset()) : 0

the issue is it creates new Date object and retrieves the browser timezone offset from that, but if the user is in DST and the date that's being casted to timestamp is not (or vice-versa), the result is incorrect.

Minimal repro:

/*
 Expected system date settings:
 TIMEZONE: Europe/Prague (+1)
 DATE: 2022-02-11
*/

const d = dayjs('2014-06-01T12:00:00Z').tz('America/Los_Angeles')
console.log(d.format()) // correct - 2014-06-01T05:00:00-07:00
console.log(new Date(d.valueOf()).toISOString()) // incorrect 2014-06-01T11:00:00.000Z, expected 2014-06-01T12:00:00.000Z
console.log(d.valueOf()) // incorrect 1401620400000, expected 1401624000000

Expected behavior
The browser timezone offset should be calculated from the date itself, not new instance. Something like this:

  proto.valueOf = function () {
    const addedOffset = !this.$utils().u(this.$offset)
      ? this.$offset + (this.$x.$localOffset || this.$d.getTimezoneOffset()) : 0
    return this.$d.valueOf() - (addedOffset * MILLISECONDS_A_MINUTE)
  }

Information

  • Day.js Version v1.10.7
@xvaara
Copy link
Contributor

xvaara commented Mar 19, 2022

there is a pull request to fix this #1448 and there is a plugin in the pulls conversation to fix this while we wait.
but since it's a year old and not merged (even though it's a big problem), one has to wonder where to focus of this project is.

@iamkun
Copy link
Owner

iamkun commented May 3, 2022

fixed in #1448

@iamkun iamkun closed this as completed May 3, 2022
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

Successfully merging a pull request may close this issue.

3 participants