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
F3724537: D9102.id30882.diff
Wed, Jan 8, 5:43 PM
F3724536: D9102.id30881.diff
Wed, Jan 8, 5:43 PM
F3724535: D9102.id30851.diff
Wed, Jan 8, 5:43 PM
Unknown Object (File)
Mon, Jan 6, 5:28 PM
Unknown Object (File)
Sun, Dec 29, 11:52 AM
Unknown Object (File)
Fri, Dec 20, 3:12 AM
Unknown Object (File)
Dec 3 2024, 2:18 AM
Unknown Object (File)
Dec 3 2024, 2:04 AM
View All 84 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
arcpatch-D9102 (branched from 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 ↗(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
atul added inline comments.Sep 8 2023, 11:03 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

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 ↗(On Diff #30851)

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

Revision Contents
Changeset List

Diff 30881

View Options

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

Loading...
View Options

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

Loading...