lib] Combine openSocketSelector from web and native
ClosedPublic
Actions

Authored by michal on Dec 12 2023, 3:47 AM.

Details

Reviewers

inka
tomek

Commits

rCOMMbc8828d76c4b: [web/native/lib] Combine openSocketSelector from web and native

Summary

ENG-6043

Should have been done when web cookies were migrated to redux, but I haven't noticed this code then. Now that cookies are in redux on all platforms we can merge these two functions.

Test Plan

Tested that web app still works fine (logging in, out, navigation, sending messages). It's hard to test the of cookie change restarts the websocket because web can't really recover from cookies changes and it just causes a logout.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Dec 12 2023, 3:47 AM

Herald added a subscriber: ashoat. · View Herald TranscriptDec 12 2023, 3:47 AM

Harbormaster completed remote builds in B24985: Diff 34525.Dec 12 2023, 4:09 AM

michal requested review of this revision.Dec 12 2023, 4:09 AM

tomek accepted this revision.Dec 13 2023, 4:51 AM

tomek added inline comments.

lib/selectors/socket-selectors.js
37–42 ↗	(On Diff #34525)	This behavior wasn't present on the web and now we're introducing it. Is it a good idea to do so?

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

michal added inline comments.Jan 4 2024, 7:49 AM

lib/selectors/socket-selectors.js
37–42 ↗	(On Diff #34525)	I can't think of a situation where we wouldn't want this behaviour. If the cookie changes we should update the websocket connection so everything is in sync. Now that cookies on web are in redux we should try and treat it uniformly.