Skip to content

Commit

Permalink
wip msglist render [nfc]: Explicitly note an undefined Flow misses.
Browse files Browse the repository at this point in the history
Kind of puzzling that Flow doesn't complain about this one on its
own!  As ESLint points out in the rule that we have to disable here,
the `= undefined` has no effect on runtime behavior -- it just
states explicitly what the code was already doing anyway.

But once we do make it explicit, Flow starts noticing that the
`undefined` was propagating into boolean expressions and to an
argument passed to `isSameRecipient` -- which also justifies a
check at the top of that function which by its types had looked
dead.  Fix that function's signature, and booleanize the value
when using as a boolean.

This Flow behavior is concerning enough that we should probably
just stop saying `let foo;`, and always explicitly initialize to
`undefined` when that's what we mean.  (The code is potentially
a bit clearer that way anyway.)  We'll do that next.
  • Loading branch information
gnprice committed Nov 11, 2020
1 parent d682d1d commit 163baed
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
9 changes: 6 additions & 3 deletions src/message/renderMessages.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ export default (
): RenderedSectionDescriptor[] => {
const showHeader = !isPrivateOrGroupNarrow(narrow) && !isTopicNarrow(narrow);

let prevItem;
// eslint-disable-next-line no-undef-init
let prevItem = undefined;
const sections = [{ key: 0, data: [], message: {} }];
messages.forEach(item => {
const diffDays =
prevItem && !isSameDay(new Date(prevItem.timestamp * 1000), new Date(item.timestamp * 1000));
!!prevItem
&& !isSameDay(new Date(prevItem.timestamp * 1000), new Date(item.timestamp * 1000));
if (!prevItem || diffDays) {
sections[sections.length - 1].data.push({
key: `time${item.timestamp}`,
Expand All @@ -23,6 +25,7 @@ export default (
firstMessage: item,
});
}

const diffRecipient = !isSameRecipient(prevItem, item);
if (showHeader && diffRecipient) {
sections.push({
Expand All @@ -37,7 +40,7 @@ export default (
// values that lack it; which is fine once the release that adds it
// has been out for a few weeks.
const shouldGroupWithPrev =
!diffRecipient && !diffDays && prevItem && prevItem.sender_email === item.sender_email;
!diffRecipient && !diffDays && !!prevItem && prevItem.sender_email === item.sender_email;

sections[sections.length - 1].data.push({
key: item.id,
Expand Down
4 changes: 2 additions & 2 deletions src/utils/recipient.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ export const pmUnreadsKeyFromMessage = (message: Message, ownUserId?: number): s
};

export const isSameRecipient = (
message1: Message | Outbox,
message2: Message | Outbox,
message1: Message | Outbox | void,
message2: Message | Outbox | void,
): boolean => {
if (message1 === undefined || message2 === undefined) {
return false;
Expand Down

0 comments on commit 163baed

Please sign in to comment.