fix(threads): review-wave fixes — decryption re-render, receipt dedupe, chip perf
Two-reviewer audit of the thread stack; confirmed findings fixed: - ThreadTimeline: wrap encrypted events in EncryptedContent so a live-arriving E2EE reply re-renders when its key decrypts (decryption emits neither RoomEvent.Timeline nor ThreadEvent.Update — previously stuck at "Unable to decrypt"). - ThreadPanel: mark-read deduped on the latest event id (RoomEvent.Timeline re-emits per backfilled event/edit/reaction; previously up to N receipt POSTs per panel open) + rejection handled with retry. - RoomTimeline: ThreadSummary chips now mount only for events carrying thread data (each chip holds a room-level listener; one per rendered message would blow the SDK's 100-listener emitter cap) with a single room-level ThreadEvent.New tick for new-thread liveness. - useThreadPendingEvents: keep a sent reply visible through the /send-response→ /sync window (was flashing out of the pending strip before landing). - ThreadTimeline: reseed the window on RoomEvent.TimelineReset (gappy sync left a detached timeline). Documented-acceptable (reviewer-noted): thread typing shows as room typing (no per-thread typing in the spec; Element matches), thread panel + members drawer can be open together, scheduled-send is thread-unaware but unreachable there. Gates: tsc clean, eslint 0 errors, build OK, 616/617 tests (1 IDB skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -18,9 +18,11 @@ import {
|
|||||||
IContent,
|
IContent,
|
||||||
MatrixClient,
|
MatrixClient,
|
||||||
MatrixEvent,
|
MatrixEvent,
|
||||||
|
RelationType,
|
||||||
Room,
|
Room,
|
||||||
RoomEvent,
|
RoomEvent,
|
||||||
RoomEventHandlerMap,
|
RoomEventHandlerMap,
|
||||||
|
ThreadEvent,
|
||||||
} from 'matrix-js-sdk';
|
} from 'matrix-js-sdk';
|
||||||
import { HTMLReactParserOptions } from 'html-react-parser';
|
import { HTMLReactParserOptions } from 'html-react-parser';
|
||||||
import classNames from 'classnames';
|
import classNames from 'classnames';
|
||||||
@@ -477,6 +479,19 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli
|
|||||||
|
|
||||||
const setReplyDraft = useSetAtom(roomIdToReplyDraftAtomFamily(room.roomId));
|
const setReplyDraft = useSetAtom(roomIdToReplyDraftAtomFamily(room.roomId));
|
||||||
const setActiveThreadId = useSetAtom(roomIdToActiveThreadIdAtomFamily(room.roomId));
|
const setActiveThreadId = useSetAtom(roomIdToActiveThreadIdAtomFamily(room.roomId));
|
||||||
|
// Thread summary chips only mount for events that already carry thread data
|
||||||
|
// (perf: a chip subscribes room-level listeners, so mounting one per rendered
|
||||||
|
// message would exceed the SDK's emitter cap). This single room-level
|
||||||
|
// ThreadEvent.New subscription re-renders the timeline once when a brand-new
|
||||||
|
// thread appears, so the root's chip shows up without unrelated activity.
|
||||||
|
const [, setThreadNewTick] = useState(0);
|
||||||
|
useEffect(() => {
|
||||||
|
const handleThreadNew = () => setThreadNewTick((c) => c + 1);
|
||||||
|
room.on(ThreadEvent.New, handleThreadNew);
|
||||||
|
return () => {
|
||||||
|
room.removeListener(ThreadEvent.New, handleThreadNew);
|
||||||
|
};
|
||||||
|
}, [room]);
|
||||||
const powerLevels = usePowerLevelsContext();
|
const powerLevels = usePowerLevelsContext();
|
||||||
const creators = useRoomCreators(room);
|
const creators = useRoomCreators(room);
|
||||||
|
|
||||||
@@ -1136,7 +1151,9 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli
|
|||||||
onReactionToggle={handleReactionToggle}
|
onReactionToggle={handleReactionToggle}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
{(!threadRootId || threadRootId === mEventId) && (
|
{(!threadRootId || threadRootId === mEventId) &&
|
||||||
|
(mEvent.getThread() !== undefined ||
|
||||||
|
mEvent.getServerAggregatedRelation(RelationType.Thread) !== undefined) && (
|
||||||
<ThreadSummary rootEvent={mEvent} room={room} onOpen={setActiveThreadId} />
|
<ThreadSummary rootEvent={mEvent} room={room} onOpen={setActiveThreadId} />
|
||||||
)}
|
)}
|
||||||
</>
|
</>
|
||||||
@@ -1227,7 +1244,9 @@ export function RoomTimeline({ room, eventId, roomInputRef, editor }: RoomTimeli
|
|||||||
onReactionToggle={handleReactionToggle}
|
onReactionToggle={handleReactionToggle}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
{(!threadRootId || threadRootId === mEventId) && (
|
{(!threadRootId || threadRootId === mEventId) &&
|
||||||
|
(mEvent.getThread() !== undefined ||
|
||||||
|
mEvent.getServerAggregatedRelation(RelationType.Thread) !== undefined) && (
|
||||||
<ThreadSummary rootEvent={mEvent} room={room} onOpen={setActiveThreadId} />
|
<ThreadSummary rootEvent={mEvent} room={room} onOpen={setActiveThreadId} />
|
||||||
)}
|
)}
|
||||||
</>
|
</>
|
||||||
|
|||||||
@@ -95,10 +95,32 @@ export function ThreadPanel({ room, threadId, requestClose }: ThreadPanelProps)
|
|||||||
);
|
);
|
||||||
|
|
||||||
// Mark the thread read when the panel is open and on each new thread event.
|
// Mark the thread read when the panel is open and on each new thread event.
|
||||||
|
// Deduped on the latest event id: RoomEvent.Timeline re-emits per event during
|
||||||
|
// backfill and for every edit/reaction, and sendReadReceipt POSTs
|
||||||
|
// unconditionally — without the guard, opening a thread with N replies would
|
||||||
|
// fire up to N receipt requests at the same event.
|
||||||
|
const lastReadEventIdRef = useRef<string | undefined>(undefined);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
lastReadEventIdRef.current = undefined;
|
||||||
if (!thread) return undefined;
|
if (!thread) return undefined;
|
||||||
const markRead = () => {
|
const markRead = () => {
|
||||||
markThreadAsRead(mx, thread, privateReadReceipts);
|
const events = thread.liveTimeline.getEvents();
|
||||||
|
let latestId: string | undefined;
|
||||||
|
for (let i = events.length - 1; i >= 0; i -= 1) {
|
||||||
|
const evt = events[i];
|
||||||
|
if (evt && !evt.isSending()) {
|
||||||
|
latestId = evt.getId() ?? undefined;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (!latestId || latestId === lastReadEventIdRef.current) return;
|
||||||
|
lastReadEventIdRef.current = latestId;
|
||||||
|
markThreadAsRead(mx, thread, privateReadReceipts).catch(() => {
|
||||||
|
// Allow a retry on the next event if the receipt POST failed.
|
||||||
|
if (lastReadEventIdRef.current === latestId) {
|
||||||
|
lastReadEventIdRef.current = undefined;
|
||||||
|
}
|
||||||
|
});
|
||||||
};
|
};
|
||||||
markRead();
|
markRead();
|
||||||
thread.on(ThreadEvent.NewReply, markRead);
|
thread.on(ThreadEvent.NewReply, markRead);
|
||||||
|
|||||||
@@ -64,7 +64,7 @@ import {
|
|||||||
} from '../../../utils/room';
|
} from '../../../utils/room';
|
||||||
import { useSetting } from '../../../state/hooks/settings';
|
import { useSetting } from '../../../state/hooks/settings';
|
||||||
import { MessageLayout, settingsAtom } from '../../../state/settings';
|
import { MessageLayout, settingsAtom } from '../../../state/settings';
|
||||||
import { Message, Reactions } from '../message';
|
import { Message, Reactions, EncryptedContent } from '../message';
|
||||||
import { RenderMessageContent } from '../../../components/RenderMessageContent';
|
import { RenderMessageContent } from '../../../components/RenderMessageContent';
|
||||||
import { Image } from '../../../components/media';
|
import { Image } from '../../../components/media';
|
||||||
import { ImageViewer } from '../../../components/image-viewer';
|
import { ImageViewer } from '../../../components/image-viewer';
|
||||||
@@ -406,12 +406,22 @@ export function ThreadTimeline({ room, thread, editor }: ThreadTimelineProps) {
|
|||||||
setTimeline((ct) => ({ ...ct }));
|
setTimeline((ct) => ({ ...ct }));
|
||||||
};
|
};
|
||||||
const handleUpdate = () => setTimeline((ct) => ({ ...ct }));
|
const handleUpdate = () => setTimeline((ct) => ({ ...ct }));
|
||||||
|
// A gappy sync / updateThreadMetadata resets the thread's live timeline —
|
||||||
|
// the stored linkedTimelines would then point at a detached timeline, so
|
||||||
|
// reseed the window from the fresh liveTimeline.
|
||||||
|
const handleReset = () => {
|
||||||
|
setTimeline(getInitialThreadTimeline(thread, getLinkedTimelines(thread.liveTimeline)));
|
||||||
|
scrollToBottomRef.current.count += 1;
|
||||||
|
scrollToBottomRef.current.smooth = false;
|
||||||
|
};
|
||||||
|
|
||||||
thread.on(RoomEvent.Timeline, handleTimeline);
|
thread.on(RoomEvent.Timeline, handleTimeline);
|
||||||
thread.on(ThreadEvent.Update, handleUpdate);
|
thread.on(ThreadEvent.Update, handleUpdate);
|
||||||
|
thread.on(RoomEvent.TimelineReset, handleReset);
|
||||||
return () => {
|
return () => {
|
||||||
thread.removeListener(RoomEvent.Timeline, handleTimeline);
|
thread.removeListener(RoomEvent.Timeline, handleTimeline);
|
||||||
thread.removeListener(ThreadEvent.Update, handleUpdate);
|
thread.removeListener(ThreadEvent.Update, handleUpdate);
|
||||||
|
thread.removeListener(RoomEvent.TimelineReset, handleReset);
|
||||||
};
|
};
|
||||||
}, [thread]);
|
}, [thread]);
|
||||||
|
|
||||||
@@ -586,6 +596,11 @@ export function ThreadTimeline({ room, thread, editor }: ThreadTimelineProps) {
|
|||||||
|
|
||||||
const renderMessageContent = useCallback(
|
const renderMessageContent = useCallback(
|
||||||
(mEvent: MatrixEvent, mEventId: string, timelineSet: EventTimelineSet): ReactNode => {
|
(mEvent: MatrixEvent, mEventId: string, timelineSet: EventTimelineSet): ReactNode => {
|
||||||
|
// Evaluated lazily so EncryptedContent can re-run it (re-reading getType())
|
||||||
|
// after MatrixEventEvent.Decrypted fires — decryption re-emits NEITHER
|
||||||
|
// RoomEvent.Timeline nor ThreadEvent.Update, so without this wrapper a
|
||||||
|
// live-arriving encrypted reply would show "Unable to decrypt" forever.
|
||||||
|
const renderByType = (): ReactNode => {
|
||||||
if (mEvent.isRedacted()) {
|
if (mEvent.isRedacted()) {
|
||||||
return <RedactedContent reason={mEvent.getUnsigned().redacted_because?.content.reason} />;
|
return <RedactedContent reason={mEvent.getUnsigned().redacted_because?.content.reason} />;
|
||||||
}
|
}
|
||||||
@@ -641,6 +656,12 @@ export function ThreadTimeline({ room, thread, editor }: ThreadTimelineProps) {
|
|||||||
eventId={mEventId}
|
eventId={mEventId}
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
if (mEvent.getType() === MessageEvent.RoomMessageEncrypted) {
|
||||||
|
return <EncryptedContent mEvent={mEvent}>{renderByType}</EncryptedContent>;
|
||||||
|
}
|
||||||
|
return renderByType();
|
||||||
},
|
},
|
||||||
[room, mediaAutoLoad, showUrlPreview, htmlReactParserOptions, linkifyOpts, messageLayout],
|
[room, mediaAutoLoad, showUrlPreview, htmlReactParserOptions, linkifyOpts, messageLayout],
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { useCallback, useEffect, useState } from 'react';
|
import { useCallback, useEffect, useState } from 'react';
|
||||||
import {
|
import {
|
||||||
|
EventStatus,
|
||||||
EventTimeline,
|
EventTimeline,
|
||||||
MatrixClient,
|
MatrixClient,
|
||||||
MatrixEvent,
|
MatrixEvent,
|
||||||
@@ -121,7 +122,15 @@ export const useThreadPendingEvents = (
|
|||||||
|
|
||||||
const alreadyInThread =
|
const alreadyInThread =
|
||||||
eventId !== undefined && thread?.findEventById(eventId) !== undefined;
|
eventId !== undefined && thread?.findEventById(eventId) !== undefined;
|
||||||
const stillPending = isPendingThreadReply(event, threadRootId) && !alreadyInThread;
|
// Keep a tracked event through the SENT window too: the /send response
|
||||||
|
// flips status to SENT before /sync delivers the event into the thread
|
||||||
|
// timeline — dropping it there would make the message flash out of view.
|
||||||
|
// It falls out on the next LocalEchoUpdated once findEventById sees it.
|
||||||
|
const trackedAndAwaitingSync =
|
||||||
|
event.status === EventStatus.SENT &&
|
||||||
|
prev.some((e) => e === event || (eventId !== undefined && e.getId() === eventId));
|
||||||
|
const stillPending =
|
||||||
|
!alreadyInThread && (isPendingThreadReply(event, threadRootId) || trackedAndAwaitingSync);
|
||||||
|
|
||||||
if (stillPending) return [...without, event];
|
if (stillPending) return [...without, event];
|
||||||
return without.length === prev.length ? prev : without;
|
return without.length === prev.length ? prev : without;
|
||||||
|
|||||||
Reference in New Issue
Block a user