Page MenuHomePhabricator

[native] Lift `component[DidMount/WillUnmount]` lifecycle methods to `useEffect` in `ConnectedChatThreadList`

Authored by atul on Sep 13 2023, 10:28 AM.
Referenced Files
Unknown Object (File)
Thu, Jul 11, 10:33 PM
Unknown Object (File)
Wed, Jul 10, 2:47 PM
Unknown Object (File)
Tue, Jul 9, 5:36 PM
Unknown Object (File)
Tue, Jul 9, 4:57 PM
Unknown Object (File)
Mon, Jul 8, 2:33 AM
Unknown Object (File)
Sun, Jul 7, 1:32 PM
Unknown Object (File)
Wed, Jul 3, 9:33 AM
Unknown Object (File)
Mon, Jul 1, 5:28 AM



Replace componentDidMount and componentWillUnmount with a useEffect in ConnectedThreadList. These lifecycle methods effectively add and remove listeners.

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 D9188

Test Plan

ChatThreadList + search experience continue to work as expected.

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

atul published this revision for review.Sep 13 2023, 10:31 AM
atul retitled this revision from [native] Lift `component[DidMount/WillUnmount]` lifestyle methods to `useEffect` in `ConnectedChatThreadList` to [native] Lift `component[DidMount/WillUnmount]` lifecycle methods to `useEffect` in `ConnectedChatThreadList`.Sep 13 2023, 10:52 AM
ashoat requested changes to this revision.Sep 13 2023, 11:01 AM

Can you decompose into three effects?

524–547 ↗(On Diff #31079)

The effect version of this is a little different because it will rerun if any of its inputs change. This isn't necessarily a bad thing... we should probably re-add these listeners in that case. But it would be a good idea to decompose this into three effects to avoid unnecessary re-mounting of unrelated listeners

This revision now requires changes to proceed.Sep 13 2023, 11:01 AM
524–547 ↗(On Diff #31079)

(Actually it can only be decomposed into two)

524–547 ↗(On Diff #31079)

On third thought, it can be three:

  1. One that only depends on hardwareBack
  2. A second that only depends on navigation (the "navigation blur listener")
  3. A third that depends on both navigation and onTabPress
524–547 ↗(On Diff #31079)

Yeah, can break them down into three useEffects. That's kind of what I did with

Specifically it looks like hardwareBack has searchStatus in the dependency list which would cause all of these listeners to be re-mounted. Will address feedback and update diff.

address @ashoat's feedback: split into three separate useEffects

This revision is now accepted and ready to land.Sep 13 2023, 11:37 AM