Page MenuHomePhabricator
Files F3513970

D14136.diff
No OneTemporary
Actions

D14136.diff
View Options

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<?number>;
+ let lastUpdatedTime: ?() => Promise<?number>;
+ let highestTimestamp: ?number;
for (const messageID of thread.messageIDs) {
const messageInfo = messages[messageID];
if (!messageInfo) {
@@ -55,48 +62,67 @@
// ?number or Promise<?number>. 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<number>,
+ +lastUpdatedAtLeastTime: number,
+lastUpdatedAtLeastTimeIncludingSidebars: number,
- +allSidebarInfos: $ReadOnlyArray<SidebarInfo>,
+ +lastUpdatedAtMostTimeIncludingSidebars: number,
+ +getFinalChatThreadItem: () => Promise<ChatThreadItem>,
}>;
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>,
+ ) => ?number | (() => Promise<?number>),
};
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<SidebarInfo>,
): 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<SidebarInfo>,
): Promise<$ReadOnlyArray<SidebarThreadItem>> {
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<number>,
+ // 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<number>,
};
export type SidebarInfo = $ReadOnly<{

File Metadata

Mime Type
text/plain
Expires
Mon, Dec 23, 2:52 AM (14 h, 18 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2690828
Default Alt Text
D14136.diff (13 KB)

Event Timeline