[keyserver/lib] Update rawMessageInfoForRowsAndRelatedMessages to check for canBeRenderedOnItsOwn
ClosedPublic
Actions

Authored by rohan on Oct 19 2023, 6:22 AM.

Details

Reviewers

atul
ginsu

Commits

rCOMMf4471a9a2717: [keyserver/lib] Update rawMessageInfoForRowsAndRelatedMessages to check for…

Summary

As mentioned in D9536, we can now simplify the check in rawMessageInfoForRowsAndRelatedMessages to only filter out what is needed (EDIT_MESSAGE and REACTION). To do this, we'll call isUnableToBeRenderedOnItsOwn, which will check the message type's messageSpec, and then return the corresponding flag. This isolates the single source of truth to the individual message specs.

There is a usage of FlowFixMe here. I'll copy my explanation for using it here in the code as well so it's closer to it's usage, but essentially: Flow doesn't realize that only certain message types have the flag set as false, so we never get the type refinement we're expecting to set the rawMessageInfo into the map. The continue will ensure we don't set an invalid type into the map anyways, but once again Flow doesn't realize that. The only workaround I found was to basically invariant against every message type we don't expect to see (i.e. REACTION and EDIT_MESSAGE), but I want to avoid that since that 1) removes the 'single source of truth' that we've been working to achieve with this stack, and 2) makes it easier for future changes to this code to forget to update this invariant and then something will inevitably go wrong.

Resolves https://linear.app/comm/issue/ENG-5217/update-rawmessageinfoforrowsandrelatedmessages-to-check-for

Depends on D9536

Test Plan

Ran yarn workspace lib test, and then opened Comm and saw no crashing. Tried to simulate normal behavior so I:

Logged back out and logged back in
Pinned some messages and opened the pinned messages modal
Unpinned some messages
Edited some messages before unpinning them and opening the pinned messages modal
Searching for messages in chats

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Oct 19 2023, 6:22 AM

rohan held this revision as a draft.

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 19 2023, 6:22 AM

rohan added a child revision: D9538: [lib] Refactor fetchDerivedMessages to call isInvalidSidebarSource or isInvalidPinSource.Oct 19 2023, 6:39 AM

Harbormaster completed remote builds in B23361: Diff 32195.Oct 19 2023, 6:40 AM

rohan mentioned this in D9538: [lib] Refactor fetchDerivedMessages to call isInvalidSidebarSource or isInvalidPinSource.Oct 19 2023, 6:41 AM

rohan edited the summary of this revision. (Show Details)

rohan added inline comments.

keyserver/src/fetchers/message-fetchers.js
898 ↗	(On Diff #32195)	Flow doesn't realize that only certain message types have the flag set as false, so we never get the type refinement we're expecting to set the `rawMessageInfo` into the map. The `continue` will ensure we don't set an invalid type into the map anyways, but once again Flow doesn't realize that. The only workaround I found was to basically `invariant` against every message type we don't expect to see (i.e. `REACTION` and `EDIT_MESSAGE`), but I want to avoid that since that 1) removes the 'single source of truth' that we've been working to achieve with this stack, and 2) makes it easier for future changes to this code to forget to update this invariant and then something will inevitably go wrong.

rohan published this revision for review.Oct 19 2023, 7:49 AM

atul accepted this revision.Oct 19 2023, 11:34 AM

atul added inline comments.

keyserver/src/fetchers/message-fetchers.js
898 ↗	(On Diff #32195)	Let's add a more concise version of this annotation to the `$FlowFixMe` so future readers can understand what's going on without `git blame`