React-dates: Performance Issues

[AntiFish03]

I am debugging the app I am working on for performance issues and have identified that react-dates might potentially be the biggest culprit of my slow down. Has anyone else run why-did-you-update on react dates and noticed the tons of potentially avoidable re-renders? On hovers, key press events all cause massive amounts of re-renders and none of the prop values have changed.

[majapw]

🤔I'm going to look into this today! I have some progress that I can put into a PR --- it looks like there's some basic stuff we could do by conditionally using pure components and making some changes to CalendarWeek itself. @lencioni has given me a bit of advice there.

[AntiFish03]

@majapw, Is there any movement on this?

[AntiFish03]

@majapw In playing around I have managed to figure out a way to eliminate a lot of the re-rendering warnings (and lag) by using the withStyles option { pureComponent: true }, and using React.PureComponent over React.Component with shouldComponentUpdate lifecycle.

I don't know if this is the direction the library wants to go as it would remove backwards compatibility with React < 15.3.0. But it is a possible remedy to the excessive re-renders that are occurring.

On CalendarDay, CalendarMonth, DayPickerKeyboardShortcuts, DateInput all of the warnings are eliminated.

The following still give a massive number of re-render warnings.
DayPickerNavigation still gives me withStyles(DayPickerNavigation).props: Changes are in functions only. Possibly avoidable re-render? ( This is actually the worst as it still occurs on every hover, blur, click, open / close of the day picker. )

DayPicker still also gives me withStyles(DayPicker).props: Changes are in functions only. Possibly avoidable re-render?

[majapw]

@AntiFish03 you are a hero!

I think that is exactly the direction we want to move in. We can def use { pureComponent: true } in withStyles and can use React.PureComponent as long as we follow a pattern like this (https://github.com/airbnb/react-with-styles/blob/master/src/withStyles.jsx#L23-L33) with fall-backs (we'll be able to just go all in once React 17 comes out).

I'd love to accept a PR with those changes and we can dig further into the DayPickerNav and DayPicker itself.

[ljharb]

I'm not sure we should be using PureComponent just yet; we should be able to obtain the same speed improvements with a shouldComponentUpdate. Even when React 17 comes out, v15.0 - v15.2 won't have PureComponent, so it's not truly safe to use until v18 is released (given our "last 2 versions" policy).

The specific changes sound good, of course.

[majapw]

I'm not sure we should be using PureComponent just yet; we should be able to obtain the same speed improvements with a shouldComponentUpdate. Even when React 17 comes out, v15.0 - v15.2 won't have PureComponent, so it's not truly safe to use until v18 is released (given our "last 2 versions" policy).

This is mitigated by the link I shared as it has a fallback for when PureComponent doesn't exist! :) So even if we don't remove that, we should be able to use it. This is something I discussed with @lencioni earlier and should address your concerns about supporting back several versions.

[ljharb]

Woudnt that mean tho that converting a component with a sCU to pureComponent, in older reacts, would lack sCU entirely?

[majapw]

@lencioni can weigh in, but I don't... think so? but if we do lose the sCU optimization on older Reacts, I think that's a fine compromise for how huge the gains are for using PureComponent.

[ljharb]

That seems to be punishing older React users, for speed ups that we should be able to obtain without PureComponent (with our own cDU).

[lencioni]

That's right, people on older versions of React will have worse performance with this approach. I think that's probably alright though. PureComponent has been part of React for 2 years now, and things will still work for folks who are on older versions--or they can keep using an older version of react-dates.

If we want to avoid this, we could implement our own shouldComponentUpdate, or our own PureComponent class that we can extend. However, if we avoid using withStyles pureComponent option, then we will be punishing newer React users instead, which doesn't seem like the right thing to do.

[ljharb]

I'd be fine with using the pureComponent option conditionally as well as extending PureComponent internally, only when it's present, as long as we can conditionally include a shouldComponentUpdate method as well for older react users.

See #851 / #313 / #565, and https://github.com/airbnb/react-dates/pull/565#issuecomment-330578117 in particular.

[majapw]

Again, as has been said many times over in this thread, I think that we can get the performance gains from PureComponent without dropping support for older versions of react in the same way that we do in react-with-styles (which conditionally checks for its presence).

As for

as long as we can conditionally include a shouldComponentUpdate method as well for older react users.
I don't personally think that this is necessary, but I'm happy to leave the shouldComponentUpdate as is in the classes that currently have it.

@AntiFish03 if you would like to spearhead these efforts with these points in mind, I think that would be super helpful! :)

[AntiFish03]

@majapw Would you like me to leave the change to storybook config to include why-did-you-update in? It really helps to find potential performance issues.

[majapw]

That sounds awesome! :)

[AntiFish03]

@majapw Can you see if you can figure out why DayPickerNavigation is still giving a ton of re-render warnings? I can't get a handle on the issue with that one and it probably something stupidly simple that I am not seeing.

[majapw]

I will take a look at that this week. :)

[majapw]

Okay! The DayPickerNavigation rerender is fixed in v18.1.0.

There are still some issues with unnecessary CalendarDay rerenders which I will look into! I'll close this issue in the meantime since the original point is addressed.

[cgriego]

Awesome!

Is there an issue to track the CalendarDay rerender?

[majapw]

I will make one!