Page MenuHomePhabricator

[lib] Introduce RawThickThreadInfo type
ClosedPublic

Authored by ashoat on Jun 19 2024, 5:15 PM.
Tags
None
Referenced Files
F3574516: D12495.id41531.diff
Sat, Dec 28, 5:41 PM
F3569586: D12495.diff
Sat, Dec 28, 3:08 AM
Unknown Object (File)
Thu, Dec 19, 5:00 AM
Unknown Object (File)
Thu, Dec 19, 5:00 AM
Unknown Object (File)
Thu, Dec 19, 5:00 AM
Unknown Object (File)
Thu, Dec 19, 5:00 AM
Unknown Object (File)
Sun, Dec 8, 12:15 PM
Unknown Object (File)
Wed, Dec 4, 8:34 PM
Subscribers

Details

Summary

This diff splits RawThreadInfo into RawThinThreadInfo (basically the old type) and RawThickThreadInfo, which will be used for E2E-encrypted DMs.

The latter type includes ThreadSubscription for each member.

I tried to break this diff down but it was tough due to Flow issues.

This diff doesn't update ThreadInfo. That type is used generally in frontend code, and I'm not sure we'll need the new ThreadSubscription.

This diff also doesn't do anything to make sure we handle RawThickThreadInfo correctly when we encounter it. That will be handled in later tasks. It's okay to commit this type change now as we aren't actually introducing RawThickThreadInfo to any stores.

On the Flow side here, I had to add a thick: true attribute. I initially tried to differentiate the two types through the type field, which should let us represent RawThreadInfo was a union of two disjoint types (RawThinThreadInfo and RawThickThreadInfo). But I wasn't able to get Flow to narrow the types when I checked the type field. Here's what I tried:

  • Enumerating all of the possible values of type at every location, for both the thick and thin conditions. This worked but was way too verbose
  • Considered using type guards, but our setup doesn't support them yet. More details in this blog
  • Considered using an older version of type guards: assert functions that use %checks. However this wasn't able to narrow a threadInfo by checking threadInfo.type for some reason
  • Considered using assert functions that just have invariants and anys instead of %checks. This resulted in four functions: assertThinRawThreadInfo, assertThickRawThreadInfo, assertLegacyThinRawThreadInfo, assertLegacyThickRawThreadInfo. A pair of these would be combined with a threadTypeIsThick check. Ultimately decided it was too verbose
  • Referenced this GitHub issue where something similar is discussed

Depends on D12494

Test Plan

Flow

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/utils/thread-ops-utils.js
55–56

Addressed in D12496

lib/types/minimally-encoded-thread-permissions-types.js
273

It would be convenient to have a list of subscriptions here if ThreadInfo refers to thick thread. However it is something I can workaround.

This revision is now accepted and ready to land.Jun 24 2024, 10:18 AM
lib/types/minimally-encoded-thread-permissions-types.js
273

It's not in the scope of this current stack, but could be added later

This revision was automatically updated to reflect the committed changes.