Use react-redux's batch instead of react's

This commit is contained in:
Fedor Indutny 2021-11-01 16:38:08 -07:00 committed by GitHub
parent 3190f95fac
commit 663cd77eac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 123 additions and 66 deletions

View file

@ -4,7 +4,8 @@
import { webFrame } from 'electron'; import { webFrame } from 'electron';
import { isNumber, noop } from 'lodash'; import { isNumber, noop } from 'lodash';
import { bindActionCreators } from 'redux'; 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 MessageReceiver from './textsecure/MessageReceiver';
import type { import type {
@ -1043,7 +1044,7 @@ export async function startApp(): Promise<void> {
`${batch.length} into ${deduped.size}` `${batch.length} into ${deduped.size}`
); );
batchedUpdates(() => { batchDispatch(() => {
deduped.forEach(conversation => { deduped.forEach(conversation => {
conversationChanged(conversation.id, conversation.format()); conversationChanged(conversation.id, conversation.format());
}); });
@ -1058,11 +1059,21 @@ export async function startApp(): Promise<void> {
maxSize: Infinity, maxSize: Infinity,
}); });
convoCollection.on('props-change', conversation => { convoCollection.on('props-change', (conversation, isBatched) => {
if (!conversation) { if (!conversation) {
return; 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); changedConvoBatcher.add(conversation);
}); });
convoCollection.on('reset', removeAllConversations); convoCollection.on('reset', removeAllConversations);

View file

@ -4,6 +4,7 @@
/* eslint-disable class-methods-use-this */ /* eslint-disable class-methods-use-this */
/* eslint-disable camelcase */ /* eslint-disable camelcase */
import { compact, isNumber } from 'lodash'; import { compact, isNumber } from 'lodash';
import { batch as batchDispatch } from 'react-redux';
import type { import type {
ConversationAttributesType, ConversationAttributesType,
ConversationModelCollectionType, ConversationModelCollectionType,
@ -201,6 +202,8 @@ export class ConversationModel extends window.Backbone
private muteTimer?: NodeJS.Timer; private muteTimer?: NodeJS.Timer;
private isInReduxBatch = false;
// eslint-disable-next-line class-methods-use-this // eslint-disable-next-line class-methods-use-this
defaults(): Partial<ConversationAttributesType> { defaults(): Partial<ConversationAttributesType> {
return { return {
@ -318,7 +321,7 @@ export class ConversationModel extends window.Backbone
this.oldCachedProps = this.cachedProps; this.oldCachedProps = this.cachedProps;
} }
this.cachedProps = null; 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); window.Signal.Data.updateConversation(this.attributes);
} }
incrementSentMessageCount({ save = true }: { save?: boolean } = {}): void { incrementSentMessageCount({ dry = false }: { dry?: boolean } = {}):
this.set({ | Partial<ConversationAttributesType>
| undefined {
const update = {
messageCount: (this.get('messageCount') || 0) + 1, messageCount: (this.get('messageCount') || 0) + 1,
sentMessageCount: (this.get('sentMessageCount') || 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 { decrementSentMessageCount(): void {
@ -3530,28 +3540,8 @@ export class ConversationModel extends window.Backbone
return; 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(); const now = timestamp || Date.now();
await this.maybeApplyUniversalTimer(false);
const expireTimer = this.get('expireTimer');
log.info( log.info(
'Sending message to conversation', 'Sending message to conversation',
this.idForLogging(), this.idForLogging(),
@ -3559,6 +3549,20 @@ export class ConversationModel extends window.Backbone
now 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 => const recipientMaybeConversations = map(recipients, identifier =>
window.ConversationController.get(identifier) window.ConversationController.get(identifier)
); );
@ -3641,29 +3645,43 @@ export class ConversationModel extends window.Backbone
const renderStart = Date.now(); 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) { if (sticker) {
await addStickerPackReference(model.id, sticker.packId); 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; const renderDuration = Date.now() - renderStart;
if (renderDuration > SEND_REPORTING_THRESHOLD_MS) { if (renderDuration > SEND_REPORTING_THRESHOLD_MS) {

View file

@ -1341,10 +1341,12 @@ function conversationChanged(
.outboundMessagesPendingConversationVerification, .outboundMessagesPendingConversationVerification,
id id
) ?? []; ) ?? [];
const messagesPending = await getMessagesById(messageIdsPending); if (messageIdsPending.length) {
messagesPending.forEach(message => { const messagesPending = await getMessagesById(messageIdsPending);
message.retrySend(); messagesPending.forEach(message => {
}); message.retrySend();
});
}
} }
dispatch({ dispatch({

View file

@ -41,6 +41,26 @@ describe('batcher', () => {
assert.ok(processBatch.calledOnceWith([1]), 'Partial batch after timeout'); 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<number>({
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 () => { it('should flushAndWait a partial batch', async () => {
const processBatch = sinon.fake.resolves(undefined); const processBatch = sinon.fake.resolves(undefined);

View file

@ -31,6 +31,7 @@ export type BatcherOptionsType<ItemType> = {
export type BatcherType<ItemType> = { export type BatcherType<ItemType> = {
add: (item: ItemType) => void; add: (item: ItemType) => void;
removeAll: (needle: ItemType) => void;
anyPending: () => boolean; anyPending: () => boolean;
onIdle: () => Promise<void>; onIdle: () => Promise<void>;
flushAndWait: () => Promise<void>; flushAndWait: () => Promise<void>;
@ -70,6 +71,10 @@ export function createBatcher<ItemType>(
} }
} }
function removeAll(needle: ItemType) {
items = items.filter(item => item !== needle);
}
function anyPending(): boolean { function anyPending(): boolean {
return queue.size > 0 || queue.pending > 0 || items.length > 0; return queue.size > 0 || queue.pending > 0 || items.length > 0;
} }
@ -108,6 +113,7 @@ export function createBatcher<ItemType>(
batcher = { batcher = {
add, add,
removeAll,
anyPending, anyPending,
onIdle, onIdle,
flushAndWait, flushAndWait,

View file

@ -3,7 +3,7 @@
/* eslint-disable camelcase */ /* 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 { debounce, flatten, omit, throttle } from 'lodash';
import { render } from 'mustache'; import { render } from 'mustache';
@ -3273,20 +3273,20 @@ export class ConversationView extends window.Backbone.View<ConversationModel> {
log.info('Send pre-checks took', sendDelta, 'milliseconds'); log.info('Send pre-checks took', sendDelta, 'milliseconds');
batchedUpdates(() => { model.enqueueMessageForSend(
model.enqueueMessageForSend( message,
message, attachments,
attachments, this.quote,
this.quote, this.getLinkPreviewForSend(message),
this.getLinkPreviewForSend(message), undefined, // sticker
undefined, // sticker mentions,
mentions, {
{ sendHQImages,
sendHQImages, timestamp,
timestamp, }
} );
);
batchDispatch(() => {
this.compositionApi.current?.reset(); this.compositionApi.current?.reset();
model.setMarkedUnread(false); model.setMarkedUnread(false);
this.setQuoteMessage(null); this.setQuoteMessage(null);