Page MenuHomePhabricator

[native] Memoize construction of `sidebars` in `ChatThreadListItem`
ClosedPublic

Authored by atul on Sep 7 2023, 10:51 AM.
Tags
None
Referenced Files
F3399147: D9102.id30851.diff
Mon, Dec 2, 2:04 AM
F3398618: D9102.diff
Sun, Dec 1, 11:41 PM
Unknown Object (File)
Thu, Nov 21, 11:11 AM
Unknown Object (File)
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Thu, Nov 21, 6:50 AM
Unknown Object (File)
Thu, Nov 21, 6:45 AM
Subscribers

Details

Summary

This was getting re-rendered every time which recursively led to ChatThreadListSidebar, etc getting re-rendered every time. This change means ChatThreadListSidebar will render only once (unless something relevant changes).

Before:

014d54.png (1×2 px, 648 KB)

After:

8cb34d.png (1×2 px, 892 KB)


Depends on D9100

Test Plan

Things continue to work as expected.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul published this revision for review.Sep 7 2023, 10:55 AM
tomek added inline comments.
native/chat/chat-thread-list-sidebar.react.js
69–72 ↗(On Diff #30851)

Maybe we can wrap SidebarItem with React.memo?

This revision is now accepted and ready to land.Sep 8 2023, 2:26 AM
native/chat/chat-thread-list-sidebar.react.js
69–72 ↗(On Diff #30851)

7cd38c.png (828×1 px, 113 KB)

SidebarItem was memoized in previous diff

This revision was landed with ongoing or failed builds.Sep 8 2023, 3:05 PM
This revision was automatically updated to reflect the committed changes.
native/chat/chat-thread-list-sidebar.react.js
69–72 ↗(On Diff #30851)

If it is memoized, then what's the benefit of wrapping it with useMemo?