Now that existing message specs each have a canBeSidebarSource method, I need to update the function isInvalidSidebarSource to check messageSpec.canBeSidebarSource.
Depends on D9325
Details
Details
Ran flow and manually tested sidebar creation on a variety of message robotext types & text messages
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| lib/shared/message-utils.js | ||
|---|---|---|
| 662–667 ↗ | (On Diff #31875) | Ideally, I would just check messageSpecs[message.type].canBeSidebarSource. Doing this though causes several flow issues throughout the message-fetchers.js since type refinement doesn't occur without explicitly checking for types. I couldn't think of a better solution to the Flow issue, but we're safeguarded by the unit tests testing both isInvalidSidebarSource and messageSpec.canBeSidebarSource anyways |
| lib/shared/message-utils.js | ||
|---|---|---|
| 662–667 ↗ | (On Diff #31875) | Update: I think this check can actually be simplified back down to !messageSpecs[message.type].canBeSidebarSource once ENG-5215 is done since that should remove the use case of this function in message-fetchers. For now to pass flow checks I'll need to keep it though (ENG-5215 will come after this in the stack, so I'll fix this check then if it can be done) |
| lib/shared/message-utils.js | ||
|---|---|---|
| 662–667 ↗ | (On Diff #31875) | Just wondering, how does Flow feel about the following? if (messageSpecs[message.type].canBeSidebarSource) { return false; } invariant( message.type === messageTypes.REACTION || message.type === messageTypes.EDIT_MESSAGE || message.type === messageTypes.SIDEBAR_SOURCE || message.type === messageTypes.TOGGLE_PIN, "only four types canBeSidebarSource", ); return true; |
| lib/shared/message-utils.js | ||
|---|---|---|
| 662–667 ↗ | (On Diff #31875) | Thanks for the suggestion, though when I tried it I got the same Flow warnings as just doing function isInvalidSidebarSource(
message: RawMessageInfo | MessageInfo,
): boolean {
return messageSpecs[message.type].canBeSidebarSource;
} |