onOutsideRange constantly and needlessly recomputes thousands of time

[timhwang21]

It looks like onOutsideRange() is recomputed for EVERY SINGLE DATE every single time the datepicker is interacted with, be it clicking, or even hovering over dates.

My test function:

isOutsideRange(day) {
  console.log(counter++); // counter is a global variable

  return false;
}

The result:

When opening the datepicker for the first time (clicking an input), Chrome debugger logs up to 2519:

When the mouse enters or leaves any calendar date, Chrome debugger logs an additional 560 (18 months worth of days?) calls. Finally, subsequent closing / opening of the datepicker logs an additional 160 calls (560 * 3, significant?)

If the isOutsideRange() function is anything more than trivial, this results in a very noticeable performance slowdown.

Ideally, the result of isOutsideRange() should be cached for each day, or should only be calculated for localized days, e.g. for all the dates currently visible ± 7 days (to account for outside days).

[ljharb]

That caching pattern wouldn't work though if the value needed to change on the same day - ie, let's say after 3PM, "today" becomes invalid.

I think it's up to the creator of isOutsideRange to ensure that it's memoized properly, but I'm open to suggestions.

[timhwang21]

I feel like the tradeoff of not having split second precision for the date picker by doing something like only running recalculation to when the picker updates (e.g. not when you hover over dates) would be worth the performance gain. So if the user idles on the page, and the system time hits 3:01PM, it won't invalidate until the next render cycle?

[ljharb]

Correct - but any user interaction (like hovering) would immediately invalidate it.

The point of a user providing a custom isOutsideRange function is that the user gets full control over whether a day is valid or not - caching it internally can't guarantee that.

[timhwang21]

After looking into the code, it seems that getModifiersForDay() could be changed to not run the day through every single modifier every single time. Currently, it is running every modifier on every action, even modifiers that are not relevant to the action that just occurred. There are some modifiers that depend on UI state (<DayPickerRange />s hoverDate) and others that depend on which days are currently selected.

To me it seems that of the modifiers, some only depend on external props (e.g. blocked) while others depend on user action (e.g. hovered, hovered-span).

So as an example, maybe <CalendarMonth /> can be changed like so:

const hoverModifiersForDay = hoverChanged ? getModifiersForDay(hoverModifiers, day) : cachedHoverModifiers;
const selectModifiersForDay = selectedDatesChanged ? getModifiersForDay(selectModifiers, day);

Where hoverChanged, selectedDatesChanged are boolean props that show why <CalendarMonth /> is rerendering.

Do you think this is viable? If so, I can work on this.

[majapw]

Hi @timhwang21,

I think that #217 should fix at least some of these duplicated calls.

My original thought is that we can't guarantee that isOutsideRange or isDayBlocked don't depend on the hoverDate. If the user is using the DateRangePicker or SingleDatePicker out of the box... then yes, this would be fine, but if we put the logic you suggested into getModifiersForDay (which is in CalendarMonth) we can't guarantee what the parent tree looks like, what the modifiers might be, and if there's any sort of customization we're overlooking.

How would hoverModifiers and selectModifiers be determined? CalendarMonth right now just gets a freeform array of modifiers that could honestly be anything.

If we could make this logic happen in the DayPickerRangeController layer or the SingleDatePicker layer, then I think that would make the most sense.

[timhwang21]

Hi, thanks for the response @majapw ! I'm glad that the 4x render issue is addressed -- that was bothering me as well (though it doesn't nearly impact performance as much).

I did consider the fact that hover state can affect the modifiers. However I'm wondering if there's ever a use case where this would happen. I mean, who would want what dates are selectable to change based on where you're hovering when it comes to a calendar widget?

Speaking purely from personal opinion, I would say that if a user 1. uses hover state to determine date validity, or 2. puts hover logic in modifiers that shouldn't use them, and their widget subsequently breaks (e.g. the validation doesn't run when the user wants it to), this would be working as intended :)

I'm not very familiar with the code base so I don't want to comment with any certainty, but from what I saw, the logic of mapping the modifiers over every single day happens in CalendarMonth, and I can't immediately see any obvious way to move this logic somewhere else.

[sontek]

I'm with @timhwang21 on this one, I don't see any reason to modify the values on hover and the performance improvements would be amazing if we could stop re-calculating all the time :)

Currently react-dates is unusable on iOS and it doesn't have hover so this would probably solve that issue pretty well without hurting anything

[majapw]

v11.0.0 (I swear the last breaking release for a while) rearchitects modifiers so now there's about the same overhead at mount, but then calendar interactions only updates the relevant CalendarDay objects instead of all of them.

You can see more details in #450, but in the meantime, I'm going to close this issue of being "generally slow". There are still some improvements to be made, notably during the month transition phase, but I think this new architecture opens up a lot of doors. :)