react-slick Forced reflow - Performance bottleneck

Hello akiran,

The slider has been an amazing addition for many projects of ours. Thank you.

We were trying to optimize all our apps using several metrics.

A case in which the slider seems to trigger expensive forced reflow of our layouts which is impacting the performance on a huge scale.

We wanted to know if it is such that this issue can be fixed or our implementation of it is not the optimal in a way.

helpers.js getWidth: function getWidth(elem) { return elem && (elem.getBoundingClientRect().width || elem.offsetWidth) || 0; },

Can this be optimized?

screen shot 2018-06-15 at 11 31 54 am 2

screen shot 2018-06-15 at 11 46 37 am 2

Thank you for your time.

Jun 15 '18 06:06 Jeevan-Kishore

Can you explain bit more

Jun 15 '18 06:06 akiran

#1204 describes the same problem.

Jun 21 '18 15:06 sapkra

Accessing elem.offsetWidth always causes a forced reflow (see https://gist.github.com/paulirish/5d52fb081b3570c81e3a for a list of other properties that causes this). The library needs this property when producing the initializedState

https://github.com/akiran/react-slick/blob/5591aab45d6c9fdc62e25fdfcf81d0c84d60764c/src/utils/innerSliderUtils.js#L102

which is used by the inner slider when state updates are made

https://github.com/akiran/react-slick/blob/be952ad7c8ae400854cc11d668870bf1bd8fa0a8/src/inner-slider.js#L216

updateState is called in a few occasions, but most notably by componentWillReceiveProps. Essentially, any time any of the props of the slider change, a reflow seems to be forced.

I'm not 100% sure, but I think we could optimize this behavior by recycling the listWidth and trackWidth parameters when the state is updated through componentWillReceiveProps in most cases.

Jun 25 '18 06:06 aripekkako

Hello aripekkako, Thanks for the details of it, it seems like recycling listWidth & trackWidth could give us a perf boost. Can we please attempt this at the earliest?

Jun 25 '18 09:06 Jeevan-Kishore

I'm not a maintainer of this repository. I think your best bet is to

make a quick-and-dirty test to see if your performance problems are fixed with the recycling.
If it works, then submit a PR with a proper fix
Hope Akiran will have time to look the PR through and prepare to maintaining your own fork in the meantime.

Jun 25 '18 09:06 aripekkako

I think this won't be the solution because we are talking about a performance problem in componentDidMount and not componentWillReceiveProps. You can see it in the screenshot.

I tried to wrap the content of componentDidMount into a setTimeout function to let offsetWidth and offsetHeight not block the first rendering of the react app.

Jun 27 '18 14:06 sapkra

Oh. Missed that from the screenshots. Getting properly rid of the initial layout forcing componentDidMount falls beyond my expertise, as I'm not that familiar with layout trashing in general.

setTimeout or similar (maybe requestAnimationFrame?) might indeed work. updateState in asynchronous by nature because it only calls React's setState in the end so if you find a working solution for componentDidMount it might be wise to just do it inside updateState so all actions updating the state will be safe from layout trashing.

Jun 28 '18 06:06 aripekkako

^^ @akiran did you get a chance to look at this?

Jul 10 '18 09:07 Jeevan-Kishore

Any more news on this issue?

Sep 04 '18 09:09 aaronkrohn

Any update on a fix?

Oct 12 '18 04:10 justinstander

@justinstander This repo has not been updated for 4 months and last release was 6 months ago. No activity from the maintainer on any issues or PRs during the last 4 months.

Oct 12 '18 05:10 aripekkako

Just tried this component for a project and styling does not work. Haven't noticed repo is not being maintained anymore. Sad I had to dig through issues to find this as a comment.