Page MenuHomePhabricator
Differential D9102

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

Authored by atul on Sep 7 2023, 10:51 AM.
Tags
None
Referenced Files
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
Unknown Object (File)
Fri, Nov 8, 3:39 PM
View All 75 Files
Subscribers
ashoat

Details

Reviewers
ginsu
rohan
tomek
Commits
rCOMMe0f8b65e0b18: [native] Memoize construction of `sidebars` in `ChatThreadListItem`
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
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Sep 7 2023, 10:51 AM
Herald added a subscriber: ashoat. · View Herald TranscriptSep 7 2023, 10:51 AM
atul edited the summary of this revision. (Show Details)Sep 7 2023, 10:52 AM
atul published this revision for review.Sep 7 2023, 10:55 AM
Harbormaster completed remote builds in B22435: Diff 30851.Sep 7 2023, 11:09 AM
atul added a child revision: D9103: [native] Memoize `swipeableThread` in `ChatThreadListItem`.Sep 7 2023, 11:22 AM
tomek accepted this revision.Sep 8 2023, 2:26 AM
tomek added inline comments.
native/chat/chat-thread-list-sidebar.react.js
69–72

Maybe we can wrap SidebarItem with React.memo?

This revision is now accepted and ready to land.Sep 8 2023, 2:26 AM
atul added inline comments.Sep 8 2023, 11:03 AM
native/chat/chat-thread-list-sidebar.react.js
69–72

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

SidebarItem was memoized in previous diff

atul attached a referenced file: F748780: 7cd38c.png. (Show Details)Sep 8 2023, 11:04 AM
atul updated this revision to Diff 30881.Sep 8 2023, 3:02 PM

rebase and land

This revision was landed with ongoing or failed builds.Sep 8 2023, 3:05 PM
Closed by commit rCOMMe0f8b65e0b18: [native] Memoize construction of `sidebars` in `ChatThreadListItem` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMe0f8b65e0b18: [native] Memoize construction of `sidebars` in `ChatThreadListItem`.
Harbormaster completed remote builds in B22462: Diff 30881.Sep 8 2023, 3:20 PM
tomek added inline comments.Sep 11 2023, 1:14 AM
native/chat/chat-thread-list-sidebar.react.js
69–72

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

Revision Contents
Changeset List

Diff 30851

View Options

native/chat/chat-thread-list-item.react.js

Loading...
View Options

native/chat/chat-thread-list-sidebar.react.js

Loading...