Closed rodgobbi closed 2 weeks ago
Interesting issue. Thanks for the report and for being open to discuss it before starting the hard work :)
It's iterating per day within the selected date range, which causes the issue when the date range is too long.
At first glance, that loop should be limited to the "display months" visible in the calendar. Why does it run over the whole selection? 🤔 If a day is selected but it is not visible in the calendar, it should be ignored.
It might not be easy to implement, but you can determine which months are displayed by using the months
prop returned by the useCalendar
hook. In this call to useSelection
, you could pass a displayMonths
argument (this should be the second argument, before dateLib
) with the value of calendar.months
.
Then, pass this value to the useRange
hook and try to filter out from the loop all the days that are selected but do not belong to one of the displayMonths
.
(not sure if all this would work - not in front of the code right now - so please feel free to try more alternatives!)
@gpbl Thanks for the encouraging message!
Let me give more context from what I understood.
The logic checks if there are any disabled
dates within the selected date range (and only does any action if excludeDisabled
is true
), if it would only consider the visible months in the calendar, it could miss disabled dates that are not currently visible but could still be within the selected date range.
E.g.: if the calendar is showing May, and only May, and the currently selected from
date is in January, if there are any dates disabled in February, it will not be displayed in the calendar, but those disabled dates will be within the date range if any day is selected in May for the to
date of the date range.
Does that make sense?
If I missed something let me know, I'm assuming you are not referring to the startMonth
and endMonth
, which disable part of the calendar and limit the available months, but referring to the visible months based on the user navigating the months in the calendar.
If I understood correctly, we must consider the whole selected date range in the logic, but I'm confident we can optimize it, I'll work later when I have time and prepare a MR.
If I understood correctly, we must consider the whole selected date range in the logic, but I'm confident we can optimize it, I'll work later when I have time and prepare a MR.
Yes, I think I misunderstood the code and wrote my own. :D If you find a way to optimize that logic, please open a PR! I plan to look at this more closely during the weekend.
When using the
react-day-picker
component to select a date range, if the selected date range is too long, the UI takes too long to respond after selecting a new date.How long the date range needs to be to cause this issue depends on the computing power of the device rendering the UI.
In the current project I work on, we use the
react-day-picker
together with a date input where users can type any date, and we were able to easily test a long date range and found the performance issue. The screen recording below shows the response time of the UI:https://github.com/user-attachments/assets/0660659b-5abd-493b-b283-bca3b6674901
The date range tested above can be rare to be ever used in a real application, but I tested a much shorter date range with CPU slowdown of 6x, the response delay is already noticeable as showed in the screen recording below:
https://github.com/user-attachments/assets/85124535-4d23-4cc3-84b1-b4a5b52310b7
Code
Expected Behavior
The UI should respond in less than 100 ms when selecting a new date no matter how long the date range is.
Actual Behavior
The UI takes a noticeable amount of time to respond after selecting a new date if the selected date range is too long.
Screenshots
Besides the screen recordings above, I ran the Chrome profiler to debug the issue, the screenshots below show the issue in the profiler:
By zooming in we can see the following:
Notes
By investigating the source code with the information from the profiler, the cause of the issue seems to be this logic in the useRange hook.
It's iterating per day within the selected date range, which causes the issue when the date range is too long.
I thought about how to improve this logic and fix the performance issue, I can gladly open a PR with the necessary changes, please let me know if I can go ahead and try to fix it. (I wasn't sure if I should open the PR right away or report the issue first, so I decided to report the issue first)