Details

Reviewers

ginsu
ashoat
tomek
rohan

Commits

rCOMM716319a400ea: [native] Replace `listDataSelector` with `useMemo` in `ChatThreadList`

Summary

This diff is just one step in the process of converting ChatThreadList into a functional component. When this work is done, we will be able to avoid a lot of re-rendering and hopefully improve performance quite a bit.

Depends on D9173

Test Plan

ChatThreadList + search experience continue to work as expected.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Tue, Sep 12, 5:10 PM

atul published this revision for review.Tue, Sep 12, 5:11 PM

Harbormaster completed remote builds in B22569: Diff 31019.Tue, Sep 12, 5:28 PM

atul edited the summary of this revision. (Show Details)Wed, Sep 13, 8:02 AM

atul added a parent revision: D9173: [native] Lift `renderItem` to `ConnectedChatThreadList`.

atul removed a parent revision: D9172: [native] Lift `hardwareBack` to `ConnectedChatThreadList`.

atul added a child revision: D9183: [native] Lift `floatingAction` to `ConnectedChatThreadList`.Wed, Sep 13, 9:29 AM

One concern inline

native/chat/chat-thread-list.react.js
587 ↗	(On Diff #31019)	Should this be read only?

This revision now requires changes to proceed.Wed, Sep 13, 9:49 AM

atul added inline comments.Wed, Sep 13, 9:50 AM

native/chat/chat-thread-list.react.js
587 ↗	(On Diff #31019)	That's how they were typed previously, can try making them $ReadOnly

address @ginsu's feedback

atul added inline comments.Wed, Sep 13, 9:57 AM

native/chat/chat-thread-list.react.js
587 ↗	(On Diff #31019)	JK, you're totally right. The `chatItems` "inside" should be `Item[]` since we're `push`ing and `unshift`ing, but both `listData` and `partialListData` should be `$ReadOnlyArray<Item>`