Prevent duplicated re-emitter setups in event-mapper (#2293)

Author: t3chguy

spec/unit/matrix-client.spec.ts

@@ -816,6 +816,9 @@ describe("MatrixClient", function() {
             },
             addPendingEvent: jest.fn(),
             updatePendingEvent: jest.fn(),
+            reEmitter: {
+                reEmit: jest.fn(),
+            },
         };
 
         beforeEach(() => {

src/client.ts

@@ -3779,6 +3779,15 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             localEvent.setThreadId(thread.id);
         }
 
+        // set up re-emitter for this new event - this is normally the job of EventMapper but we don't use it here
+        this.reEmitter.reEmit(localEvent, [
+            MatrixEventEvent.Replaced,
+            MatrixEventEvent.VisibilityChange,
+        ]);
+        room?.reEmitter.reEmit(localEvent, [
+            MatrixEventEvent.BeforeRedaction,
+        ]);
+
         // if this is a relation or redaction of an event
         // that hasn't been sent yet (e.g. with a local id starting with a ~)
         // then listen for the remote echo of that event so that by the time

src/event-mapper.ts

@@ -25,7 +25,7 @@ export interface MapperOpts {
 }
 
 export function eventMapperFor(client: MatrixClient, options: MapperOpts): EventMapper {
-    const preventReEmit = Boolean(options.preventReEmit);
+    let preventReEmit = Boolean(options.preventReEmit);
     const decrypt = options.decrypt !== false;
 
     function mapper(plainOldJsObject: Partial<IEvent>) {
@@ -43,6 +43,8 @@ export function eventMapperFor(client: MatrixClient, options: MapperOpts): Event
         } else {
             // merge the latest unsigned data from the server
             event.setUnsigned({ ...event.getUnsigned(), ...plainOldJsObject.unsigned });
+            // prevent doubling up re-emitters
+            preventReEmit = true;
         }
 
         const thread = room?.findThreadForEvent(event);

src/models/thread.ts

@@ -95,6 +95,7 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
         ]);
 
         this.room.on(MatrixEventEvent.BeforeRedaction, this.onBeforeRedaction);
+        this.room.on(RoomEvent.Redaction, this.onRedaction);
         this.room.on(RoomEvent.LocalEchoUpdated, this.onEcho);
         this.timelineSet.on(RoomEvent.Timeline, this.onEcho);
 
@@ -115,23 +116,24 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
         }
     }
 
-    private onBeforeRedaction = (event: MatrixEvent) => {
+    private onBeforeRedaction = (event: MatrixEvent, redaction: MatrixEvent) => {
         if (event?.isRelation(THREAD_RELATION_TYPE.name) &&
-            this.room.eventShouldLiveIn(event).threadId === this.id
+            this.room.eventShouldLiveIn(event).threadId === this.id &&
+            !redaction.status // only respect it when it succeeds
         ) {
             this.replyCount--;
             this.emit(ThreadEvent.Update, this);
         }
+    };
 
-        if (this.lastEvent?.getId() === event.getId()) {
-            const events = [...this.timelineSet.getLiveTimeline().getEvents()].reverse();
-            this.lastEvent = events.find(e => (
-                !e.isRedacted() &&
-                e.getId() !== event.getId() &&
-                e.isRelation(THREAD_RELATION_TYPE.name)
-            )) ?? this.rootEvent;
-            this.emit(ThreadEvent.NewReply, this, this.lastEvent);
-        }
+    private onRedaction = (event: MatrixEvent) => {
+        if (event.threadRootId !== this.id) return; // ignore redactions for other timelines
+        const events = [...this.timelineSet.getLiveTimeline().getEvents()].reverse();
+        this.lastEvent = events.find(e => (
+            !e.isRedacted() &&
+            e.isRelation(THREAD_RELATION_TYPE.name)
+        )) ?? this.rootEvent;
+        this.emit(ThreadEvent.Update, this);
     };
 
     private onEcho = (event: MatrixEvent) => {
@@ -142,7 +144,7 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
         // when threads are used over federation. That could result in the reply
         // count value drifting away from the value returned by the server
         const isThreadReply = event.isRelation(THREAD_RELATION_TYPE.name);
-        if (!this.lastEvent || (isThreadReply
+        if (!this.lastEvent || this.lastEvent.isRedacted() || (isThreadReply
             && (event.getId() !== this.lastEvent.getId())
             && (event.localTimestamp > this.lastEvent.localTimestamp))
         ) {