jkbrzt / rrule

JavaScript library for working with recurrence rules for calendar dates as defined in the iCalendar RFC and more.
https://jkbrzt.github.io/rrule
Other
3.27k stars 507 forks source link

Recurrence series changes time of day in time zone after Daylight Savings #550

Closed DevonStern closed 1 year ago

DevonStern commented 2 years ago

I have tried various configurations for the rrule as indicated in the documentation here and here. It says the time of day should be the same across Daylight Savings, but that is not what I am seeing.

Code sample

const rrule = new RRule({
  freq: RRule.DAILY,
  dtstart: new Date(Date.UTC(2022, 7, 18, 12, 0, 0)),
  // tzid: 'America/Denver', // output is the same whether this is included or not
})
const datetimes = rrule.between(
  new Date('2022-10-31'),
  new Date('2022-11-10')
)

Try out the CodeSandbox. Should get similar results as long as you are in a time zone that has Daylight Savings and the between range includes a change in Daylight Savings.

Expected output

The time of day in America/Denver time zone should not change after Daylight Savings (i.e. recurrence should account for Daylight Savings):

Mon Oct 31 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Tue Nov 01 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Wed Nov 02 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Thu Nov 03 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Fri Nov 04 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Sat Nov 05 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Sun Nov 06 2022 06:00:00 GMT-0700 (Mountain Standard Time) <-- Daylight savings change
Mon Nov 07 2022 06:00:00 GMT-0700 (Mountain Standard Time)
Tue Nov 08 2022 06:00:00 GMT-0700 (Mountain Standard Time)
Wed Nov 09 2022 06:00:00 GMT-0700 (Mountain Standard Time)
                ^^

Actual output

The time of day in America/Denver time zone is changing from 6:00 to 5:00:

Mon Oct 31 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Tue Nov 01 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Wed Nov 02 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Thu Nov 03 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Fri Nov 04 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Sat Nov 05 2022 06:00:00 GMT-0600 (Mountain Daylight Time)
Sun Nov 06 2022 05:00:00 GMT-0700 (Mountain Standard Time) <-- Daylight savings change
Mon Nov 07 2022 05:00:00 GMT-0700 (Mountain Standard Time)
Tue Nov 08 2022 05:00:00 GMT-0700 (Mountain Standard Time)
Wed Nov 09 2022 05:00:00 GMT-0700 (Mountain Standard Time)
                ^^

Info

Version: rrule 2.7.1 (also tested with 2.4.1 and issue occurred) OS: Windows 10 Pro, version 21H1 Browser: Chrome 104.0.5112.81 (replicated in Firefox) Time zone: America/Denver

kmaic commented 1 year ago

Hi there, I am facing the same issue. Any workaround?

dereekb commented 1 year ago

I recall running into this issue when refactoring for the 2.7.1 update. It has the correct output when using GMT.

Screen Shot 2022-09-19 at 12 26 57 PM

I don't use the built-in "tzid" field and conversions (since historically rrule hasn't particularly recommended its usage), but I was running into this myself when adding 1 day using date-util's addDay function, which took into account the mdt/mst change and gave the wrong offset. If I recall correctly, in order to properly compute the time I needed to compute the array of date times using GMT+0/UTC, then convert afterwards.

The reason you do this is because 6PM UTC is always 6PM UTC, whereas 4PM MDT is not the same in UTC as 4PM MST is in UTC. That's where the 1 hour difference/offset comes from. The way you want to do the calculation is to calculate everything it UTC, then just "replace" UTC with your intended timezone by adding the appropriate offset.

For my project I've got a .spec file with some notes written on that since I had run into it while testing my own time zone conversions:

https://github.com/dereekb/dbx-components/blob/2afe2bd9638f47ce87e387f0fb329c5ba9327d34/packages/date/src/lib/rrule/date.rrule.spec.ts

Without looking at or remembering any of the rrule tzid code, my guess is the built-in timezone support may not be working in GMT then doing each conversion, which would make the time off by 1 hour after the date change.

michaelKaefer commented 1 year ago

I had to dig very deep and surprisingly it seems like there is no bug. If you change your CodeSandbox to the following then it should work:

import { RRule } from "rrule";
import "./styles.css";

const rrule = new RRule({
  freq: RRule.DAILY,
  dtstart: new Date(Date.UTC(2022, 7 - 1, 18, 12, 0)), // See note 1
  tzid: 'America/Denver',
})
const datetimes = rrule.between(
  new Date('2022-10-31'),
  new Date('2022-11-10')
)
const output = datetimes.map((d) => new Date( // See note 2
  d.getUTCFullYear(),
  d.getUTCMonth(),
  d.getUTCDate(),
  d.getUTCHours(),
  d.getUTCMinutes(),
)).join('<br/>')
document.getElementById("app").innerHTML = output;

Notes:

  1. The docs at https://github.com/jakubroztocil/rrule#timezone-support state that using new Date(2022, 7, ...) will produce unexpected timezone offsets. Instead use rrule's datetime function. Since this function currently is not working (see https://github.com/jakubroztocil/rrule/issues/551) we have to use new Date(Date.UTC(2022, 7 - 1, ...)) (which is excatly what the datetime function is doing).
  2. The result will be a date in UTC which you have to interpret as a date in the timezone you specified in tzid. It seems very weird but it is stated in the docs at https://github.com/jakubroztocil/rrule#important-use-utc-dates that: Even though the given offset is Z (UTC), these are local times, not UTC times. Just to emphasize this my code example transforms the UTC dates to America/Denver dates by creating a new instance of Date using the UTC values (only works if your operating's system timezone is America/Denver).

@DevonStern Can you confirm that I'm right?

dereekb commented 1 year ago

That's correct. There's not really a "bug" but confusion on how many of us interpret it as how it should work, ultimately why I suggest avoiding the usage of TZID in rrule directly.

What you're talking about with local times and not UTC times is what I was explaining (or attempting to) above.

michaelKaefer commented 1 year ago

@dereekb Ok, thanks for clarification. - But I think there is no problem with using tzid when needed IMO.

DevonStern commented 1 year ago

Thank you for the clarification, @michaelKaefer! There are pieces of that in the documentation but it doesn't put it all together into a cohesive example like you did. Now I can get it to work as expected. It's a little bit weird, but I'll take weird and working over not working. :)