[keyserver, lib] extract checking if message is an edit, reaction or sidebar_source
ClosedPublic
Actions

Authored by inka on Apr 26 2023, 7:06 AM.

Details

Reviewers

ashoat
bartek
kamil

Commits

rCOMM55975c711e0d: [keyserver, lib] extract checking if message is an edit, reaction or…

Summary

In response to review on D7584 I am fixing a invariant message and extracting checking if message is an edit, reaction or sidebar_source.

I cannot use the assertion pattern (using invariants like in assertComposableRawMessage) because in D7584 I need to check the type, not assert it.
I cannot have a function that checks the type and returns a boolean, because then in message-fetchers in line 765 flow is not convinced that the type is correct, because the check is
done elsewhere, and it just sees that the type is RawMessageInfo, which is too broad. Same for sidebar-source-message

I don't think this is a great solution, but I didn't find a better one.

Test Plan

run flow check in keyserver/ and lib/

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Apr 26 2023, 7:06 AM

Herald added subscribers: atul, tomek. · View Herald TranscriptApr 26 2023, 7:06 AM

inka added a child revision: D7584: [keyserver] Add function for fetching RawMessageInfo for rows and their related messages.Apr 26 2023, 7:07 AM

inka mentioned this in D7584: [keyserver] Add function for fetching RawMessageInfo for rows and their related messages.Apr 26 2023, 7:11 AM

inka added a comment.Apr 26 2023, 7:22 AM

This comment was removed by inka.

inka edited the summary of this revision. (Show Details)Apr 26 2023, 7:22 AM

Harbormaster completed remote builds in B18809: Diff 25768.Apr 26 2023, 7:25 AM

inka requested review of this revision.Apr 26 2023, 7:25 AM

inka planned changes to this revision.Apr 26 2023, 7:26 AM

Couldn't this function be simply:

function isMessageSidebarSourceReactionOrEdit(
  message: MessageInfo | RawMessageInfo
): boolean {
  return (
    message.type === messageTypes.SIDEBAR_SOURCE ||
    message.type === messageTypes.REACTION ||
    message.type === messageTypes.EDIT_MESSAGE
  );
}

// then usages
if (isMessageSidebarSourceReactionOrEdit(messageInfo)) { ... }

invariant(
  !isMessageSidebarSourceReactionOrEdit(message),
  "error text here",
);

I did something similar in D7616

We don't need this check for our logic, we only need it to reassure flow that type is correct. We know it is correct. And with the solution you propose flow doesn't know the type is correct inside of the if in message-fetchers.js

In D7632#226400, @inka wrote:

We don't need this check for our logic, we only need it to reassure flow that type is correct. We know it is correct. And with the solution you propose flow doesn't know the type is correct inside of the if in message-fetchers.js

Ah, so maybe try marking this as "predicate function":

function isMessageSidebarSourceReactionOrEdit(
  message: MessageInfo | RawMessageInfo
): boolean %checks {
  return (
    message.type === messageTypes.SIDEBAR_SOURCE ||
    message.type === messageTypes.REACTION ||
    message.type === messageTypes.EDIT_MESSAGE
  );
}

Refactor, use %checks

Although, as we talked, flows documentation states that this shouldn't work (here), I tried it and it works.
I ran flow check in lib/ and keyserver/ to be sure there are no errors.

Harbormaster completed remote builds in B18861: Diff 25830.Apr 27 2023, 7:05 AM

In D7632#226887, @inka wrote:

Although, as we talked, flows documentation states that this shouldn't work (here), I tried it and it works.
I ran flow check in lib/ and keyserver/ to be sure there are no errors.

Awesome!!

I thought it wouldn't work because Flow docs mention that it isn't able to deduce an object's properties' types, but it looks like it is able to do so for the object itself, based on its properties.

This revision is now accepted and ready to land.Apr 27 2023, 7:19 AM

inka added a parent revision: D7562: [keyserver] Add function to fetch reactions/edits/sidebar_source for messages.May 3 2023, 1:08 AM

Closed by commit rCOMM55975c711e0d: [keyserver, lib] extract checking if message is an edit, reaction or… (authored by • InkaSokolowska). · Explain WhyMay 3 2023, 1:14 AM

This revision was automatically updated to reflect the committed changes.

• InkaSokolowska added a commit: rCOMM55975c711e0d: [keyserver, lib] extract checking if message is an edit, reaction or….