Page MenuHomePhabricator

[lib] Separate out bindKeyserverCall cache for each useKeyserverCall callsite
ClosedPublic

Authored by ashoat on Dec 27 2023, 6:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 6 2024, 3:32 PM
Unknown Object (File)
Mar 6 2024, 3:32 PM
Unknown Object (File)
Mar 6 2024, 3:32 PM
Unknown Object (File)
Mar 6 2024, 3:21 PM
Unknown Object (File)
Mar 6 2024, 2:58 PM
Unknown Object (File)
Feb 5 2024, 11:17 AM
Unknown Object (File)
Jan 29 2024, 8:30 PM
Unknown Object (File)
Jan 27 2024, 8:11 AM
Subscribers

Details

Summary

The previous diff has one shortcoming, which is mentioned there:

However, we still have an issue owing to the fact that the useDerivedObject will cache on a per-hook-invocation level, whereas bindCallKeyserverEndpointSelector is caching globally.

This diff resolves that by lifting the bindCallKeyserverEndpointSelector cache to also be on a per-hook level.

We can't make the cache global because the individual hook calls can have their params overriden.

Depends on D10465

Test Plan

Before this stack, I was able to reproduce ENG-3612 by going to the ThreadSettings screen in native while my local keyserver was down. After this stack, the issue no longer repros.

I also compiled a release build of the iOS app to my phone to confirm that there were no regressions in TTI, or the time it takes to open a MessageList and go back to the ChatThreadList.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

When I initially introduced this createSelector+ _memoize memoization, we wanted to memoize globally. If we are removing this requirement anyway, we can probably use the approach you suggested on the original diff D9143. That is remove createSelector and just call the _memoized function inside of useMemo.

That's fair – I'll make that change. Worth noting that the new createSelector usage I introduce in D10465 is different, however – in that case, we can't simply replace it with a React.useMemo, because we're taking advantage of the "function that caches its last result" functionality of createSelector in useDerivedObject

Replace createSelector with useMemo

This revision is now accepted and ready to land.Dec 29 2023, 2:51 AM