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

BYDAY returns wrong days (based on README code) #556

Open raniglas opened 1 year ago

raniglas commented 1 year ago

rrule returns the wrong day when using BYWEEKDAY - returns Sunday instead of Monday specified in BYWEEKDAY, likely caused by timezone issue.

I used the following code, just changed the time in DTSTART from 19 to 01 to induce the timezone issue.

const rule = RRule.fromString(
  "DTSTART;TZID=America/Denver:20220925T010000;\n"
  + "RRULE:FREQ=WEEKLY;BYDAY=MO,WE,TH;INTERVAL=1;COUNT=3"
)
rule.all()

it returned (when run in America/Los_Angeles local timezone):

[Sun Sep 25 2022 17:00:00 GMT-0700 (Pacific Daylight Time), 
 Tue Sep 27 2022 17:00:00 GMT-0700 (Pacific Daylight Time), 
 Wed Sep 28 2022 17:00:00 GMT-0700 (Pacific Daylight Time)]

when the expected result is (when run in America/Los_Angeles local timezone):

[Mon Sep 26 2022 17:00:00 GMT-0700 (Pacific Daylight Time), 
 Wed Sep 28 2022 17:00:00 GMT-0700 (Pacific Daylight Time), 
 Thu Sep 29 2022 17:00:00 GMT-0700 (Pacific Daylight Time)]

since BYDAY=MO,WE,TH

BTW, running this in python-dateutil returns the correct results.

Any idea how to solve this issue?

FallenMax commented 1 year ago

tl;dr: convert local date to UTC time before passing to rrule.js, and vice versa.

Someone please correct me if I'm wrong, but this is my current understanding and solution.

I think this is the problem Section: Important: Use UTC dates states (somewhat confusing to me).

Let's say we want "every Monday 6:00 local time in Asia/Shanghai", we might be tempted to use:

// WRONG CODE:
// Sat Oct 01 2022 06:00:00 GMT+0800
const localSixAM = new Date('2022-09-30T22:00:00.000Z')
const rrule = new RRule({
  dtstart: localSixAM,
  freq: RRule.WEEKLY,
  byweekday: RRule.MO,
  count: 3,
})
const occurrences = rrule.all()

// results (not as expected):
[
  // Why Tuesday here??
  'Tue Oct 04 2022 06:00:00 GMT+0800 (China Standard Time)',
  'Tue Oct 11 2022 06:00:00 GMT+0800 (China Standard Time)',
  'Tue Oct 18 2022 06:00:00 GMT+0800 (China Standard Time)'
]

The code above will generate Tuesdays instead of Mondays. The reason is that rrule.js uses UTC time to do all those computations internally, including generating weekdays in year. So it effectively treat RRule.MO as "Monday in UTC time", that messes thing up.

So to get Mondays we desired, we need to adjust local Monday to UTC Monday (Monday for people in Greenwich, London), then let rrule.js to its job to give us repeated UTC Mondays, then we convert UTC Mondays back to local Monday. It's kinda confusing, just see figure below.

rrule_timezone

So here is the adjusted code:

// FIXED CODE

// +8 timezone -> -480 (min)
const tzOffset = new Date().getTimezoneOffset()

// local Monday to utc Monday
const toRRuleInput = (date: Date): Date => {
  return addMinutes(date, -tzOffset)  // using date-fn lib here
}
// reverse
const fromRRuleOutput = (date: Date): Date => {
  return addMinutes(date, tzOffset)
}

//  Sat Oct 01 2022 06:00:00 GMT+0800
const localSixAM = new Date('2022-09-30T22:00:00.000Z')
const rrule = new RRule({
  dtstart: toRRuleInput(localSixAM),  // <-- note
  freq: RRule.WEEKLY,
  byweekday: RRule.MO,
  count: 3,
})
const occurrences = rrule.all().map(fromRRuleOutput)   // <-- note

// results (as expected)
[
  'Mon Oct 03 2022 06:00:00 GMT+0800 (China Standard Time)',
  'Mon Oct 10 2022 06:00:00 GMT+0800 (China Standard Time)',
  'Mon Oct 17 2022 06:00:00 GMT+0800 (China Standard Time)'
]

So in short, any local date should be converted to/from UTC in rrule.js input/output, including rrule.between() etc.

raniglas commented 1 year ago

Thank you for the very elaborate response.

Amazingly enough, if you specify the dtstart in local time and include the tzid, it works great - it generates the events on the correct day. But, if you specify the dtstart on UTC, the events end up on the wrong day local time.

The big issue here is that it generates the event at the correct time but wrong day - if it was just shifted it would have been possible to fix this by just shifting local time - but it's "double broken".

Having said all that, if you shift the entire code base to use local timezone with TZID, then rrule.js has a different bug 🤦‍♂️ where it created the UNTIL in the local timezone, whereas the standard requires it to always be UTC (with a 'Z' at the end). So, any way you try to get around this, it doesn't work 'out of the box'.

I was able to hack the UNTIL response with a regex on the string result of rrule.js but as it stands, rrule.js has 2 bugs that block each other 😕