Page MenuHomePhabricator

[lib][web][native] Move some types to keyserver-conn-types.js
ClosedPublic

Authored by ashoat on Jan 18 2024, 12:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 22, 4:57 PM
Unknown Object (File)
Sun, Oct 13, 11:49 PM
Unknown Object (File)
Sun, Oct 13, 11:49 PM
Unknown Object (File)
Sun, Oct 13, 11:47 PM
Unknown Object (File)
Sep 28 2024, 4:42 PM
Unknown Object (File)
Sep 5 2024, 11:53 PM
Unknown Object (File)
Sep 5 2024, 11:53 PM
Unknown Object (File)
Sep 5 2024, 11:53 PM
Subscribers
None

Details

Summary

We need the ActionFunc and SingleKeyserverActionFunc types in CallKeyserverEndpointProvider, but this would currently lead to a import cycle.

It's actually fine some Flow can handle import cycles and the cycle is types-only, but in general it's good practice to avoid cycles.

To avoid the cycles, I'm moving these types to keyserver-conn-types.js.

Depends on D10692

Test Plan

I tested this stack using the following procedure:

  1. I tested primarily on native
    1. I compiled a dev build and deployed it to an iOS simulator
    2. I created a brand new account on my local keyserver using the iOS app
    3. I ran Redux dev tools: cd native && yarn redux-devtools
    4. I added a 30s sleep at the start of resolveKeyserverSessionInvalidation
    5. I made KeyserverConnectionsHandler return null so that the socket wouldn’t automatically recover the session prior to my testing
    6. I killed the app
    7. I deleted all of the test user’s cookie
    8. I then opened the app again and navigated to a chat and sent two messages
    9. By following the Redux monitor, I was able to see that the keyserver session invalidation recovery was successful, and both messages were eventually sent after the 30s sleep concluded
  2. On web, we don’t support keyserver session invalidation. However, I tested to make sure that the web app still loaded after my changes

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added inline comments.
lib/actions/device-actions.js
10 ↗(On Diff #35764)

Should we also move KeyserverCallParamOverride?

This revision is now accepted and ready to land.Jan 18 2024, 6:14 AM
lib/actions/device-actions.js
10 ↗(On Diff #35764)

No, I don't think so. It's only use in two places: this file, and where it's defined in lib/utils/keyserver-call.js.