[lib] Introduce thread specs and thread traits
ClosedPublic
Actions

Authored by tomek on Apr 22 2025, 10:34 AM.

Details

Reviewers

kamil
angelika

Commits

rCOMMcc27c7d31c5c: [lib] Introduce thread specs and thread traits

Summary

Introduced thread specs that will contain thread-type-specific logic. Changed the approach of determining some behaviors by introducing thread traits, e.g. sidebar, or announcement - this will make it easier to introduce new threads and is consistent with other specs.

To do all of this, I had to move some functions from thread-types-enum.

https://linear.app/comm/issue/ENG-10263/introduce-a-thread-protocol-type

Test Plan

Flow. Tested with the rest of the stack.
Logged all the types array from thread-specs and compared with the previous values.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

tomek created this revision.Apr 22 2025, 10:34 AM

Herald added a subscriber: ashoat. · View Herald TranscriptApr 22 2025, 10:34 AM

Format

tomek added a child revision: D14589: [lib] Move some DM op types.Apr 22 2025, 10:42 AM

Harbormaster completed remote builds in B34006: Diff 47839.Apr 22 2025, 10:53 AM

tomek requested review of this revision.Apr 22 2025, 10:53 AM

Harbormaster completed remote builds in B34005: Diff 47838.Apr 22 2025, 1:04 PM

kamil accepted this revision.Apr 23 2025, 9:43 AM

kamil added inline comments.

lib/shared/threads/thread-specs.js
25–40 ↗	(On Diff #47839)	nit: I would group them logically or alphabetically but probably it doesn't really matter
62–71 ↗	(On Diff #47839)	I would log it and check and compare with previous values, and add it to the test plan
native/input/input-state-container.react.js
79–80 ↗	(On Diff #47839)	can be merged

This revision is now accepted and ready to land.Apr 23 2025, 9:43 AM

tomek planned changes to this revision.Apr 23 2025, 1:49 PM

tomek added inline comments.

lib/shared/threads/thread-specs.js
83–85 ↗	(On Diff #47839)	The new code isn't fully equivalent to the old one. When `threadType` is undefined, the old code would return false, while the new code throws. According to our types, we should never expect to receive `undefined` here, but in practice it happens... Here: https://github.com/CommE2E/comm/blob/03b266d8e07be80c341bf916b4ae33f59811d95f/lib/selectors/chat-selectors.js#L526 we're providing a value that can clearly be `undefined` and Flow doesn't complain about it. Going to update the code to keep the previous behavior.

tomek edited the test plan for this revision. (Show Details)Apr 23 2025, 1:56 PM

Update the approach

This revision is now accepted and ready to land.Apr 23 2025, 2:01 PM

tomek added inline comments.Apr 23 2025, 2:01 PM

lib/shared/threads/thread-specs.js
25–40 ↗	(On Diff #47839)	I kept the same ordering as in `thread-types-enum` - I guess that keeping them consistent has some value. That file is also consistent with `ThreadTypeEnum.h`. I don't think it is worth reordering all of them.
62–71 ↗	(On Diff #47839)	I've done exactly that, but haven't included it in the test plan. Updating it.