fix(threads): review-wave fixes for per-thread notifications
- useRoomsListener now PREPENDS the emitting Room (was appended): the SDK emits RoomEvent.UnreadNotifications with VARIABLE arity (0/1/2 args), so a trailing extra arg landed in the wrong positional slot on the most common room-count sync path — room.isSpaceRoom() threw inside the SDK emit loop and the badge PUT never ran. Both consumers updated (CONFIRMED HIGH review finding). - roomToUnread: SpaceChild RESET now passes the thread prefs so muted-thread subtraction survives space-child state changes. Reviewer also verified: badge subtraction math exact (no double-subtraction), encrypted thread replies caught by the timeline guard (m.relates_to is cleartext), fresh prefs flow to handlers, single-owner wiring load-bearing. Documented-acceptable: hasCurrentUserParticipated can lag until the server bundle refreshes after your first reply; dedupe maps grow per-session only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -16,16 +16,18 @@ import {
|
|||||||
* need to memoize it — changing the handler identity never re-attaches the
|
* need to memoize it — changing the handler identity never re-attaches the
|
||||||
* per-room listeners.
|
* per-room listeners.
|
||||||
*
|
*
|
||||||
* The emitting {@link Room} is appended as a FINAL extra argument after the
|
* The emitting {@link Room} is PREPENDED as the first argument, before the
|
||||||
* event's own args: several room-level SDK events (e.g.
|
* event's own args: several room-level SDK events (e.g.
|
||||||
* `RoomEvent.UnreadNotifications`) don't include the room in their payload,
|
* `RoomEvent.UnreadNotifications`) don't include the room in their payload,
|
||||||
* which callers need for per-room updates. Handlers that don't care can simply
|
* which callers need for per-room updates. Prepending (not appending) is
|
||||||
* ignore it.
|
* load-bearing — some SDK events emit with VARIABLE arity
|
||||||
|
* (UnreadNotifications fires with 0, 1, or 2 args), so a trailing extra arg
|
||||||
|
* would land in a different positional slot per emit.
|
||||||
*/
|
*/
|
||||||
export function useRoomsListener<E extends RoomEmittedEvents>(
|
export function useRoomsListener<E extends RoomEmittedEvents>(
|
||||||
mx: MatrixClient,
|
mx: MatrixClient,
|
||||||
event: E,
|
event: E,
|
||||||
handler: (...args: [...Parameters<RoomEventHandlerMap[E]>, Room]) => void,
|
handler: (room: Room, ...args: Parameters<RoomEventHandlerMap[E]>) => void,
|
||||||
): void {
|
): void {
|
||||||
const handlerRef = useRef(handler);
|
const handlerRef = useRef(handler);
|
||||||
handlerRef.current = handler;
|
handlerRef.current = handler;
|
||||||
@@ -39,9 +41,9 @@ export function useRoomsListener<E extends RoomEmittedEvents>(
|
|||||||
const attach = (room: Room) => {
|
const attach = (room: Room) => {
|
||||||
if (attached.has(room.roomId)) return;
|
if (attached.has(room.roomId)) return;
|
||||||
// Per-room trampoline: forwards to the current ref value with the
|
// Per-room trampoline: forwards to the current ref value with the
|
||||||
// emitting room appended.
|
// emitting room PREPENDED (stable slot regardless of emit arity).
|
||||||
const roomHandler = (...args: unknown[]) =>
|
const roomHandler = (...args: unknown[]) =>
|
||||||
(handlerRef.current as (...a: unknown[]) => void)(...args, room);
|
(handlerRef.current as (...a: unknown[]) => void)(room, ...args);
|
||||||
attached.set(room.roomId, roomHandler);
|
attached.set(room.roomId, roomHandler);
|
||||||
// `event`/`roomHandler` are correlated through E but TS can't prove it
|
// `event`/`roomHandler` are correlated through E but TS can't prove it
|
||||||
// for the open generic, so we assert at the boundary.
|
// for the open generic, so we assert at the boundary.
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import {
|
|||||||
Room,
|
Room,
|
||||||
RoomEvent,
|
RoomEvent,
|
||||||
RoomEventHandlerMap,
|
RoomEventHandlerMap,
|
||||||
|
Thread,
|
||||||
ThreadEvent,
|
ThreadEvent,
|
||||||
} from 'matrix-js-sdk';
|
} from 'matrix-js-sdk';
|
||||||
import { focusAssistActiveAtom } from '../../state/focusAssist';
|
import { focusAssistActiveAtom } from '../../state/focusAssist';
|
||||||
@@ -446,8 +447,10 @@ function MessageNotifications() {
|
|||||||
};
|
};
|
||||||
}, [mx, notificationSelected, selectedRoomId, deliverNotification]);
|
}, [mx, notificationSelected, selectedRoomId, deliverNotification]);
|
||||||
|
|
||||||
const handleNewReply = useCallback<RoomEventHandlerMap[ThreadEvent.NewReply]>(
|
const handleNewReply = useCallback(
|
||||||
(thread, mEvent) => {
|
// useRoomsListener prepends the emitting Room; the thread's own room lookup
|
||||||
|
// below is kept as the authority (identical object in practice).
|
||||||
|
(_room: Room, thread: Thread, mEvent: MatrixEvent) => {
|
||||||
if (mx.getSyncState() !== 'SYNCING') return;
|
if (mx.getSyncState() !== 'SYNCING') return;
|
||||||
const room = mx.getRoom(thread.roomId);
|
const room = mx.getRoom(thread.roomId);
|
||||||
if (!room || room.isSpaceRoom()) return;
|
if (!room || room.isSpaceRoom()) return;
|
||||||
@@ -457,8 +460,7 @@ function MessageNotifications() {
|
|||||||
// Suppress when the user is actively looking at this thread (or the inbox).
|
// Suppress when the user is actively looking at this thread (or the inbox).
|
||||||
if (
|
if (
|
||||||
document.hasFocus() &&
|
document.hasFocus() &&
|
||||||
(notificationSelected ||
|
(notificationSelected || (selectedRoomId === thread.roomId && activeThreadId === thread.id))
|
||||||
(selectedRoomId === thread.roomId && activeThreadId === thread.id))
|
|
||||||
) {
|
) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,7 +4,6 @@ import {
|
|||||||
IRoomTimelineData,
|
IRoomTimelineData,
|
||||||
MatrixClient,
|
MatrixClient,
|
||||||
MatrixEvent,
|
MatrixEvent,
|
||||||
NotificationCount,
|
|
||||||
Room,
|
Room,
|
||||||
RoomEvent,
|
RoomEvent,
|
||||||
SyncState,
|
SyncState,
|
||||||
@@ -282,18 +281,15 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo
|
|||||||
|
|
||||||
// RoomEvent.UnreadNotifications is emitted room-level only (never re-emitted
|
// RoomEvent.UnreadNotifications is emitted room-level only (never re-emitted
|
||||||
// client-side), so the main Timeline pathway misses thread-count changes and
|
// client-side), so the main Timeline pathway misses thread-count changes and
|
||||||
// room badges lag. useRoomsListener appends the emitting Room as the final
|
// room badges lag. useRoomsListener PREPENDS the emitting Room (the SDK emits
|
||||||
// arg, making this a surgical per-room PUT (not a full RESET per emit) with
|
// this event with variable arity — 0/1/2 args — so only a leading slot is
|
||||||
// muted-thread subtraction re-applied. Room-mute keeps its DELETE semantics.
|
// positionally stable), making this a surgical per-room PUT with muted-thread
|
||||||
|
// subtraction re-applied. Room-mute keeps its DELETE semantics.
|
||||||
useRoomsListener(
|
useRoomsListener(
|
||||||
mx,
|
mx,
|
||||||
RoomEvent.UnreadNotifications,
|
RoomEvent.UnreadNotifications,
|
||||||
useCallback(
|
useCallback(
|
||||||
(
|
(room: Room) => {
|
||||||
_unreadNotifications: NotificationCount | undefined,
|
|
||||||
_threadId: string | undefined,
|
|
||||||
room: Room,
|
|
||||||
) => {
|
|
||||||
if (room.isSpaceRoom()) return;
|
if (room.isSpaceRoom()) return;
|
||||||
if (getNotificationType(mx, room.roomId) === NotificationType.Mute) {
|
if (getNotificationType(mx, room.roomId) === NotificationType.Mute) {
|
||||||
setUnreadAtom({ type: 'DELETE', roomId: room.roomId });
|
setUnreadAtom({ type: 'DELETE', roomId: room.roomId });
|
||||||
@@ -333,7 +329,7 @@ export const useBindRoomToUnreadAtom = (mx: MatrixClient, unreadAtom: typeof roo
|
|||||||
if (mEvent.getType() === StateEvent.SpaceChild) {
|
if (mEvent.getType() === StateEvent.SpaceChild) {
|
||||||
setUnreadAtom({
|
setUnreadAtom({
|
||||||
type: 'RESET',
|
type: 'RESET',
|
||||||
unreadInfos: getUnreadInfos(mx),
|
unreadInfos: getUnreadInfos(mx, threadNotificationsRef.current),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user