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.29k stars 514 forks source link

Fetching from cache ignores milliseconds when comparing keys #489

Open hgrsd opened 2 years ago

hgrsd commented 2 years ago

Hello 👋

I noticed a very edge-case-y bug where I was having an occurrence generated from an RRule that was unexpected given the bounds passed to the between method. The returned occurrence was milliseconds out of the request bounds.

Reproduction

const rrule = rrulestr('DTSTART:20211118T090000Z\nRRULE:FREQ=DAILY;BYHOUR=9');
const occurrences_1 = rrule.between(
    new Date('2021-11-18T09:00:00.000Z'),
    new Date('2021-11-20T09:00:00.000Z'),
    true
); // ['2021-11-18T09:00:00.000Z', '2021-11-19T09:00:00.000Z', '2021-11-20T09:00:00.000Z']

const occurrences_2 = rrule.between(
    new Date('2021-11-18T09:00:00.001Z'), // note: starts 1ms after the 9am occurrence on 2021-11-18
    new Date('2021-11-20T09:00:00.000Z'),
    true
); // ['2021-11-18T09:00:00.000Z' <- returned unexpectedly, '2021-11-19T09:00:00.000Z', '2021-11-20T09:00:00.000Z']

Cause

After doing a brief investigation, this seems to be caused by the comparison strategy used in the cache. The arguments used for the request are compared to the arguments that have been used to insert dates into the cache using

String(left) === String(right)

Unfortunately:

String(new Date('2021-01-01T00:00:00.000+00:00')) 
  === String(new Date('2021-01-01T00:00:00.001+00:00')) // true, despite the 1ms difference between dates
/**
 * The node REPL shows:
 * > String(new Date('2021-01-01T00:00:00.000+00:00'))
 *   'Fri Jan 01 2021 00:00:00 GMT+0000 (Greenwich Mean Time)' (ignoring milliseconds).
 */

I've opened a pull request in #490 to address this - let me know if you agree with the approach!