diff --git a/ts/background.ts b/ts/background.ts index 7046d9371..16d70a242 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -4,7 +4,8 @@ import { webFrame } from 'electron'; import { isNumber, noop } from 'lodash'; import { bindActionCreators } from 'redux'; -import { render, unstable_batchedUpdates as batchedUpdates } from 'react-dom'; +import { render } from 'react-dom'; +import { batch as batchDispatch } from 'react-redux'; import MessageReceiver from './textsecure/MessageReceiver'; import type { @@ -1043,7 +1044,7 @@ export async function startApp(): Promise { `${batch.length} into ${deduped.size}` ); - batchedUpdates(() => { + batchDispatch(() => { deduped.forEach(conversation => { conversationChanged(conversation.id, conversation.format()); }); @@ -1058,11 +1059,21 @@ export async function startApp(): Promise { maxSize: Infinity, }); - convoCollection.on('props-change', conversation => { + convoCollection.on('props-change', (conversation, isBatched) => { if (!conversation) { return; } + // `isBatched` is true when the `.set()` call on the conversation model + // already runs from within `react-redux`'s batch. Instead of batching + // the redux update for later - clear all queued updates and update + // immediately. + if (isBatched) { + changedConvoBatcher.removeAll(conversation); + conversationChanged(conversation.id, conversation.format()); + return; + } + changedConvoBatcher.add(conversation); }); convoCollection.on('reset', removeAllConversations); diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 66af03587..5ab4a1ab5 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -4,6 +4,7 @@ /* eslint-disable class-methods-use-this */ /* eslint-disable camelcase */ import { compact, isNumber } from 'lodash'; +import { batch as batchDispatch } from 'react-redux'; import type { ConversationAttributesType, ConversationModelCollectionType, @@ -201,6 +202,8 @@ export class ConversationModel extends window.Backbone private muteTimer?: NodeJS.Timer; + private isInReduxBatch = false; + // eslint-disable-next-line class-methods-use-this defaults(): Partial { return { @@ -318,7 +321,7 @@ export class ConversationModel extends window.Backbone this.oldCachedProps = this.cachedProps; } this.cachedProps = null; - this.trigger('props-change', this); + this.trigger('props-change', this, this.isInReduxBatch); } ); @@ -1613,14 +1616,21 @@ export class ConversationModel extends window.Backbone window.Signal.Data.updateConversation(this.attributes); } - incrementSentMessageCount({ save = true }: { save?: boolean } = {}): void { - this.set({ + incrementSentMessageCount({ dry = false }: { dry?: boolean } = {}): + | Partial + | undefined { + const update = { messageCount: (this.get('messageCount') || 0) + 1, sentMessageCount: (this.get('sentMessageCount') || 0) + 1, - }); - if (save) { - window.Signal.Data.updateConversation(this.attributes); + }; + + if (dry) { + return update; } + this.set(update); + window.Signal.Data.updateConversation(this.attributes); + + return undefined; } decrementSentMessageCount(): void { @@ -3530,28 +3540,8 @@ export class ConversationModel extends window.Backbone return; } - this.clearTypingTimers(); - - const { clearUnreadMetrics } = window.reduxActions.conversations; - clearUnreadMetrics(this.id); - - const mandatoryProfileSharingEnabled = window.Signal.RemoteConfig.isEnabled( - 'desktop.mandatoryProfileSharing' - ); - if (mandatoryProfileSharingEnabled && !this.get('profileSharing')) { - this.set({ profileSharing: true }); - } - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const destination = this.getSendTarget()!; - const recipients = this.getRecipients(); - const now = timestamp || Date.now(); - await this.maybeApplyUniversalTimer(false); - - const expireTimer = this.get('expireTimer'); - log.info( 'Sending message to conversation', this.idForLogging(), @@ -3559,6 +3549,20 @@ export class ConversationModel extends window.Backbone now ); + this.clearTypingTimers(); + + const mandatoryProfileSharingEnabled = window.Signal.RemoteConfig.isEnabled( + 'desktop.mandatoryProfileSharing' + ); + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const destination = this.getSendTarget()!; + const recipients = this.getRecipients(); + + await this.maybeApplyUniversalTimer(false); + + const expireTimer = this.get('expireTimer'); + const recipientMaybeConversations = map(recipients, identifier => window.ConversationController.get(identifier) ); @@ -3641,29 +3645,43 @@ export class ConversationModel extends window.Backbone const renderStart = Date.now(); - this.addSingleMessage(model); + this.isInReduxBatch = true; + batchDispatch(() => { + try { + const { clearUnreadMetrics } = window.reduxActions.conversations; + clearUnreadMetrics(this.id); + + const enableProfileSharing = Boolean( + mandatoryProfileSharingEnabled && !this.get('profileSharing') + ); + this.addSingleMessage(model); + + const draftProperties = dontClearDraft + ? {} + : { + draft: null, + draftTimestamp: null, + lastMessage: model.getNotificationText(), + lastMessageStatus: 'sending' as const, + }; + + this.set({ + ...draftProperties, + ...(enableProfileSharing ? { profileSharing: true } : {}), + ...this.incrementSentMessageCount({ dry: true }), + active_at: now, + timestamp: now, + isArchived: false, + }); + } finally { + this.isInReduxBatch = false; + } + }); + if (sticker) { await addStickerPackReference(model.id, sticker.packId); } - const draftProperties = dontClearDraft - ? {} - : { - draft: null, - draftTimestamp: null, - lastMessage: model.getNotificationText(), - lastMessageStatus: 'sending' as const, - }; - - this.set({ - ...draftProperties, - active_at: now, - timestamp: now, - isArchived: false, - }); - - this.incrementSentMessageCount({ save: false }); - const renderDuration = Date.now() - renderStart; if (renderDuration > SEND_REPORTING_THRESHOLD_MS) { diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index ed70acffa..8b772db09 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -1341,10 +1341,12 @@ function conversationChanged( .outboundMessagesPendingConversationVerification, id ) ?? []; - const messagesPending = await getMessagesById(messageIdsPending); - messagesPending.forEach(message => { - message.retrySend(); - }); + if (messageIdsPending.length) { + const messagesPending = await getMessagesById(messageIdsPending); + messagesPending.forEach(message => { + message.retrySend(); + }); + } } dispatch({ diff --git a/ts/test-both/util/batcher_test.ts b/ts/test-both/util/batcher_test.ts index d89a80041..1e72673d6 100644 --- a/ts/test-both/util/batcher_test.ts +++ b/ts/test-both/util/batcher_test.ts @@ -41,6 +41,26 @@ describe('batcher', () => { assert.ok(processBatch.calledOnceWith([1]), 'Partial batch after timeout'); }); + it('should remove scheduled items from a batch', async () => { + const processBatch = sinon.fake.resolves(undefined); + + const batcher = createBatcher({ + name: 'test', + wait: 5, + maxSize: 100, + processBatch, + }); + + batcher.add(1); + batcher.add(1); + batcher.add(2); + batcher.removeAll(1); + + await sleep(10); + + assert.ok(processBatch.calledOnceWith([2]), 'Remove all'); + }); + it('should flushAndWait a partial batch', async () => { const processBatch = sinon.fake.resolves(undefined); diff --git a/ts/util/batcher.ts b/ts/util/batcher.ts index 64cacd6ce..acd8054db 100644 --- a/ts/util/batcher.ts +++ b/ts/util/batcher.ts @@ -31,6 +31,7 @@ export type BatcherOptionsType = { export type BatcherType = { add: (item: ItemType) => void; + removeAll: (needle: ItemType) => void; anyPending: () => boolean; onIdle: () => Promise; flushAndWait: () => Promise; @@ -70,6 +71,10 @@ export function createBatcher( } } + function removeAll(needle: ItemType) { + items = items.filter(item => item !== needle); + } + function anyPending(): boolean { return queue.size > 0 || queue.pending > 0 || items.length > 0; } @@ -108,6 +113,7 @@ export function createBatcher( batcher = { add, + removeAll, anyPending, onIdle, flushAndWait, diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts index 349399df3..b55d66674 100644 --- a/ts/views/conversation_view.ts +++ b/ts/views/conversation_view.ts @@ -3,7 +3,7 @@ /* eslint-disable camelcase */ -import { unstable_batchedUpdates as batchedUpdates } from 'react-dom'; +import { batch as batchDispatch } from 'react-redux'; import { debounce, flatten, omit, throttle } from 'lodash'; import { render } from 'mustache'; @@ -3273,20 +3273,20 @@ export class ConversationView extends window.Backbone.View { log.info('Send pre-checks took', sendDelta, 'milliseconds'); - batchedUpdates(() => { - model.enqueueMessageForSend( - message, - attachments, - this.quote, - this.getLinkPreviewForSend(message), - undefined, // sticker - mentions, - { - sendHQImages, - timestamp, - } - ); + model.enqueueMessageForSend( + message, + attachments, + this.quote, + this.getLinkPreviewForSend(message), + undefined, // sticker + mentions, + { + sendHQImages, + timestamp, + } + ); + batchDispatch(() => { this.compositionApi.current?.reset(); model.setMarkedUnread(false); this.setQuoteMessage(null);