react-dates / react-dates

An easily internationalizable, mobile-friendly datepicker library for the web
https://react-dates.github.io/react-dates/
MIT License
12.23k stars 1.71k forks source link

Performance Issues on iOS #1387

Open AntiFish03 opened 6 years ago

AntiFish03 commented 6 years ago

@majapw & @ljharb, I am continuing to see a huge performance issue with react-date picker in iOS I can even replicate it on airbnb.com in the simulator with an iPhone X. in the screen shot I am attaching the first purple bar in the highlighted area is the tap on the start date, the second LARGE purple bar(starting at 17s continuing to nearly 25s ~8 seconds lag) is when selecting a startDate in the calendar and the endDate works without issue.

screen shot 2018-10-05 at 3 11 47 pm

majapw commented 6 years ago

Is this only in iOS? We don't render any of the DayPickerRangeController tree until the first tap so that's likely causing the issue. I think we're going to set some time aside this quarter to do a bit of an audit and try to cut down the render time.

majapw commented 6 years ago

Sorry that's the first bar, the second lag seems... unnecessary. That I will dig into more immediately.

AntiFish03 commented 6 years ago

Thank you @majapw

AntiFish03 commented 6 years ago

@majapw, Any ideas?

AntiFish03 commented 6 years ago

@majapw Not trying to be a squeaky wheel here, but any chance to look at this?

majapw commented 6 years ago

I'm really sorry! It has been a particularly slammed month, to be honest, in work and elsewhere. I was mulling it over and was particularly interested in the start date => end date focus change. My main guess is that we do a lot of computation in componenWillReceiveProps in the DayPickerRangeController (https://github.com/airbnb/react-dates/blob/master/src/components/DayPickerRangeController.jsx#L450-L461). Two improvements off the bat:

This phrase object rejiggering is probably causing a dramatic amount of rerendering of the entire tree: https://github.com/airbnb/react-dates/blob/master/src/components/DayPickerRangeController.jsx#L450-L461 just to change one particular phrase.

These three could probably be consolidated to only go through the visible days once instead of three times: https://github.com/airbnb/react-dates/blob/master/src/components/DayPickerRangeController.jsx#L377-L416

Gah, I think that starting this week, I will actually start trying to devote at least 5 hours week to react-dates and this will be one of the first things I can work on, but those two are good starting points at least.

AntiFish03 commented 6 years ago

@majapw could that componenWillReceiveProps be refactored to use componentDidUpdate its generally more performant and has less issues with causing multiple renders. And is more compatible with the direction of future versions of React.

diegotsi commented 5 years ago

Any news on that? It's almost impossible to use in Iphones before X.

<DayPickerRangeController numberOfMonths={10} orientation="verticalScrollable" startDate={this.state.startDate} endDate={this.state.endDate} onDatesChange={this.selectDate} focusedInput={this.state.focusedInput} onFocusChange={this.handleFocus} />

diegotsi commented 5 years ago

Hey @majapw, whats the purpose of always recompute everything when focus changed like if (didFocusChange || recomputeOutsideRange) if (didFocusChange || recomputeDayBlocked) if (didFocusChange || recomputeDayHighlighted)

Can't we just use recomputeOutsideRange or recomputeDayBlocked or recomputeDayHighlighted ? Because thats the problem, everytime that focusChange they recomputed everything and if i have for example 12 months displayed on mobile this will cause a poor performance.