fix: between() optimization may cause earlier event occurrences to be skipped#103
Merged
ggaabe merged 4 commits intoggaabe:mainfrom Jan 28, 2026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Currently, to reduce the amount of iteration done in
between()(the method which gets recurrences of an eventaftera specified date/time andbeforeanother), the originalRRuleTemporalinstance is re-created with an optimizeddtstartthat is closer to the providedafterdate, and iteration is done with this re-createdRRuleTemporalinstance starting from the newdtstartinstead.Problem
The calculation to determine the new, optimized
dtstartmay jump too far ahead, resulting in some event recurrences potentially being skipped.Specifically, to calculate the optimized
dtstart, the library takes the originaldtstartdate and jumps forward by the event frequencyinterval(e.g., every 1 day) until the newdtstartlies within the same frequency interval (e.g., the same day) asafter.The problem lies in the fact that
dtstartmight actually occur relatively late within the frequency interval (specifically, I ran into this issue with daily recurring events — e.g.,dtstarton a daily event could have a very late time attached to it), but the actual event might recur sometime earlier within the frequency interval (e.g., in the morning). If the passedafterparameter is also on the earlier side of the frequency window, using the later, optimizeddtstartfor calculations will result in missed recurrences — specifically, those that are scheduled to occur between the time of theafterparameter and the time of the optimizeddtstart.Example / Bug Walkthrough
Let's say
dtstartis 7:00pm on January 25th, 2026 (rather late in the day), and our event recursDAILY, every day (intervalis 1) at 10am, 2pm, and 8pm (byHour: [10, 14, 20]). Now we call thebetweenmethod with theafterparameter set to 9:00am on January 27th, 2026, about two days afterdtstartbut relatively earlier in the day.First (see the original
between()code for variable references), thebetweenmethod convertsaftertoaligned, which isafterwith the same date as it had originally, but with the time updated to match that ofdtstart. So nowalignedis 7:00pm (the time ofdtstart) on January 27th (date is unchanged). Then the method calculates the difference betweendtstartandaligned, which is now 2 full days (unitsBetween = 2).So then
Math.floor(unitsBetween / interval)is 2, which means to calculate the optimizeddtstart, the method jumps forward two whole days from the originaldtstartto 7:00pm on January 27th. Butafterwas set to 9:00am on that day! This means that the iteration misses the 10am and 2pm event occurrences, even though they are actually after theafterparameter, too.Fix
We need to subtract
1more from the number ofstepswe jump forward from the originaldtstart, because the relative time of the originaldtstartwithin the frequency interval may not actually be before earlier occurrences of the event in the interval, even though theafterparameter could be before those occurrences. By jumping forward one less step (e.g. to the day before theafterdate for a daily event), we run through the entire frequency interval in which theafterparameter is contained, ensuring we catch the earlier occurrences.