From 55a1c5f6c5a5ee0c127e0b34251fe869b4ac2d59 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Thu, 22 Dec 2022 16:32:03 -0800 Subject: [PATCH] Use proxy-compare for message bubbles --- package.json | 1 + ts/state/selectors/conversations.ts | 74 ---- ts/state/selectors/message.ts | 624 ++++++++++++---------------- ts/state/selectors/timeline.ts | 72 ++++ ts/state/smart/Timeline.tsx | 5 +- ts/state/smart/TimelineItem.tsx | 12 +- ts/test-both/util/memoizeByRoot.ts | 62 --- ts/util/isShallowEqual.ts | 24 +- ts/util/memoizeByRoot.ts | 47 --- ts/util/proxyMemoize.ts | 78 ++++ yarn.lock | 5 + 11 files changed, 442 insertions(+), 562 deletions(-) create mode 100644 ts/state/selectors/timeline.ts delete mode 100644 ts/test-both/util/memoizeByRoot.ts delete mode 100644 ts/util/memoizeByRoot.ts create mode 100644 ts/util/proxyMemoize.ts diff --git a/package.json b/package.json index 5a983c2b03d..0ef5f14fc08 100644 --- a/package.json +++ b/package.json @@ -143,6 +143,7 @@ "pino": "8.6.1", "protobufjs": "6.11.3", "proxy-agent": "5.0.0", + "proxy-compare": "2.3.0", "qrcode-generator": "1.4.4", "quill": "1.3.7", "quill-delta": "4.0.1", diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index a11fa30fac2..aebe4c3d2a2 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -27,7 +27,6 @@ import { getOwn } from '../../util/getOwn'; import type { UUIDFetchStateType } from '../../util/uuidFetchState'; import { deconstructLookup } from '../../util/deconstructLookup'; import type { PropsDataType as TimelinePropsType } from '../../components/conversation/Timeline'; -import type { TimelineItemType } from '../../components/conversation/TimelineItem'; import { assertDev } from '../../util/assert'; import { isConversationUnregistered } from '../../util/isConversationUnregistered'; import { filterAndSortConversationsByRecent } from '../../util/filterAndSortConversations'; @@ -51,15 +50,8 @@ import { getRegionCode, getUserConversationId, getUserNumber, - getUserACI, - getUserPNI, } from './user'; import { getPinnedConversationIds } from './items'; -import { getPropsForBubble } from './message'; -import type { CallSelectorType, CallStateType } from './calling'; -import { getActiveCall, getCallSelector } from './calling'; -import type { AccountSelectorType } from './accounts'; -import { getAccountSelector } from './accounts'; import * as log from '../../logging/log'; import { TimelineMessageLoadingState } from '../../util/timelineUtil'; import { isSignalConversation } from '../../util/isSignalConversation'; @@ -772,18 +764,6 @@ export const getConversationByUuidSelector = createSelector( getOwn(conversationsByUuid, uuid) ); -// A little optimization to reset our selector cache whenever high-level application data -// changes: regionCode and userNumber. -export const getCachedSelectorForMessage = createSelector( - getRegionCode, - getUserNumber, - (): typeof getPropsForBubble => { - // Note: memoizee will check all parameters provided, and only run our selector - // if any of them have changed. - return memoizee(getPropsForBubble, { max: 2000 }); - } -); - const getCachedConversationMemberColorsSelector = createSelector( getConversationSelector, getUserConversationId, @@ -855,60 +835,6 @@ export const getContactNameColorSelector = createSelector( } ); -type GetMessageByIdType = (id: string) => TimelineItemType | undefined; -export const getMessageSelector = createSelector( - getCachedSelectorForMessage, - getMessages, - getSelectedMessage, - getConversationSelector, - getRegionCode, - getUserNumber, - getUserACI, - getUserPNI, - getUserConversationId, - getCallSelector, - getActiveCall, - getAccountSelector, - getContactNameColorSelector, - ( - messageSelector: typeof getPropsForBubble, - messageLookup: MessageLookupType, - selectedMessage: SelectedMessageType | undefined, - conversationSelector: GetConversationByIdType, - regionCode: string | undefined, - ourNumber: string | undefined, - ourACI: UUIDStringType | undefined, - ourPNI: UUIDStringType | undefined, - ourConversationId: string | undefined, - callSelector: CallSelectorType, - activeCall: undefined | CallStateType, - accountSelector: AccountSelectorType, - contactNameColorSelector: ContactNameColorSelectorType - ): GetMessageByIdType => { - return (id: string) => { - const message = messageLookup[id]; - if (!message) { - return undefined; - } - - return messageSelector(message, { - conversationSelector, - ourConversationId, - ourNumber, - ourACI, - ourPNI, - regionCode, - selectedMessageId: selectedMessage?.id, - selectedMessageCounter: selectedMessage?.counter, - contactNameColorSelector, - callSelector, - activeCall, - accountSelector, - }); - }; - } -); - export function _conversationMessagesSelector( conversation: ConversationMessageType ): TimelinePropsType { diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts index e7ae2a2a8fd..da7a4f311a5 100644 --- a/ts/state/selectors/message.ts +++ b/ts/state/selectors/message.ts @@ -1,18 +1,8 @@ // Copyright 2021-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import { - groupBy, - identity, - isEmpty, - isEqual, - isNumber, - isObject, - map, - omit, - pick, -} from 'lodash'; -import { createSelector, createSelectorCreator } from 'reselect'; +import { groupBy, isEmpty, isNumber, isObject, map, omit } from 'lodash'; +import { createSelector } from 'reselect'; import filesize from 'filesize'; import getDirection from 'direction'; import emojiRegex from 'emoji-regex'; @@ -66,7 +56,7 @@ import { isVoiceMessage, canBeDownloaded } from '../../types/Attachment'; import { ReadStatus } from '../../messages/MessageReadStatus'; import type { CallingNotificationType } from '../../util/callingNotification'; -import { memoizeByRoot } from '../../util/memoizeByRoot'; +import { proxyMemoize } from '../../util/proxyMemoize'; import { missingCaseError } from '../../util/missingCaseError'; import { getRecipients } from '../../util/getRecipients'; import { getOwn } from '../../util/getOwn'; @@ -280,13 +270,11 @@ export function getConversation( // Message -export const getAttachmentsForMessage = createSelectorCreator(memoizeByRoot)( - // `memoizeByRoot` requirement - identity, - - ({ sticker }: MessageWithUIFieldsType) => sticker, - ({ attachments }: MessageWithUIFieldsType) => attachments, - (_, sticker, attachments = []): Array => { +export const getAttachmentsForMessage = proxyMemoize( + ({ + sticker, + attachments = [], + }: MessageWithUIFieldsType): Array => { if (sticker && sticker.data) { const { data } = sticker; @@ -311,16 +299,16 @@ export const getAttachmentsForMessage = createSelectorCreator(memoizeByRoot)( .filter(attachment => !attachment.error || canBeDownloaded(attachment)) .map(attachment => getPropsForAttachment(attachment)) .filter(isNotNil); + }, + { + name: 'getAttachmentsForMessage', } ); -export const processBodyRanges = createSelectorCreator(memoizeByRoot, isEqual)( - // `memoizeByRoot` requirement - identity, - +export const processBodyRanges = proxyMemoize( ( { bodyRanges }: Pick, - { conversationSelector }: { conversationSelector: GetConversationByIdType } + options: { conversationSelector: GetConversationByIdType } ): HydratedBodyRangesType | undefined => { if (!bodyRanges) { return undefined; @@ -329,6 +317,7 @@ export const processBodyRanges = createSelectorCreator(memoizeByRoot, isEqual)( return bodyRanges .filter(range => range.mentionUuid) .map(range => { + const { conversationSelector } = options; const conversation = conversationSelector(range.mentionUuid); return { @@ -339,16 +328,83 @@ export const processBodyRanges = createSelectorCreator(memoizeByRoot, isEqual)( }) .sort((a, b) => b.start - a.start); }, - (_, ranges): undefined | HydratedBodyRangesType => ranges + { + name: 'processBodyRanges', + } ); -const getAuthorForMessage = createSelectorCreator(memoizeByRoot)( - // `memoizeByRoot` requirement - identity, +const getAuthorForMessage = ( + message: MessageWithUIFieldsType, + options: GetContactOptions +): PropsData['author'] => { + const { + acceptedMessageRequest, + avatarPath, + badges, + color, + id, + isMe, + name, + phoneNumber, + profileName, + sharedGroupNames, + title, + unblurredAvatarPath, + } = getContact(message, options); - getContact, + const unsafe = { + acceptedMessageRequest, + avatarPath, + badges, + color, + id, + isMe, + name, + phoneNumber, + profileName, + sharedGroupNames, + title, + unblurredAvatarPath, + }; + + const safe: AssertProps = unsafe; + + return safe; +}; + +const getPreviewsForMessage = ({ + preview: previews = [], +}: MessageWithUIFieldsType): Array => { + return previews.map(preview => ({ + ...preview, + isStickerPack: isStickerPack(preview.url), + domain: getDomain(preview.url), + image: preview.image ? getPropsForAttachment(preview.image) : undefined, + })); +}; + +const getReactionsForMessage = ( + { reactions = [] }: MessageWithUIFieldsType, + { conversationSelector }: { conversationSelector: GetConversationByIdType } +) => { + const reactionBySender = new Map(); + for (const reaction of reactions) { + const existingReaction = reactionBySender.get(reaction.fromId); + if (!existingReaction || reaction.timestamp > existingReaction.timestamp) { + reactionBySender.set(reaction.fromId, reaction); + } + } + + const reactionsWithEmpties = reactionBySender.values(); + const reactionsWithEmoji = iterables.filter( + reactionsWithEmpties, + re => re.emoji + ); + const formattedReactions = iterables.map(reactionsWithEmoji, re => { + const c = conversationSelector(re.fromId); + + type From = NonNullable[0]['from']; - (_, convo: ConversationType): PropsData['author'] => { const { acceptedMessageRequest, avatarPath, @@ -361,8 +417,7 @@ const getAuthorForMessage = createSelectorCreator(memoizeByRoot)( profileName, sharedGroupNames, title, - unblurredAvatarPath, - } = convo; + } = c; const unsafe = { acceptedMessageRequest, @@ -376,155 +431,65 @@ const getAuthorForMessage = createSelectorCreator(memoizeByRoot)( profileName, sharedGroupNames, title, - unblurredAvatarPath, }; - const safe: AssertProps = unsafe; + const from: AssertProps = unsafe; - return safe; - } -); - -const getCachedAuthorForMessage = createSelectorCreator(memoizeByRoot, isEqual)( - // `memoizeByRoot` requirement - identity, - getAuthorForMessage, - (_, author): PropsData['author'] => author -); - -export const getPreviewsForMessage = createSelectorCreator(memoizeByRoot)( - // `memoizeByRoot` requirement - identity, - ({ preview }: MessageWithUIFieldsType) => preview, - (_, previews = []): Array => { - return previews.map(preview => ({ - ...preview, - isStickerPack: isStickerPack(preview.url), - domain: getDomain(preview.url), - image: preview.image ? getPropsForAttachment(preview.image) : undefined, - })); - } -); - -export const getReactionsForMessage = createSelectorCreator( - memoizeByRoot, - isEqual -)( - // `memoizeByRoot` requirement - identity, - - ( - { reactions = [] }: MessageWithUIFieldsType, - { conversationSelector }: { conversationSelector: GetConversationByIdType } - ) => { - const reactionBySender = new Map(); - for (const reaction of reactions) { - const existingReaction = reactionBySender.get(reaction.fromId); - if ( - !existingReaction || - reaction.timestamp > existingReaction.timestamp - ) { - reactionBySender.set(reaction.fromId, reaction); - } - } - - const reactionsWithEmpties = reactionBySender.values(); - const reactionsWithEmoji = iterables.filter( - reactionsWithEmpties, - re => re.emoji - ); - const formattedReactions = iterables.map(reactionsWithEmoji, re => { - const c = conversationSelector(re.fromId); - - type From = NonNullable[0]['from']; - - const unsafe = pick(c, [ - 'acceptedMessageRequest', - 'avatarPath', - 'badges', - 'color', - 'id', - 'isMe', - 'name', - 'phoneNumber', - 'profileName', - 'sharedGroupNames', - 'title', - ]); - - const from: AssertProps = unsafe; - - strictAssert(re.emoji, 'Expected all reactions to have an emoji'); - - return { - emoji: re.emoji, - timestamp: re.timestamp, - from, - }; - }); - - return [...formattedReactions]; - }, - - (_, reactions): PropsData['reactions'] => reactions -); - -export const getPropsForStoryReplyContext = createSelectorCreator( - memoizeByRoot, - isEqual -)( - // `memoizeByRoot` requirement - identity, - - ( - message: Pick< - MessageWithUIFieldsType, - 'body' | 'conversationId' | 'storyReaction' | 'storyReplyContext' - >, - { - conversationSelector, - ourConversationId, - }: { - conversationSelector: GetConversationByIdType; - ourConversationId?: string; - } - ): PropsData['storyReplyContext'] => { - const { storyReaction, storyReplyContext } = message; - if (!storyReplyContext) { - return undefined; - } - - const contact = conversationSelector(storyReplyContext.authorUuid); - - const authorTitle = contact.firstName || contact.title; - const isFromMe = contact.id === ourConversationId; - - const conversation = getConversation(message, conversationSelector); - - const { conversationColor, customColor } = - getConversationColorAttributes(conversation); + strictAssert(re.emoji, 'Expected all reactions to have an emoji'); return { - authorTitle, - conversationColor, - customColor, - emoji: storyReaction?.emoji, - isFromMe, - rawAttachment: storyReplyContext.attachment - ? processQuoteAttachment(storyReplyContext.attachment) - : undefined, - storyId: storyReplyContext.messageId, - text: getStoryReplyText(window.i18n, storyReplyContext.attachment), + emoji: re.emoji, + timestamp: re.timestamp, + from, }; - }, + }); - (_, storyReplyContext): PropsData['storyReplyContext'] => storyReplyContext -); + return [...formattedReactions]; +}; -export const getPropsForQuote = createSelectorCreator(memoizeByRoot, isEqual)( - // `memoizeByRoot` requirement - identity, +const getPropsForStoryReplyContext = ( + message: Pick< + MessageWithUIFieldsType, + 'body' | 'conversationId' | 'storyReaction' | 'storyReplyContext' + >, + { + conversationSelector, + ourConversationId, + }: { + conversationSelector: GetConversationByIdType; + ourConversationId?: string; + } +): PropsData['storyReplyContext'] => { + const { storyReaction, storyReplyContext } = message; + if (!storyReplyContext) { + return undefined; + } + const contact = conversationSelector(storyReplyContext.authorUuid); + + const authorTitle = contact.firstName || contact.title; + const isFromMe = contact.id === ourConversationId; + + const conversation = getConversation(message, conversationSelector); + + const { conversationColor, customColor } = + getConversationColorAttributes(conversation); + + return { + authorTitle, + conversationColor, + customColor, + emoji: storyReaction?.emoji, + isFromMe, + rawAttachment: storyReplyContext.attachment + ? processQuoteAttachment(storyReplyContext.attachment) + : undefined, + storyId: storyReplyContext.messageId, + text: getStoryReplyText(window.i18n, storyReplyContext.attachment), + }; +}; + +export const getPropsForQuote = proxyMemoize( ( message: Pick< MessageWithUIFieldsType, @@ -591,8 +556,9 @@ export const getPropsForQuote = createSelectorCreator(memoizeByRoot, isEqual)( text, }; }, - - (_, quote): PropsData['quote'] => quote + { + name: 'getPropsForQuote', + } ); export type GetPropsForMessageOptions = Pick< @@ -609,136 +575,6 @@ export type GetPropsForMessageOptions = Pick< | 'contactNameColorSelector' >; -type ShallowPropsType = Pick< - PropsForMessage, - | 'canDeleteForEveryone' - | 'canDownload' - | 'canReact' - | 'canReply' - | 'canRetry' - | 'canRetryDeleteForEveryone' - | 'contact' - | 'contactNameColor' - | 'conversationColor' - | 'conversationId' - | 'conversationTitle' - | 'conversationType' - | 'customColor' - | 'deletedForEveryone' - | 'direction' - | 'displayLimit' - | 'expirationLength' - | 'expirationTimestamp' - | 'giftBadge' - | 'id' - | 'isBlocked' - | 'isMessageRequestAccepted' - | 'isSelected' - | 'isSelectedCounter' - | 'isSticker' - | 'isTapToView' - | 'isTapToViewError' - | 'isTapToViewExpired' - | 'readStatus' - | 'selectedReaction' - | 'status' - | 'text' - | 'textDirection' - | 'timestamp' ->; - -const getShallowPropsForMessage = createSelectorCreator(memoizeByRoot, isEqual)( - // `memoizeByRoot` requirement - identity, - - ( - message: MessageWithUIFieldsType, - { - accountSelector, - conversationSelector, - ourConversationId, - ourNumber, - ourACI, - regionCode, - selectedMessageId, - selectedMessageCounter, - contactNameColorSelector, - }: GetPropsForMessageOptions - ): ShallowPropsType => { - const { expireTimer, expirationStartTimestamp, conversationId } = message; - const expirationLength = expireTimer - ? DurationInSeconds.toMillis(expireTimer) - : undefined; - - const conversation = getConversation(message, conversationSelector); - const isGroup = conversation.type === 'group'; - const { sticker } = message; - - const isMessageTapToView = isTapToView(message); - - const isSelected = message.id === selectedMessageId; - - const selectedReaction = ( - (message.reactions || []).find(re => re.fromId === ourConversationId) || - {} - ).emoji; - - const authorId = getContactId(message, { - conversationSelector, - ourConversationId, - ourNumber, - ourACI, - }); - const contactNameColor = contactNameColorSelector(conversationId, authorId); - - const { conversationColor, customColor } = - getConversationColorAttributes(conversation); - - return { - canDeleteForEveryone: canDeleteForEveryone(message), - canDownload: canDownload(message, conversationSelector), - canReact: canReact(message, ourConversationId, conversationSelector), - canReply: canReply(message, ourConversationId, conversationSelector), - canRetry: hasErrors(message), - canRetryDeleteForEveryone: canRetryDeleteForEveryone(message), - contact: getPropsForEmbeddedContact(message, regionCode, accountSelector), - contactNameColor, - conversationColor, - conversationId, - conversationTitle: conversation.title, - conversationType: isGroup ? 'group' : 'direct', - customColor, - deletedForEveryone: message.deletedForEveryone || false, - direction: isIncoming(message) ? 'incoming' : 'outgoing', - displayLimit: message.displayLimit, - expirationLength, - expirationTimestamp: calculateExpirationTimestamp({ - expireTimer, - expirationStartTimestamp, - }), - giftBadge: message.giftBadge, - id: message.id, - isBlocked: conversation.isBlocked || false, - isMessageRequestAccepted: conversation?.acceptedMessageRequest ?? true, - isSelected, - isSelectedCounter: isSelected ? selectedMessageCounter : undefined, - isSticker: Boolean(sticker), - isTapToView: isMessageTapToView, - isTapToViewError: - isMessageTapToView && isIncoming(message) && message.isTapToViewInvalid, - isTapToViewExpired: isMessageTapToView && message.isErased, - readStatus: message.readStatus ?? ReadStatus.Read, - selectedReaction, - status: getMessagePropStatus(message, ourConversationId), - text: message.body, - textDirection: getTextDirection(message.body), - timestamp: message.sent_at, - }; - }, - - (_: unknown, props: ShallowPropsType) => props -); - function getTextAttachment( message: MessageWithUIFieldsType ): AttachmentType | undefined { @@ -806,51 +642,116 @@ function getTextDirection(body?: string): TextDirection { } } -export const getPropsForMessage: ( - message: MessageWithUIFieldsType, - options: GetPropsForMessageOptions -) => Omit = - createSelectorCreator(memoizeByRoot)( - // `memoizeByRoot` requirement - identity, +export const getPropsForMessage = proxyMemoize( + ( + message: MessageWithUIFieldsType, + options: GetPropsForMessageOptions + ): Omit => { + const attachments = getAttachmentsForMessage(message); + const bodyRanges = processBodyRanges(message, options); + const author = getAuthorForMessage(message, options); + const previews = getPreviewsForMessage(message); + const reactions = getReactionsForMessage(message, options); + const quote = getPropsForQuote(message, options); + const storyReplyContext = getPropsForStoryReplyContext(message, options); + const textAttachment = getTextAttachment(message); + const payment = getPayment(message); - getAttachmentsForMessage, - processBodyRanges, - getCachedAuthorForMessage, - getPreviewsForMessage, - getReactionsForMessage, - getPropsForQuote, - getPropsForStoryReplyContext, - getTextAttachment, - getPayment, - getShallowPropsForMessage, - ( - _, - attachments: Array, - bodyRanges: HydratedBodyRangesType | undefined, - author: PropsData['author'], - previews: Array, - reactions: PropsData['reactions'], - quote: PropsData['quote'], - storyReplyContext: PropsData['storyReplyContext'], - textAttachment: PropsData['textAttachment'], - payment: PropsData['payment'], - shallowProps: ShallowPropsType - ): Omit => { - return { - attachments, - author, - bodyRanges, - previews, - quote, - reactions, - storyReplyContext, - textAttachment, - payment, - ...shallowProps, - }; - } - ); + const { + accountSelector, + conversationSelector, + ourConversationId, + ourNumber, + ourACI, + regionCode, + selectedMessageId, + selectedMessageCounter, + contactNameColorSelector, + } = options; + + const { expireTimer, expirationStartTimestamp, conversationId } = message; + const expirationLength = expireTimer + ? DurationInSeconds.toMillis(expireTimer) + : undefined; + + const conversation = getConversation(message, conversationSelector); + const isGroup = conversation.type === 'group'; + const { sticker } = message; + + const isMessageTapToView = isTapToView(message); + + const isSelected = message.id === selectedMessageId; + + const selectedReaction = ( + (message.reactions || []).find(re => re.fromId === ourConversationId) || + {} + ).emoji; + + const authorId = getContactId(message, { + conversationSelector, + ourConversationId, + ourNumber, + ourACI, + }); + const contactNameColor = contactNameColorSelector(conversationId, authorId); + + const { conversationColor, customColor } = + getConversationColorAttributes(conversation); + + return { + attachments, + author, + bodyRanges, + previews, + quote, + reactions, + storyReplyContext, + textAttachment, + payment, + canDeleteForEveryone: canDeleteForEveryone(message), + canDownload: canDownload(message, conversationSelector), + canReact: canReact(message, ourConversationId, conversationSelector), + canReply: canReply(message, ourConversationId, conversationSelector), + canRetry: hasErrors(message), + canRetryDeleteForEveryone: canRetryDeleteForEveryone(message), + contact: getPropsForEmbeddedContact(message, regionCode, accountSelector), + contactNameColor, + conversationColor, + conversationId, + conversationTitle: conversation.title, + conversationType: isGroup ? 'group' : 'direct', + customColor, + deletedForEveryone: message.deletedForEveryone || false, + direction: isIncoming(message) ? 'incoming' : 'outgoing', + displayLimit: message.displayLimit, + expirationLength, + expirationTimestamp: calculateExpirationTimestamp({ + expireTimer, + expirationStartTimestamp, + }), + giftBadge: message.giftBadge, + id: message.id, + isBlocked: conversation.isBlocked || false, + isMessageRequestAccepted: conversation?.acceptedMessageRequest ?? true, + isSelected, + isSelectedCounter: isSelected ? selectedMessageCounter : undefined, + isSticker: Boolean(sticker), + isTapToView: isMessageTapToView, + isTapToViewError: + isMessageTapToView && isIncoming(message) && message.isTapToViewInvalid, + isTapToViewExpired: isMessageTapToView && message.isErased, + readStatus: message.readStatus ?? ReadStatus.Read, + selectedReaction, + status: getMessagePropStatus(message, ourConversationId), + text: message.body, + textDirection: getTextDirection(message.body), + timestamp: message.sent_at, + }; + }, + { + name: 'getPropsForMessage', + } +); // This is getPropsForMessage but wrapped in reselect's createSelector so that // we can derive all of the selector dependencies that getPropsForMessage @@ -893,19 +794,6 @@ export const getMessagePropsSelector = createSelector( } ); -export const getBubblePropsForMessage = createSelectorCreator(memoizeByRoot)( - // `memoizeByRoot` requirement - identity, - - getPropsForMessage, - - (_, data): TimelineItemType => ({ - type: 'message' as const, - data, - timestamp: data.timestamp, - }) -); - // Top-level prop generation for the message bubble export function getPropsForBubble( message: MessageWithUIFieldsType, @@ -1038,7 +926,13 @@ export function getPropsForBubble( }; } - return getBubblePropsForMessage(message, options); + const data = getPropsForMessage(message, options); + + return { + type: 'message' as const, + data, + timestamp: data.timestamp, + }; } function getPropsForPaymentEvent( @@ -1047,7 +941,7 @@ function getPropsForPaymentEvent( ): Omit { return { sender: conversationSelector(message.sourceUuid), - conversation: conversationSelector(message.conversationId), + conversation: getConversation(message, conversationSelector), event: message.payment, }; } @@ -1583,7 +1477,7 @@ function getPropsForDeliveryIssue( { conversationSelector }: GetPropsForBubbleOptions ): DeliveryIssuePropsType { const sender = conversationSelector(message.sourceUuid); - const conversation = conversationSelector(message.conversationId); + const conversation = getConversation(message, conversationSelector); return { sender, diff --git a/ts/state/selectors/timeline.ts b/ts/state/selectors/timeline.ts new file mode 100644 index 00000000000..70d9c7b03c7 --- /dev/null +++ b/ts/state/selectors/timeline.ts @@ -0,0 +1,72 @@ +// Copyright 2021-2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { TimelineItemType } from '../../components/conversation/TimelineItem'; +import { proxyMemoize } from '../../util/proxyMemoize'; + +import type { StateType } from '../reducer'; +import type { MessageWithUIFieldsType } from '../ducks/conversations'; +import { + getContactNameColorSelector, + getConversationSelector, + getSelectedMessage, + getMessages, +} from './conversations'; +import { getAccountSelector } from './accounts'; +import { + getRegionCode, + getUserConversationId, + getUserNumber, + getUserACI, + getUserPNI, +} from './user'; +import { getActiveCall, getCallSelector } from './calling'; +import { getPropsForBubble } from './message'; + +const getTimelineItemInner = proxyMemoize( + (message: MessageWithUIFieldsType, state: StateType): TimelineItemType => { + const selectedMessage = getSelectedMessage(state); + const conversationSelector = getConversationSelector(state); + const regionCode = getRegionCode(state); + const ourNumber = getUserNumber(state); + const ourACI = getUserACI(state); + const ourPNI = getUserPNI(state); + const ourConversationId = getUserConversationId(state); + const callSelector = getCallSelector(state); + const activeCall = getActiveCall(state); + const accountSelector = getAccountSelector(state); + const contactNameColorSelector = getContactNameColorSelector(state); + + return getPropsForBubble(message, { + conversationSelector, + ourConversationId, + ourNumber, + ourACI, + ourPNI, + regionCode, + selectedMessageId: selectedMessage?.id, + selectedMessageCounter: selectedMessage?.counter, + contactNameColorSelector, + callSelector, + activeCall, + accountSelector, + }); + }, + { + name: 'getTimelineItemInner', + } +); + +export const getTimelineItem = ( + state: StateType, + id: string +): TimelineItemType | undefined => { + const messageLookup = getMessages(state); + + const message = messageLookup[id]; + if (!message) { + return undefined; + } + + return getTimelineItemInner(message, state); +}; diff --git a/ts/state/smart/Timeline.tsx b/ts/state/smart/Timeline.tsx index 955b343a87a..af391539313 100644 --- a/ts/state/smart/Timeline.tsx +++ b/ts/state/smart/Timeline.tsx @@ -17,12 +17,12 @@ import type { ConversationType } from '../ducks/conversations'; import { getIntl, getTheme } from '../selectors/user'; import { + getMessages, getConversationByUuidSelector, getConversationMessagesSelector, getConversationSelector, getConversationsByTitleSelector, getInvitedContactsForNewlyCreatedGroup, - getMessageSelector, getSelectedMessage, } from '../selectors/conversations'; @@ -229,9 +229,8 @@ const mapStateToProps = (state: StateType, props: ExternalProps) => { const conversationMessages = getConversationMessagesSelector(state)(id); const selectedMessage = getSelectedMessage(state); - const messageSelector = getMessageSelector(state); const getTimestampForMessage = (messageId: string): undefined | number => - messageSelector(messageId)?.timestamp; + getMessages(state)[messageId]?.timestamp; return { id, diff --git a/ts/state/smart/TimelineItem.tsx b/ts/state/smart/TimelineItem.tsx index b7c3b66e11f..dee8d1000a4 100644 --- a/ts/state/smart/TimelineItem.tsx +++ b/ts/state/smart/TimelineItem.tsx @@ -13,9 +13,9 @@ import { getPreferredBadgeSelector } from '../selectors/badges'; import { getIntl, getInteractionMode, getTheme } from '../selectors/user'; import { getConversationSelector, - getMessageSelector, getSelectedMessage, } from '../selectors/conversations'; +import { getTimelineItem } from '../selectors/timeline'; import { areMessagesInSameGroup, shouldCurrentMessageHideMetadata, @@ -55,13 +55,13 @@ const mapStateToProps = (state: StateType, props: ExternalProps) => { unreadIndicatorPlacement, } = props; - const messageSelector = getMessageSelector(state); - - const item = messageSelector(messageId); + const item = getTimelineItem(state, messageId); const previousItem = previousMessageId - ? messageSelector(previousMessageId) + ? getTimelineItem(state, previousMessageId) + : undefined; + const nextItem = nextMessageId + ? getTimelineItem(state, nextMessageId) : undefined; - const nextItem = nextMessageId ? messageSelector(nextMessageId) : undefined; const selectedMessage = getSelectedMessage(state); const isSelected = Boolean( diff --git a/ts/test-both/util/memoizeByRoot.ts b/ts/test-both/util/memoizeByRoot.ts deleted file mode 100644 index 618d42ce8e5..00000000000 --- a/ts/test-both/util/memoizeByRoot.ts +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2021 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import { assert } from 'chai'; -import * as sinon from 'sinon'; - -import { memoizeByRoot } from '../../util/memoizeByRoot'; - -class Root {} - -describe('memoizeByRoot', () => { - it('should memoize by last passed arguments', () => { - const root = new Root(); - - const stub = sinon.stub(); - stub.withArgs(sinon.match.same(root), 1).returns(1); - stub.withArgs(sinon.match.same(root), 2).returns(2); - - const fn = memoizeByRoot(stub); - - assert.strictEqual(fn(root, 1), 1); - assert.strictEqual(fn(root, 1), 1); - assert.isTrue(stub.calledOnce); - - assert.strictEqual(fn(root, 2), 2); - assert.strictEqual(fn(root, 2), 2); - assert.isTrue(stub.calledTwice); - - assert.strictEqual(fn(root, 1), 1); - assert.strictEqual(fn(root, 1), 1); - assert.isTrue(stub.calledThrice); - }); - - it('should memoize results by root', () => { - const rootA = new Root(); - const rootB = new Root(); - - const stub = sinon.stub(); - stub.withArgs(sinon.match.same(rootA), 1).returns(1); - stub.withArgs(sinon.match.same(rootA), 2).returns(2); - stub.withArgs(sinon.match.same(rootB), 1).returns(3); - stub.withArgs(sinon.match.same(rootB), 2).returns(4); - - const fn = memoizeByRoot(stub); - - assert.strictEqual(fn(rootA, 1), 1); - assert.strictEqual(fn(rootB, 1), 3); - assert.strictEqual(fn(rootA, 1), 1); - assert.strictEqual(fn(rootB, 1), 3); - assert.isTrue(stub.calledTwice); - - assert.strictEqual(fn(rootA, 2), 2); - assert.strictEqual(fn(rootB, 2), 4); - assert.strictEqual(fn(rootA, 2), 2); - assert.strictEqual(fn(rootB, 2), 4); - assert.strictEqual(stub.callCount, 4); - - assert.strictEqual(fn(rootA, 1), 1); - assert.strictEqual(fn(rootB, 1), 3); - assert.strictEqual(stub.callCount, 6); - }); -}); diff --git a/ts/util/isShallowEqual.ts b/ts/util/isShallowEqual.ts index 275395219a5..10c9326fa29 100644 --- a/ts/util/isShallowEqual.ts +++ b/ts/util/isShallowEqual.ts @@ -1,17 +1,31 @@ // Copyright 2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -export function isShallowEqual>( - a: Obj, - b: Obj -): boolean { +export function isShallowEqual(a: unknown, b: unknown): boolean { + if (a === b) { + return true; + } + if (typeof a !== typeof b) { + return false; + } + + if (a == null || b == null) { + return false; + } + if (typeof a !== 'object') { + return false; + } + const keys = Object.keys(a); if (keys.length !== Object.keys(b).length) { return false; } for (const key of keys) { - if (a[key] !== b[key]) { + if ( + (a as Record)[key] !== + (b as Record)[key] + ) { return false; } } diff --git a/ts/util/memoizeByRoot.ts b/ts/util/memoizeByRoot.ts deleted file mode 100644 index e2d3290d6da..00000000000 --- a/ts/util/memoizeByRoot.ts +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2021 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import { defaultMemoize } from 'reselect'; - -import { strictAssert } from './assert'; - -// The difference between the function below and `defaultMemoize` from -// `reselect` is that it supports multiple "root" states. `reselect` is designed -// to interact with a single redux store and by default it memoizes only the -// last result of the selector (matched by its arguments). This works well when -// applied to singular entities living in the redux's state, but we need to -// apply selector to multitide of conversations and messages. -// -// The way it works is that it adds an extra memoization step that uses the -// first argument ("root") as a key in a weak map, and then applies the default -// `reselect`'s memoization function to the rest of the arguments. This way -// we essentially get a weak map of selectors by the "root". - -// eslint-disable-next-line @typescript-eslint/ban-types -export function memoizeByRoot( - fn: F, - equalityCheck?: (a: T, b: T) => boolean -): F { - // eslint-disable-next-line @typescript-eslint/ban-types - const cache = new WeakMap(); - - const wrap = (root: unknown, ...rest: Array): unknown => { - strictAssert( - typeof root === 'object' && root != null, - 'Root is not object' - ); - - let partial = cache.get(root); - if (!partial) { - partial = defaultMemoize((...args: Array): unknown => { - return fn(root, ...args); - }, equalityCheck); - - cache.set(root, partial); - } - - return partial(...rest); - }; - - return wrap as unknown as F; -} diff --git a/ts/util/proxyMemoize.ts b/ts/util/proxyMemoize.ts new file mode 100644 index 00000000000..c4a9626e020 --- /dev/null +++ b/ts/util/proxyMemoize.ts @@ -0,0 +1,78 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { createProxy, getUntracked, isChanged } from 'proxy-compare'; + +export type ExcludeNull = Exclude; + +export type ProxyMemoizeOptions = Readonly<{ + // For debugging + name: string; + + equalityFn?: (prev: Result, next: Result) => boolean; +}>; + +export function proxyMemoize, Result>( + fn: (...params: Params) => ExcludeNull, + { equalityFn }: ProxyMemoizeOptions> +): (...param: Params) => ExcludeNull { + type CacheEntryType = Readonly<{ + params: Params; + result: ExcludeNull; + }>; + + const cache = new WeakMap(); + const affected = new WeakMap(); + const proxyCache = new WeakMap(); + const changedCache = new WeakMap(); + + return (...params: Params): ExcludeNull => { + if (params.length < 1) { + throw new Error('At least one parameter is required'); + } + + const cacheKey = params[0]; + + const entry = cache.get(cacheKey); + + if (entry && entry.params.length === params.length) { + let isValid = true; + for (const [i, cachedParam] of entry.params.entries()) { + // Proxy wasn't even touched - we are good to go. + const wasUsed = affected.has(cachedParam); + if (!wasUsed) { + continue; + } + + if (isChanged(cachedParam, params[i], affected, changedCache)) { + isValid = false; + break; + } + } + if (isValid) { + return entry.result; + } + } + + const proxies = params.map(param => + createProxy(param, affected, proxyCache) + ) as unknown as Params; + + const trackedResult = fn(...proxies); + const untrackedResult = getUntracked(trackedResult); + + // eslint-disable-next-line eqeqeq + let result = untrackedResult === null ? trackedResult : untrackedResult; + + // Try to reuse result if custom equality check is configured. + if (entry && equalityFn && equalityFn(entry.result, result)) { + ({ result } = entry); + } + + cache.set(cacheKey, { + params, + result, + }); + return result; + }; +} diff --git a/yarn.lock b/yarn.lock index 024f509df3f..fc525ac2545 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14369,6 +14369,11 @@ proxy-agent@5.0.0: proxy-from-env "^1.0.0" socks-proxy-agent "^5.0.0" +proxy-compare@2.3.0: + version "2.3.0" + resolved "https://registry.yarnpkg.com/proxy-compare/-/proxy-compare-2.3.0.tgz#ac9633ae52918ff9c9fcc54dfe6316c7a02d20ee" + integrity sha512-c3L2CcAi7f7pvlD0D7xsF+2CQIW8C3HaYx2Pfgq8eA4HAl3GAH6/dVYsyBbYF/0XJs2ziGLrzmz5fmzPm6A0pQ== + proxy-from-env@^1.0.0, proxy-from-env@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/proxy-from-env/-/proxy-from-env-1.1.0.tgz#e102f16ca355424865755d2c9e8ea4f24d58c3e2"