Page MenuHomePhabricator

[native] Memoize `SwipeableThread`
ClosedPublic

Authored by atul on Sep 7 2023, 10:05 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 22, 2:18 PM
Unknown Object (File)
Tue, Oct 22, 2:18 PM
Unknown Object (File)
Tue, Oct 22, 2:18 PM
Unknown Object (File)
Tue, Oct 22, 2:17 PM
Unknown Object (File)
Tue, Oct 22, 5:04 AM
Unknown Object (File)
Sep 22 2024, 5:58 PM
Unknown Object (File)
Sep 18 2024, 7:02 PM
Unknown Object (File)
Sep 2 2024, 3:52 PM
Subscribers

Details

Summary

The real performance gains will come from adding memoization to ChatThreadListItem and ChatThreadListSidebar, but this is a prerequisite for those optimizations to make a difference.


Depends on D9099

Test Plan

Continue to look and behave as expected.

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D9100 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul requested review of this revision.Sep 7 2023, 10:23 AM
tomek added inline comments.
native/chat/swipeable-thread.react.js
82–93 ↗(On Diff #30848)

Creating a memo that depends on children doesn't always work. The problem appears when SwipeableThread has more than one child

<SwipeableThread>
  <View .../>
</SwipeableThread>

vs

<SwipeableThread>
  <View .../>
  <View .../>
</SwipeableThread>

In the second case, props.children is an array that is created on every render, which means that this memo will be recomputed every time. Not sure if it can be fixed from this component, but we can check the usages of SwipeableThread.

This revision is now accepted and ready to land.Sep 8 2023, 2:22 AM
native/chat/swipeable-thread.react.js
82–93 ↗(On Diff #30848)

I think we should be fine based on

a0eadb.png (1×2 px, 208 KB)

This revision was automatically updated to reflect the committed changes.