fix(dp): address TPVR review findings (DP2 redo, DP4 dedup, DP15 gap)

- DP2: the earlier seed fix was ineffective (allInvitesAtom populates post-mount,
  so first render is still empty). Rewrite to track invite room ids and stay
  'unarmed' until the initial sync settles (+3s grace), notifying only for ids that
  first appear after arming — robust to the async population race.
- DP4: batch the mutually-exclusive tag writes via Promise.all so a failure surfaces
  a single toast instead of one per operation.
- DP15: route the two remaining StateEvent writes (RoomSoundboardPack / RoomImagePack,
  which used an 'as unknown as keyof StateEvents' idiom the sweep missed) through the
  typed sendStateEvent helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-08 00:02:04 -04:00
parent 4fa4327a18
commit c2598d21cd
4 changed files with 78 additions and 39 deletions
@@ -5,6 +5,7 @@ import { useMatrixClient } from '../../hooks/useMatrixClient';
import { ImagePackContent } from './ImagePackContent'; import { ImagePackContent } from './ImagePackContent';
import { ImagePack, PackContent } from '../../plugins/custom-emoji'; import { ImagePack, PackContent } from '../../plugins/custom-emoji';
import { StateEvent } from '../../../types/matrix/room'; import { StateEvent } from '../../../types/matrix/room';
import { sendStateEvent } from '../../utils/room';
import { useRoomImagePack } from '../../hooks/useImagePacks'; import { useRoomImagePack } from '../../hooks/useImagePacks';
import { randomStr } from '../../utils/common'; import { randomStr } from '../../utils/common';
import { useRoomPermissions } from '../../hooks/useRoomPermissions'; import { useRoomPermissions } from '../../hooks/useRoomPermissions';
@@ -45,12 +46,7 @@ export function RoomImagePack({ room, stateKey }: RoomImagePackProps) {
const { address } = imagePack; const { address } = imagePack;
if (!address) return; if (!address) return;
await mx.sendStateEvent( await sendStateEvent(mx, address.roomId, StateEvent.PoniesRoomEmotes, packContent, address.stateKey);
address.roomId,
StateEvent.PoniesRoomEmotes as unknown as keyof import('matrix-js-sdk').StateEvents,
packContent as any,
address.stateKey,
);
}, },
[mx, imagePack], [mx, imagePack],
); );
@@ -5,6 +5,7 @@ import { useMatrixClient } from '../../hooks/useMatrixClient';
import { SoundboardPackEditor } from './SoundboardPackEditor'; import { SoundboardPackEditor } from './SoundboardPackEditor';
import { SoundboardContent, SoundboardPack } from '../../plugins/soundboard'; import { SoundboardContent, SoundboardPack } from '../../plugins/soundboard';
import { StateEvent } from '../../../types/matrix/room'; import { StateEvent } from '../../../types/matrix/room';
import { sendStateEvent } from '../../utils/room';
import { useRoomSoundboardPack } from '../../hooks/useSoundboardPacks'; import { useRoomSoundboardPack } from '../../hooks/useSoundboardPacks';
import { PackAddress } from '../../plugins/custom-emoji/PackAddress'; import { PackAddress } from '../../plugins/custom-emoji/PackAddress';
import { randomStr } from '../../utils/common'; import { randomStr } from '../../utils/common';
@@ -35,12 +36,7 @@ export function RoomSoundboardPack({ room, stateKey }: RoomSoundboardPackProps)
const handleUpdate = useCallback( const handleUpdate = useCallback(
async (content: SoundboardContent) => { async (content: SoundboardContent) => {
await mx.sendStateEvent( await sendStateEvent(mx, room.roomId, StateEvent.LotusSoundboardRoom, content, stateKey);
room.roomId,
StateEvent.LotusSoundboardRoom as unknown as keyof import('matrix-js-sdk').StateEvents,
content as never,
stateKey,
);
}, },
[mx, room.roomId, stateKey], [mx, room.roomId, stateKey],
); );
+10 -5
View File
@@ -344,9 +344,11 @@ const RoomNavItemMenu = forwardRef<HTMLDivElement, RoomNavItemMenuProps>(
if (isFavorite) { if (isFavorite) {
mx.deleteRoomTag(room.roomId, 'm.favourite').catch(notifyTagFailure); mx.deleteRoomTag(room.roomId, 'm.favourite').catch(notifyTagFailure);
} else { } else {
// Favourite and low-priority are mutually exclusive. // Favourite and low-priority are mutually exclusive. Batch both writes so a
if (isLowPriority) mx.deleteRoomTag(room.roomId, 'm.lowpriority').catch(notifyTagFailure); // failure surfaces a single toast, not one per operation.
mx.setRoomTag(room.roomId, 'm.favourite', { order: 0.5 }).catch(notifyTagFailure); const ops: Promise<unknown>[] = [mx.setRoomTag(room.roomId, 'm.favourite', { order: 0.5 })];
if (isLowPriority) ops.push(mx.deleteRoomTag(room.roomId, 'm.lowpriority'));
Promise.all(ops).catch(notifyTagFailure);
} }
requestClose(); requestClose();
}; };
@@ -355,8 +357,11 @@ const RoomNavItemMenu = forwardRef<HTMLDivElement, RoomNavItemMenuProps>(
if (isLowPriority) { if (isLowPriority) {
mx.deleteRoomTag(room.roomId, 'm.lowpriority').catch(notifyTagFailure); mx.deleteRoomTag(room.roomId, 'm.lowpriority').catch(notifyTagFailure);
} else { } else {
if (isFavorite) mx.deleteRoomTag(room.roomId, 'm.favourite').catch(notifyTagFailure); const ops: Promise<unknown>[] = [
mx.setRoomTag(room.roomId, 'm.lowpriority', { order: 0.5 }).catch(notifyTagFailure); mx.setRoomTag(room.roomId, 'm.lowpriority', { order: 0.5 }),
];
if (isFavorite) ops.push(mx.deleteRoomTag(room.roomId, 'm.favourite'));
Promise.all(ops).catch(notifyTagFailure);
} }
requestClose(); requestClose();
}; };
+58 -16
View File
@@ -2,10 +2,13 @@ import { useAtomValue, useSetAtom } from 'jotai';
import React, { ReactNode, useCallback, useEffect, useRef } from 'react'; import React, { ReactNode, useCallback, useEffect, useRef } from 'react';
import { useNavigate } from 'react-router-dom'; import { useNavigate } from 'react-router-dom';
import { import {
ClientEvent,
ClientEventHandlerMap,
MatrixEvent, MatrixEvent,
Room, Room,
RoomEvent, RoomEvent,
RoomEventHandlerMap, RoomEventHandlerMap,
SyncState,
Thread, Thread,
ThreadEvent, ThreadEvent,
} from 'matrix-js-sdk'; } from 'matrix-js-sdk';
@@ -22,7 +25,10 @@ import { NOTIFICATION_SOUND_MAP } from '../../utils/notificationSounds';
import { useSetting } from '../../state/hooks/settings'; import { useSetting } from '../../state/hooks/settings';
import { settingsAtom } from '../../state/settings'; import { settingsAtom } from '../../state/settings';
import { allInvitesAtom } from '../../state/room-list/inviteList'; import { allInvitesAtom } from '../../state/room-list/inviteList';
import { usePreviousValue } from '../../hooks/usePreviousValue';
// Grace period after the initial sync settles before invite notifications arm, so
// the async invite-atom population lands first and isn't mistaken for new invites.
const INVITE_NOTIFY_ARM_DELAY_MS = 3000;
import { useMatrixClient } from '../../hooks/useMatrixClient'; import { useMatrixClient } from '../../hooks/useMatrixClient';
import { getDirectRoomPath, getHomeRoomPath, getInboxInvitesPath } from '../pathUtils'; import { getDirectRoomPath, getHomeRoomPath, getInboxInvitesPath } from '../pathUtils';
import { mDirectAtom } from '../../state/mDirectList'; import { mDirectAtom } from '../../state/mDirectList';
@@ -134,13 +140,15 @@ function FaviconUpdater() {
function InviteNotifications() { function InviteNotifications() {
const audioRef = useRef<HTMLAudioElement>(null); const audioRef = useRef<HTMLAudioElement>(null);
const invites = useAtomValue(allInvitesAtom); const invites = useAtomValue(allInvitesAtom);
// Seed the baseline with the invite count present at mount, NOT 0. allInvitesAtom
// populates synchronously from cache on a warm reload while sync is already
// SYNCING, so a 0 seed made every pre-existing invite look "new" and re-fired the
// toast+sound on each reload. Starting from the current length means only a
// genuine INCREASE (a newly-arrived invite) notifies.
const perviousInviteLen = usePreviousValue(invites.length, invites.length);
const mx = useMatrixClient(); const mx = useMatrixClient();
// Notify only for invites that ARRIVE while the app runs — never for invites
// already present at load. allInvitesAtom (a string[] of invited room ids)
// populates asynchronously post-mount from the initial/cached sync, so any count/
// seed baseline mis-fires on reload. Stay "unarmed" until the initial sync settles
// (keeping the id baseline synced to whatever loads), then notify only for room
// ids that first appear AFTER arming.
const armedRef = useRef(false);
const knownInviteIdsRef = useRef<Set<string>>(new Set());
const navigate = useNavigate(); const navigate = useNavigate();
const [showNotifications] = useSetting(settingsAtom, 'showNotifications'); const [showNotifications] = useSetting(settingsAtom, 'showNotifications');
@@ -204,26 +212,60 @@ function InviteNotifications() {
audioElement?.play(); audioElement?.play();
}, []); }, []);
// Arm once the client's initial sync has settled (+ a short grace so the async
// atom population lands first). Until armed, the effect below only tracks the
// baseline of already-present invites without notifying.
useEffect(() => { useEffect(() => {
if (invites.length > perviousInviteLen && mx.getSyncState() === 'SYNCING') { let timer: ReturnType<typeof setTimeout> | undefined;
const arm = () => {
timer = setTimeout(() => {
armedRef.current = true;
}, INVITE_NOTIFY_ARM_DELAY_MS);
};
if (mx.getSyncState() === SyncState.Syncing) {
arm();
return () => {
if (timer) clearTimeout(timer);
};
}
const onSync: ClientEventHandlerMap[ClientEvent.Sync] = (state) => {
if (state === SyncState.Syncing) {
mx.off(ClientEvent.Sync, onSync);
arm();
}
};
mx.on(ClientEvent.Sync, onSync);
return () => {
mx.off(ClientEvent.Sync, onSync);
if (timer) clearTimeout(timer);
};
}, [mx]);
useEffect(() => {
const currentIds = new Set(invites);
if (!armedRef.current) {
// Not yet armed: keep the baseline synced to whatever the initial sync loads.
knownInviteIdsRef.current = currentIds;
return;
}
const newCount = invites.filter((id) => !knownInviteIdsRef.current.has(id)).length;
knownInviteIdsRef.current = currentIds;
if (newCount <= 0) return;
const quietActive = const quietActive =
focusAssistActive || focusAssistActive ||
manualDnd || manualDnd ||
(quietHoursEnabled && isInQuietHours(quietHoursStart, quietHoursEnd)); (quietHoursEnabled && isInQuietHours(quietHoursStart, quietHoursEnd));
if (!quietActive) { if (quietActive) return;
if (showNotifications && notificationPermission('granted')) {
notify(invites.length - perviousInviteLen);
}
if (showNotifications && notificationPermission('granted')) {
notify(newCount);
}
if (notificationSound && inviteSoundId !== 'none') { if (notificationSound && inviteSoundId !== 'none') {
playSound(); playSound();
} }
}
}
}, [ }, [
mx,
invites, invites,
perviousInviteLen,
showNotifications, showNotifications,
notificationSound, notificationSound,
notify, notify,