Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Masonry: Scroll throttle + memo component (MasonryV2) #3533

Merged
merged 1 commit into from Apr 22, 2024

Conversation

liuyenwei
Copy link
Contributor

@liuyenwei liuyenwei commented Apr 19, 2024

Summary

This PR applies two more optimizations to MasonryV2: scroll throttle and memoizing each masonry item that we render

What changed?

Scroll Throttle

I realized that one change I did not carry over from current Masonry is throttling the callback of the scroll event listener, which means that we're currently triggering a render every time scroll position changes (which is a lot). This PR reintroduces the same throttle that was currently use in control Masonry.

MasonryItem + Memo

I was doing some investigation into React's startTransition API and noticed that it actually causes React to render more. A github thread I came across which was particularly interesting was : facebook/react#24269. Based on this, one recommendation to reduce the impact from rendering more was to memoize the component that is expected to change (as a result of the transition).

In Masonry's case, this would be the actual grid items that we render so I moved this out into a separate MasonryItem component and wrapped it in a React.memo.

function MasonryItem() {}
const MasonryItemMemo = memo(MasonryItem)

This is effectively the functional equivalent of React's PureComponent class where React will skip re-rendering if props have not changed.

Testing

I did quite a bit of testing and the results are promising. To simulate rendering a more expensive grid item, I updated ExampleGridItem to recursively render a deeply nested tree.

Initial Load

The main impact to initial load is that in control, all renders take about the same time whereas in V2, the render time falls off significantly after the first two renders.

V1

image

V2

image

Scrolling

This is where the impact really shows. During scroll, we can see that in both V1 and V2 (without changes), every grid item is repeatedly rendered. With the changes in this PR, however, only a small subset of grid items are rendered which reduces render item significantly (50ms per render to ~5ms)

V1

image

V2 (without change)

image

V2 (with change)

image

Additionally, we also see significantly less long tasks and scroll jank with this change

V1

image

V2 (without change)

image

V2 (with change)

image

@liuyenwei liuyenwei requested a review from a team as a code owner April 19, 2024 23:17
Copy link

netlify bot commented Apr 19, 2024

Deploy Preview for gestalt ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ea05643
🔍 Latest deploy log https://app.netlify.com/sites/gestalt/deploys/6622fb766e3bcf000798c92a
😎 Deploy Preview https://deploy-preview-3533--gestalt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@liuyenwei liuyenwei added the patch release Patch release label Apr 20, 2024
@liuyenwei liuyenwei enabled auto-merge (squash) April 20, 2024 08:36
@liuyenwei liuyenwei merged commit 2d9da7e into pinterest:master Apr 22, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch release Patch release
Projects
None yet
3 participants