diff --git a/lib/hooks/sidebar-hooks.js b/lib/hooks/sidebar-hooks.js --- a/lib/hooks/sidebar-hooks.js +++ b/lib/hooks/sidebar-hooks.js @@ -28,7 +28,11 @@ ) { continue; } - const { lastUpdatedTime, lastUpdatedAtLeastTime } = getLastUpdatedTimes( + const { + lastUpdatedTime, + lastUpdatedAtLeastTime, + lastUpdatedAtMostTime, + } = getLastUpdatedTimes( childThreadInfo, messageStore, messageStore.messages, @@ -41,6 +45,7 @@ threadInfo: childThreadInfo, lastUpdatedTime, lastUpdatedAtLeastTime, + lastUpdatedAtMostTime, mostRecentNonLocalMessage, }); } diff --git a/lib/hooks/thread-time.js b/lib/hooks/thread-time.js --- a/lib/hooks/thread-time.js +++ b/lib/hooks/thread-time.js @@ -28,13 +28,19 @@ // should use while we're waiting for lastUpdatedTime to resolve. It's // set based on the most recent message whose spec returns a non-Promise // when getLastUpdatedTime is called + // - lastUpdatedAtMostTime: this is a number that helps us when trying to + // pick the top N items. we initially sort by this, then resolve the + // lastUpdatedTime promises until we have N items that after resolving + // lastUpdatedTime, have higher values than any of the other items' + // lastUpdatedAtMostTime let lastUpdatedAtLeastTime = threadInfo.creationTime; const thread = messageStore.threads[threadInfo.id]; if (!thread || !viewerID) { return { lastUpdatedAtLeastTime, - lastUpdatedTime: Promise.resolve(lastUpdatedAtLeastTime), + lastUpdatedAtMostTime: threadInfo.creationTime, + lastUpdatedTime: () => Promise.resolve(threadInfo.creationTime), }; } @@ -44,7 +50,8 @@ fetchMessage, }; - let lastUpdatedTime: ?Promise; + let lastUpdatedTime: ?() => Promise; + let highestTimestamp: ?number; for (const messageID of thread.messageIDs) { const messageInfo = messages[messageID]; if (!messageInfo) { @@ -55,48 +62,67 @@ // ?number or Promise. If the message spec doesn't implement // getLastUpdatedTime, then we default to messageInfo.time. const { getLastUpdatedTime } = messageSpecs[messageInfo.type]; - const lastUpdatedTimePromisable = getLastUpdatedTime + const lastUpdatedTimeResult = getLastUpdatedTime ? getLastUpdatedTime(messageInfo, getLastUpdatedTimeParams) : messageInfo.time; + // We only need to consider the first positive number result because + // thread.messageIDs ordered chronologically. + if ( + !highestTimestamp && + lastUpdatedTimeResult && + typeof lastUpdatedTimeResult === 'number' + ) { + highestTimestamp = lastUpdatedTimeResult; + } + // We rely on the fact that thread.messageIDs is ordered chronologically // (newest first) to chain together lastUpdatedTime. An older message's // lastUpdatedTime is only considered if all of the newer messages // return falsey. - lastUpdatedTime = (async () => { + lastUpdatedTime = async () => { if (lastUpdatedTime) { - const earlierChecks = await lastUpdatedTime; + const earlierChecks = await lastUpdatedTime(); if (earlierChecks) { return earlierChecks; } } - return await lastUpdatedTimePromisable; - })(); + if ( + !lastUpdatedTimeResult || + typeof lastUpdatedTimeResult === 'number' + ) { + return lastUpdatedTimeResult; + } + return await lastUpdatedTimeResult(); + }; - if (typeof lastUpdatedTimePromisable === 'number') { + if (typeof lastUpdatedTimeResult === 'number') { // We break from the loop the first time this condition is met. // There's no need to consider any older messages, since both // lastUpdated and lastUpdatedAtLeastTime will be this value (or // higher, in the case of lastUpdated). That is also why this loop // only sets lastUpdatedAtLeastTime once: once we get to this // "baseline" case, there's no need to consider any more messages. - lastUpdatedAtLeastTime = lastUpdatedTimePromisable; + lastUpdatedAtLeastTime = lastUpdatedTimeResult; break; } } - const lastUpdatedWithFallback = (async () => { + const lastUpdatedWithFallback = async () => { if (lastUpdatedTime) { - const earlierChecks = await lastUpdatedTime; + const earlierChecks = await lastUpdatedTime(); if (earlierChecks) { return earlierChecks; } } return lastUpdatedAtLeastTime; - })(); + }; + + const lastUpdatedAtMostTime = highestTimestamp ?? threadInfo.creationTime; return { lastUpdatedAtLeastTime, + lastUpdatedAtMostTime, lastUpdatedTime: lastUpdatedWithFallback, }; }, diff --git a/lib/selectors/chat-selectors.js b/lib/selectors/chat-selectors.js --- a/lib/selectors/chat-selectors.js +++ b/lib/selectors/chat-selectors.js @@ -42,7 +42,6 @@ } from '../types/minimally-encoded-thread-permissions-types.js'; import type { BaseAppState } from '../types/redux-types.js'; import { threadTypeIsSidebar } from '../types/thread-types-enum.js'; -import type { SidebarInfo, LastUpdatedTimes } from '../types/thread-types.js'; import type { AccountUserInfo, RelativeUserInfo, @@ -85,10 +84,10 @@ type CreatedChatThreadItem = $ReadOnly<{ ...ChatThreadItemBase, - ...LastUpdatedTimes, - +lastUpdatedTimeIncludingSidebars: Promise, + +lastUpdatedAtLeastTime: number, +lastUpdatedAtLeastTimeIncludingSidebars: number, - +allSidebarInfos: $ReadOnlyArray, + +lastUpdatedAtMostTimeIncludingSidebars: number, + +getFinalChatThreadItem: () => Promise, }>; function useCreateChatThreadItem(): ThreadInfo => CreatedChatThreadItem { const messageInfos = useSelector(messageInfoSelector); @@ -102,41 +101,66 @@ messageStore, ); - const { lastUpdatedTime, lastUpdatedAtLeastTime } = getLastUpdatedTimes( - threadInfo, - messageStore, - messageInfos, - ); + const { lastUpdatedTime, lastUpdatedAtLeastTime, lastUpdatedAtMostTime } = + getLastUpdatedTimes(threadInfo, messageStore, messageInfos); const sidebars = sidebarInfos[threadInfo.id] ?? []; + const chatThreadItemBase = { + type: 'chatThreadItem', + threadInfo, + mostRecentNonLocalMessage, + }; + + const getFinalChatThreadItem = async () => { + const lastUpdatedTimePromise = lastUpdatedTime(); + + const lastUpdatedTimeIncludingSidebarsPromise = (async () => { + const lastUpdatedTimePromises = [ + lastUpdatedTimePromise, + ...sidebars.map(sidebar => sidebar.lastUpdatedTime()), + ]; + const lastUpdatedTimes = await Promise.all(lastUpdatedTimePromises); + const max = lastUpdatedTimes.reduce((a, b) => Math.max(a, b), -1); + return max; + })(); + + const [ + lastUpdatedTimeResult, + lastUpdatedTimeIncludingSidebars, + allSidebarItems, + ] = await Promise.all([ + lastUpdatedTimePromise, + lastUpdatedTimeIncludingSidebarsPromise, + getAllFinalSidebarItems(sidebars), + ]); + + return { + ...chatThreadItemBase, + sidebars: getSidebarItems(allSidebarItems), + lastUpdatedTime: lastUpdatedTimeResult, + lastUpdatedTimeIncludingSidebars: lastUpdatedTimeIncludingSidebars, + }; + }; + const lastUpdatedAtLeastTimeIncludingSidebars = sidebars.length > 0 ? Math.max(lastUpdatedAtLeastTime, sidebars[0].lastUpdatedAtLeastTime) : lastUpdatedAtLeastTime; - const lastUpdatedTimeIncludingSidebars = (async () => { - const lastUpdatedTimePromises = [ - lastUpdatedTime, - ...sidebars.map(sidebar => sidebar.lastUpdatedTime), - ]; - const lastUpdatedTimes = await Promise.all(lastUpdatedTimePromises); - const max = lastUpdatedTimes.reduce((a, b) => Math.max(a, b), -1); - return max; - })(); + const lastUpdatedAtMostTimeIncludingSidebars = + sidebars.length > 0 + ? Math.max(lastUpdatedAtMostTime, sidebars[0].lastUpdatedAtMostTime) + : lastUpdatedAtMostTime; const allInitialSidebarItems = getAllInitialSidebarItems(sidebars); - const sidebarItems = getSidebarItems(allInitialSidebarItems); return { - type: 'chatThreadItem', - threadInfo, - mostRecentNonLocalMessage, - lastUpdatedTime, + ...chatThreadItemBase, + sidebars: getSidebarItems(allInitialSidebarItems), lastUpdatedAtLeastTime, - lastUpdatedTimeIncludingSidebars, lastUpdatedAtLeastTimeIncludingSidebars, - sidebars: sidebarItems, - allSidebarInfos: sidebars, + lastUpdatedAtMostTimeIncludingSidebars, + getFinalChatThreadItem, }; }, [messageInfos, messageStore, sidebarInfos, getLastUpdatedTimes], @@ -172,11 +196,10 @@ () => createdChatThreadItems.map(createdChatThreadItem => { const { - allSidebarInfos, - lastUpdatedTime, lastUpdatedAtLeastTime, - lastUpdatedTimeIncludingSidebars, lastUpdatedAtLeastTimeIncludingSidebars, + lastUpdatedAtMostTimeIncludingSidebars, + getFinalChatThreadItem, ...rest } = createdChatThreadItem; return { @@ -204,33 +227,9 @@ void (async () => { const finalChatThreadItems = await Promise.all( - createdChatThreadItems.map(async createdChatThreadItem => { - const { - allSidebarInfos, - lastUpdatedTime: lastUpdatedTimePromise, - lastUpdatedAtLeastTime, - lastUpdatedTimeIncludingSidebars: - lastUpdatedTimeIncludingSidebarsPromise, - lastUpdatedAtLeastTimeIncludingSidebars, - ...rest - } = createdChatThreadItem; - const [ - lastUpdatedTime, - lastUpdatedTimeIncludingSidebars, - allSidebarItems, - ] = await Promise.all([ - lastUpdatedTimePromise, - lastUpdatedTimeIncludingSidebarsPromise, - getAllFinalSidebarItems(allSidebarInfos), - ]); - const sidebars = getSidebarItems(allSidebarItems); - return { - ...rest, - lastUpdatedTime, - lastUpdatedTimeIncludingSidebars, - sidebars, - }; - }), + createdChatThreadItems.map(createdChatThreadItem => + createdChatThreadItem.getFinalChatThreadItem(), + ), ); if (createdChatThreadItems !== prevCreatedChatThreadItemsRef.current) { // If these aren't equal, it indicates that the effect has fired again. diff --git a/lib/shared/messages/message-spec.js b/lib/shared/messages/message-spec.js --- a/lib/shared/messages/message-spec.js +++ b/lib/shared/messages/message-spec.js @@ -144,5 +144,5 @@ +getLastUpdatedTime?: ( messageInfoOrRawMessageInfo: Info | RawInfo, params: ShowInMessagePreviewParams, - ) => ?number | Promise, + ) => ?number | (() => Promise), }; diff --git a/lib/shared/sidebar-item-utils.js b/lib/shared/sidebar-item-utils.js --- a/lib/shared/sidebar-item-utils.js +++ b/lib/shared/sidebar-item-utils.js @@ -79,7 +79,12 @@ sidebarInfos: $ReadOnlyArray, ): SidebarThreadItem[] { return sidebarInfos.map(sidebarItem => { - const { lastUpdatedTime, lastUpdatedAtLeastTime, ...rest } = sidebarItem; + const { + lastUpdatedTime, + lastUpdatedAtLeastTime, + lastUpdatedAtMostTime, + ...rest + } = sidebarItem; return { ...rest, type: 'sidebar', @@ -92,8 +97,13 @@ sidebarInfos: $ReadOnlyArray, ): Promise<$ReadOnlyArray> { const allSidebarItemPromises = sidebarInfos.map(async sidebarItem => { - const { lastUpdatedTime, lastUpdatedAtLeastTime, ...rest } = sidebarItem; - const finalLastUpdatedTime = await lastUpdatedTime; + const { + lastUpdatedTime, + lastUpdatedAtLeastTime, + lastUpdatedAtMostTime, + ...rest + } = sidebarItem; + const finalLastUpdatedTime = await lastUpdatedTime(); return { ...rest, type: 'sidebar', diff --git a/lib/types/thread-types.js b/lib/types/thread-types.js --- a/lib/types/thread-types.js +++ b/lib/types/thread-types.js @@ -440,7 +440,10 @@ // The last updated time is at least this number, but possibly higher // We won't know for sure until the below Promise resolves +lastUpdatedAtLeastTime: number, - +lastUpdatedTime: Promise, + // The last updated time is no more than this number, but possible lower + // We won't know for sure until the below Promise resolves + +lastUpdatedAtMostTime: number, + +lastUpdatedTime: () => Promise, }; export type SidebarInfo = $ReadOnly<{