search

captivationsoftware / react-sticky

<Sticky /> component for awesome React apps
MIT License
2.64k stars 385 forks source link

Improve performance with `shouldComponentUpdate` #230

Closed ntdb closed 6 years ago

ntdb commented 6 years ago

We noticed that the contents of Sticky were getting re-rendered on every single scroll event on the entire page. We weren't taking a huge performance hit but this feels a lot better. All of the examples work as before and tests all pass so I don't think this should introduce any regressions.

ntdb commented 6 years ago

I suspect that the performance benefits will vary widely depending on what is being rendered inside the Sticky component.

This does need some further consideration any way you look at it. I had to revert our use of this change because it caused the contents of Sticky to not be updated when other, unbound props were changed. I'll look for a better solution.

vcarl commented 6 years ago

That was my other concern, other props being passed through has bitten me many times on reusable components that I try to optimize.

vcarl commented 6 years ago

I'm going to close this, since there are a few issues with it that don't have an obvious solution and I don't want to leave things sitting idle for an extended period of time. I'd be happy to discuss further in an issue if you want to make one!