-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Inconsistent behaviour for utcOffset
.
#1340
Comments
FYI, it seems the bug is in moment to me. Right now the correct offset for See here: https://www.zeitverschiebung.net/en/timezone/america--new_york |
DST is picked up by import dayjs from 'dayjs';
import tz from 'dayjs/plugin/timezone.js';
import utc from 'dayjs/plugin/utc.js';
dayjs.extend(tz);
dayjs.extend(utc);
console.log(dayjs.tz(new Date('2020-03-29T00:30:00Z').getTime(), 'Europe/Berlin').format(), 'is correct');
console.log(dayjs.tz(new Date('2020-03-29T01:30:00Z').getTime(), 'Europe/Berlin').format(), 'should be 2020-03-29T03:30:00+02:00, because DST kicked in half an hour before'); yields:
|
Not sure your bug is similar to the OPs, unless I'm missing the connection. But I did confirm and figure out what the problem is with your case. It's actually a pretty easy fix (I think). The problemI'm in CST, which has an offset of -5/-6 (depending on DST). Let's use this as an example. Given the input you gave: The code converts to local machines timezone (at this time it was -5 for me). Then it converts to the desired timezone. These are the results: Then these two dates are subtracted (disregard the offset), giving -07:00. Next, we take local machine offset (-5) and subtract from -7. -5 - (-7) = +2. Correct answer. That's how it should work. Let's look at the code and I'll show you the bug. proto.tz = function (timezone = defaultTimezone, keepLocalTime) {
const oldOffset = this.utcOffset()
const target = this.toDate().toLocaleString('en-US', { timeZone: timezone })
const diff = Math.round((this.toDate() - new Date(target)) / 1000 / 60)
let ins = d(target).$set(MS, this.$ms).utcOffset(localUtcOffset - diff, true)
if (keepLocalTime) {
const newOffset = ins.utcOffset()
ins = ins.add(oldOffset - newOffset, MIN)
}
ins.$x.$timezone = timezone
return ins
} Source: https://github.com/iamkun/dayjs/blob/dev/src/plugin/timezone/index.js#L101 Check out that it takes const localUtcOffset = d().utcOffset() Source: https://github.com/iamkun/dayjs/blob/dev/src/plugin/timezone/index.js#L39 The fixReplace What you should doI haven't tested this but I think it'll work. If that works for you, I'd encourage you to submit a PR and fix this. When was the bug introducedBug has existed ever since the timezone plugin was added. |
Thanks!
I'll do as you have suggested, but I won't find the time for it today.
|
The proposed works for utcOffset when starting from
Have you meant changing let ins = d(target).$set(MS, this.$ms).utcOffset(localUtcOffset - diff, true) to let ins = d(target).$set(MS, this.$ms).utcOffset(this.utcOffset() - diff, true) or have you had another line in mind? Do you directly see the cause of this or would you have to dig deeper as well? |
No you changed the code correctly. Good catch. I think I know the problem. Try the below snippet. Basically, the issue is that Another thing is I'm not sure that proto.tz = function (timezone = defaultTimezone, keepLocalTime) {
const oldOffset = this.utcOffset()
const date = this.toDate();
const target = date.toLocaleString('en-US', { timeZone: timezone })
const diff = Math.round((date - new Date(target)) / 1000 / 60)
let ins = d(target).$set(MS, this.$ms).utcOffset((-Math.round(date.getTimezoneOffset() / 15) * 15) - diff, true)
if (keepLocalTime) {
const newOffset = ins.utcOffset()
ins = ins.add(oldOffset - newOffset, MIN)
}
ins.$x.$timezone = timezone
return ins
} |
fixed in #1352 |
Describe the bug
Inconsistent behaviour for
utcOffset
. Probably issue with Daylight Saving Time.Expected behavior
Should be
-4
as in the row below.Information
The text was updated successfully, but these errors were encountered: