diff --git a/src/app/hooks/useReminders.ts b/src/app/hooks/useReminders.ts index a8dee4bc3..d26b6f7d4 100644 --- a/src/app/hooks/useReminders.ts +++ b/src/app/hooks/useReminders.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { MatrixClient } from 'matrix-js-sdk'; import { useMatrixClient } from './useMatrixClient'; import { useAccountDataCallback } from './useAccountDataCallback'; @@ -32,39 +32,64 @@ export function useReminders(): { const mx = useMatrixClient(); const [reminders, setReminders] = useState(() => readReminders(mx)); + // Authoritative local snapshot used to compute mutations. Reading + // mx.getAccountData() per-mutation is racy: two quick add/remove calls both + // read the same stale baseline and the second write clobbers the first + // (N113). We instead mutate from this ref, kept in sync with server echoes. + const latestRef = useRef(reminders); + // Serialize writes so overlapping setAccountData calls can't land out of + // order on the server (last-write-wins would otherwise drop data). + const writeQueueRef = useRef>(Promise.resolve()); + + const applyServerState = useCallback((list: Reminder[]) => { + latestRef.current = list; + setReminders(list); + }, []); + useAccountDataCallback( mx, useCallback( (evt) => { if (evt.getType() === REMINDERS_KEY) { - setReminders(evt.getContent()?.reminders ?? []); + applyServerState(evt.getContent()?.reminders ?? []); } }, - [setReminders], + [applyServerState], ), ); // Re-read on mx change useEffect(() => { - setReminders(readReminders(mx)); - }, [mx]); + applyServerState(readReminders(mx)); + }, [mx, applyServerState]); - const addReminder = useCallback( - async (r: Reminder) => { - const current = readReminders(mx); - const next = [...current, r]; - await (mx as any).setAccountData(REMINDERS_KEY, { reminders: next }); + const enqueueWrite = useCallback( + (compute: (current: Reminder[]) => Reminder[]): Promise => { + const run = writeQueueRef.current.then(async () => { + const next = compute(latestRef.current); + latestRef.current = next; + setReminders(next); + await (mx as any).setAccountData(REMINDERS_KEY, { reminders: next }); + }); + // Keep the chain alive even if one write rejects, but propagate the + // rejection to this caller so it can react (e.g. retry). + writeQueueRef.current = run.catch(() => undefined); + return run; }, [mx], ); + const addReminder = useCallback( + (r: Reminder) => enqueueWrite((current) => [...current, r]), + [enqueueWrite], + ); + const removeReminder = useCallback( - async (eventId: string, timestamp: number) => { - const current = readReminders(mx); - const next = current.filter((r) => !(r.eventId === eventId && r.timestamp === timestamp)); - await (mx as any).setAccountData(REMINDERS_KEY, { reminders: next }); - }, - [mx], + (eventId: string, timestamp: number) => + enqueueWrite((current) => + current.filter((r) => !(r.eventId === eventId && r.timestamp === timestamp)), + ), + [enqueueWrite], ); const getReminders = useCallback(() => reminders, [reminders]); diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index bd9d6840d..5517cdb24 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -390,16 +390,26 @@ function ReminderMonitor() { const setToast = useSetAtom(toastQueueAtom); const mDirects = useAtomValue(mDirectAtom); const firedRef = useRef>(new Set()); + const removingRef = useRef>(new Set()); + // Read the latest reminders / DM map via refs so the poll interval below is + // created once — not torn down and restarted (which resets its 30s countdown + // and can indefinitely defer a near-due reminder) on every reminder sync (N115). + const remindersRef = useRef(reminders); + remindersRef.current = reminders; + const mDirectsRef = useRef(mDirects); + mDirectsRef.current = mDirects; useEffect(() => { const check = () => { const now = Date.now(); - reminders.forEach((r) => { + remindersRef.current.forEach((r) => { + if (r.timestamp > now) return; const key = `${r.eventId}-${r.timestamp}`; - if (r.timestamp <= now && !firedRef.current.has(key)) { + // Show the toast exactly once. + if (!firedRef.current.has(key)) { firedRef.current.add(key); const room = mx.getRoom(r.roomId); - const hashPath = mDirects.has(r.roomId) + const hashPath = mDirectsRef.current.has(r.roomId) ? getDirectRoomPath(r.roomId, r.eventId) : getHomeRoomPath(r.roomId, r.eventId); setToast({ @@ -410,7 +420,15 @@ function ReminderMonitor() { roomId: r.roomId, hashPath, }); - removeReminder(r.eventId, r.timestamp); + } + // Persist the removal, retrying on a later tick if it fails — without + // re-showing the toast (N114). The server echo drops it from + // `reminders` once the write lands. + if (!removingRef.current.has(key)) { + removingRef.current.add(key); + removeReminder(r.eventId, r.timestamp).catch(() => { + removingRef.current.delete(key); + }); } }); }; @@ -425,7 +443,7 @@ function ReminderMonitor() { clearInterval(interval); document.removeEventListener('visibilitychange', onVisible); }; - }, [mx, reminders, setToast, removeReminder, mDirects]); + }, [mx, setToast, removeReminder]); return null; }