DP13: extract shared account-data list-store engine

useBookmarks / useReminders / useUserNotes were near-verbatim copies of a
concurrency-critical engine (module-scoped singleton, per-client
subscribe/teardown, a serialized write-queue that prevents lost-update
clobbering, a listener Set, and the account-data subscription).

Extract it into createAccountDataListStore<T, C>({ eventType, read, write }) in
src/app/hooks/createAccountDataListStore.ts. The write-serialization semantics
are preserved identically (still the lost-update fix). The differing payload
shapes are parameterized via read/write: bookmarks/reminders wrap a list
({bookmarks}/{reminders}); notes is a flat Record passed through unchanged.

The three hooks become thin wrappers with their exact public APIs unchanged
(same exported names, signatures, return shapes, and mutators), so no call site
changes. The DP16 setAccountData helper is used inside the queue.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/app/hooks/createAccountDataListStore.ts

@@ -0,0 +1,130 @@
+import { useEffect, useState } from 'react';
+import { ClientEvent, ClientEventHandlerMap, MatrixClient } from 'matrix-js-sdk';
+import { AccountDataEvent } from '../../types/matrix/accountData';
+import { getAccountData, setAccountData } from '../utils/accountData';
+
+// Generic, concurrency-safe store for a single global account-data event that
+// holds a mutable collection (a list or a record). It is the shared engine
+// behind useBookmarks / useReminders / useUserNotes.
+//
+// Why a module-scoped singleton (per store, per client):
+//
+// These hooks are mounted by many components at once (e.g. useBookmarks() lives
+// on every message row). A per-instance snapshot + write queue would only
+// serialize writes within a single instance, so two edits issued from different
+// instances before the server echo lands would each compute from a stale
+// snapshot and clobber each other — setAccountData replaces the whole content
+// with no server-side merge. We therefore keep ONE shared `latest` snapshot and
+// ONE write queue per store, keyed off the active MatrixClient, and serialize
+// every write through that queue (the lost-update fix). Do not weaken this.
+
+// How the differing payload shapes are parameterized:
+// - `read` maps the raw account-data content (C | undefined) to the in-memory
+//   value T (e.g. `content?.bookmarks ?? []`, or `content ?? {}` for notes).
+// - `write` maps the in-memory value T back to the content object to persist
+//   (e.g. `{ bookmarks: value }`, or just `value` for the flat notes record).
+export type AccountDataListStoreConfig<T, C> = {
+  eventType: AccountDataEvent | string;
+  read: (content: C | undefined) => T;
+  write: (value: T) => object;
+};
+
+export type AccountDataListStore<T> = {
+  /** React hook: subscribe to the store and return the latest value. */
+  useValue: (mx: MatrixClient) => T;
+  /** Read the current shared snapshot without subscribing. */
+  getLatest: (mx: MatrixClient) => T;
+  /**
+   * Serialize a write. `compute` receives the latest snapshot and returns the
+   * next value; writes run one at a time so none is computed from stale data.
+   */
+  enqueueWrite: (mx: MatrixClient, compute: (current: T) => T) => Promise<void>;
+};
+
+export function createAccountDataListStore<T, C = unknown>({
+  eventType,
+  read,
+  write,
+}: AccountDataListStoreConfig<T, C>): AccountDataListStore<T> {
+  type ModuleState = {
+    mx: MatrixClient;
+    latest: T;
+    writeQueue: Promise<unknown>;
+    listeners: Set<(value: T) => void>;
+    onAccountData: ClientEventHandlerMap[ClientEvent.AccountData];
+  };
+
+  let moduleState: ModuleState | null = null;
+
+  const readLatest = (mx: MatrixClient): T => read(getAccountData<C>(mx, eventType));
+
+  // Lazily initialize the shared state for the given client. On a client change
+  // (login/logout swaps the MatrixClient) we tear down the old subscription and
+  // re-initialize against the new client so we never leak or double-subscribe.
+  function ensureModuleState(mx: MatrixClient): ModuleState {
+    if (moduleState && moduleState.mx === mx) {
+      return moduleState;
+    }
+
+    if (moduleState) {
+      moduleState.mx.removeListener(ClientEvent.AccountData, moduleState.onAccountData);
+    }
+
+    const state: ModuleState = {
+      mx,
+      latest: readLatest(mx),
+      writeQueue: Promise.resolve(),
+      listeners: new Set(),
+      // Reassigned below once `state` is captured.
+      onAccountData: () => undefined,
+    };
+
+    state.onAccountData = (evt) => {
+      if (evt.getType() === eventType) {
+        const value = read(evt.getContent() as C);
+        state.latest = value;
+        state.listeners.forEach((listener) => listener(value));
+      }
+    };
+
+    mx.on(ClientEvent.AccountData, state.onAccountData);
+    moduleState = state;
+    return state;
+  }
+
+  function enqueueWrite(mx: MatrixClient, compute: (current: T) => T): Promise<void> {
+    const state = ensureModuleState(mx);
+    const run = state.writeQueue.then(async () => {
+      const next = compute(state.latest);
+      state.latest = next;
+      state.listeners.forEach((listener) => listener(next));
+      await setAccountData(mx, eventType, write(next));
+    });
+    // Keep the chain alive even if one write rejects, but propagate the
+    // rejection to this caller so it can react (e.g. retry).
+    state.writeQueue = run.catch(() => undefined);
+    return run;
+  }
+
+  function useValue(mx: MatrixClient): T {
+    const [value, setValue] = useState<T>(() => ensureModuleState(mx).latest);
+
+    // Subscribe to the shared module state. A single AccountData listener is
+    // installed per client (in ensureModuleState); each hook instance only
+    // registers a local setter and unregisters it on unmount / client change.
+    useEffect(() => {
+      const state = ensureModuleState(mx);
+      setValue(state.latest);
+      state.listeners.add(setValue);
+      return () => {
+        state.listeners.delete(setValue);
+      };
+    }, [mx]);
+
+    return value;
+  }
+
+  const getLatest = (mx: MatrixClient): T => ensureModuleState(mx).latest;
+
+  return { useValue, getLatest, enqueueWrite };
+}

src/app/hooks/useBookmarks.ts

@@ -1,7 +1,6 @@
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback } from 'react';
-import { ClientEvent, ClientEventHandlerMap, MatrixClient } from 'matrix-js-sdk';
 import { useMatrixClient } from './useMatrixClient';
-import { getAccountData, setAccountData } from '../utils/accountData';
+import { createAccountDataListStore } from './createAccountDataListStore';
 
 export type Bookmark = {
   roomId: string;
@@ -18,78 +17,14 @@ type BookmarksContent = {
   bookmarks: Bookmark[];
 };
 
-function readBookmarks(mx: MatrixClient): Bookmark[] {
+// Shared, concurrency-safe store. See createAccountDataListStore for why the
-  return getAccountData<BookmarksContent>(mx, BOOKMARKS_KEY)?.bookmarks ?? [];
+// snapshot + write queue must be module-scoped (writes are serialized to avoid
-}
+// lost updates, since setAccountData replaces the whole content with no merge).
-
+const bookmarksStore = createAccountDataListStore<Bookmark[], BookmarksContent>({
-// Module-scoped serialization state.
+  eventType: BOOKMARKS_KEY,
-//
+  read: (content) => content?.bookmarks ?? [],
-// useBookmarks() is mounted once per message row (dozens of live instances), so
+  write: (bookmarks) => ({ bookmarks }),
-// a per-instance latest/queue would only serialize writes within a single row —
-// bookmarking message A then message B from different rows (before the server
-// echo lands) would let B compute from a stale snapshot and clobber A
-// (setAccountData replaces the whole content, no server merge). We therefore
-// keep a single shared latest ref + write queue, keyed off the active client.
-type BookmarksModuleState = {
-  mx: MatrixClient;
-  latest: Bookmark[];
-  writeQueue: Promise<unknown>;
-  listeners: Set<(list: Bookmark[]) => void>;
-  onAccountData: ClientEventHandlerMap[ClientEvent.AccountData];
-};
-
-let moduleState: BookmarksModuleState | null = null;
-
-// Lazily initialize the shared state for the given client. On a client change
-// (login/logout swaps the MatrixClient) we tear down the old subscription and
-// re-initialize against the new client so we never leak or double-subscribe.
-function ensureModuleState(mx: MatrixClient): BookmarksModuleState {
-  if (moduleState && moduleState.mx === mx) {
-    return moduleState;
-  }
-
-  if (moduleState) {
-    moduleState.mx.removeListener(ClientEvent.AccountData, moduleState.onAccountData);
-  }
-
-  const state: BookmarksModuleState = {
-    mx,
-    latest: readBookmarks(mx),
-    writeQueue: Promise.resolve(),
-    listeners: new Set(),
-    // Reassigned below once `state` is captured.
-    onAccountData: () => undefined,
-  };
-
-  state.onAccountData = (evt) => {
-    if (evt.getType() === BOOKMARKS_KEY) {
-      const list = evt.getContent<BookmarksContent>()?.bookmarks ?? [];
-      state.latest = list;
-      state.listeners.forEach((listener) => listener(list));
-    }
-  };
-
-  mx.on(ClientEvent.AccountData, state.onAccountData);
-  moduleState = state;
-  return state;
-}
-
-function enqueueBookmarkWrite(
-  mx: MatrixClient,
-  compute: (current: Bookmark[]) => Bookmark[],
-): Promise<void> {
-  const state = ensureModuleState(mx);
-  const run = state.writeQueue.then(async () => {
-    const next = compute(state.latest);
-    state.latest = next;
-    state.listeners.forEach((listener) => listener(next));
-    await setAccountData(mx, BOOKMARKS_KEY, { bookmarks: next });
 });
-  // Keep the chain alive even if one write rejects, but propagate the
-  // rejection to this caller so it can react (e.g. retry).
-  state.writeQueue = run.catch(() => undefined);
-  return run;
-}
 
 export function useBookmarks(): {
   bookmarks: Bookmark[];
@@ -98,23 +33,11 @@ export function useBookmarks(): {
   isBookmarked: (eventId: string) => boolean;
 } {
   const mx = useMatrixClient();
-  const [bookmarks, setBookmarks] = useState<Bookmark[]>(() => ensureModuleState(mx).latest);
+  const bookmarks = bookmarksStore.useValue(mx);
-
-  // Subscribe to the shared module state. A single AccountData listener is
-  // installed per client (in ensureModuleState); each hook instance only
-  // registers a local setter and unregisters it on unmount / client change.
-  useEffect(() => {
-    const state = ensureModuleState(mx);
-    setBookmarks(state.latest);
-    state.listeners.add(setBookmarks);
-    return () => {
-      state.listeners.delete(setBookmarks);
-    };
-  }, [mx]);
 
   const addBookmark = useCallback(
     (b: Bookmark) =>
-      enqueueBookmarkWrite(mx, (current) => {
+      bookmarksStore.enqueueWrite(mx, (current) => {
         // Avoid duplicates
         const filtered = current.filter((bk) => bk.eventId !== b.eventId);
         let next = [b, ...filtered];
@@ -128,7 +51,7 @@ export function useBookmarks(): {
 
   const removeBookmark = useCallback(
     (eventId: string) =>
-      enqueueBookmarkWrite(mx, (current) => current.filter((bk) => bk.eventId !== eventId)),
+      bookmarksStore.enqueueWrite(mx, (current) => current.filter((bk) => bk.eventId !== eventId)),
     [mx],
   );
 

src/app/hooks/useReminders.ts

@@ -1,7 +1,6 @@
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback } from 'react';
-import { ClientEvent, ClientEventHandlerMap, MatrixClient } from 'matrix-js-sdk';
 import { useMatrixClient } from './useMatrixClient';
-import { getAccountData, setAccountData } from '../utils/accountData';
+import { createAccountDataListStore } from './createAccountDataListStore';
 
 export type Reminder = {
   roomId: string;
@@ -16,78 +15,14 @@ type RemindersContent = {
   reminders: Reminder[];
 };
 
-function readReminders(mx: MatrixClient): Reminder[] {
+// Shared, concurrency-safe store. See createAccountDataListStore for why the
-  return getAccountData<RemindersContent>(mx, REMINDERS_KEY)?.reminders ?? [];
+// snapshot + write queue must be module-scoped (writes are serialized to avoid
-}
+// lost updates, since setAccountData replaces the whole content with no merge).
-
+const remindersStore = createAccountDataListStore<Reminder[], RemindersContent>({
-// Module-scoped serialization state.
+  eventType: REMINDERS_KEY,
-//
+  read: (content) => content?.reminders ?? [],
-// The latest snapshot and the write queue must be shared across every hook
+  write: (reminders) => ({ reminders }),
-// instance: ReminderMonitor (auto-removes fired reminders) and RemindMeDialog
-// (adds reminders) mount separate hooks, and a per-instance queue would let a
-// remove and an add race across instances and clobber each other (setAccountData
-// replaces the whole content, no server merge). We therefore keep a single
-// shared queue + latest ref, keyed off the active MatrixClient.
-type ReminderModuleState = {
-  mx: MatrixClient;
-  latest: Reminder[];
-  writeQueue: Promise<unknown>;
-  listeners: Set<(list: Reminder[]) => void>;
-  onAccountData: ClientEventHandlerMap[ClientEvent.AccountData];
-};
-
-let moduleState: ReminderModuleState | null = null;
-
-// Lazily initialize the shared state for the given client. On a client change
-// (login/logout swaps the MatrixClient) we tear down the old subscription and
-// re-initialize against the new client so we never leak or double-subscribe.
-function ensureModuleState(mx: MatrixClient): ReminderModuleState {
-  if (moduleState && moduleState.mx === mx) {
-    return moduleState;
-  }
-
-  if (moduleState) {
-    moduleState.mx.removeListener(ClientEvent.AccountData, moduleState.onAccountData);
-  }
-
-  const state: ReminderModuleState = {
-    mx,
-    latest: readReminders(mx),
-    writeQueue: Promise.resolve(),
-    listeners: new Set(),
-    // Reassigned below once `state` is captured.
-    onAccountData: () => undefined,
-  };
-
-  state.onAccountData = (evt) => {
-    if (evt.getType() === REMINDERS_KEY) {
-      const list = evt.getContent<RemindersContent>()?.reminders ?? [];
-      state.latest = list;
-      state.listeners.forEach((listener) => listener(list));
-    }
-  };
-
-  mx.on(ClientEvent.AccountData, state.onAccountData);
-  moduleState = state;
-  return state;
-}
-
-function enqueueReminderWrite(
-  mx: MatrixClient,
-  compute: (current: Reminder[]) => Reminder[],
-): Promise<void> {
-  const state = ensureModuleState(mx);
-  const run = state.writeQueue.then(async () => {
-    const next = compute(state.latest);
-    state.latest = next;
-    state.listeners.forEach((listener) => listener(next));
-    await setAccountData(mx, 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).
-  state.writeQueue = run.catch(() => undefined);
-  return run;
-}
 
 export function useReminders(): {
   reminders: Reminder[];
@@ -96,34 +31,22 @@ export function useReminders(): {
   getReminders: () => Reminder[];
 } {
   const mx = useMatrixClient();
-  const [reminders, setReminders] = useState<Reminder[]>(() => ensureModuleState(mx).latest);
+  const reminders = remindersStore.useValue(mx);
-
-  // Subscribe to the shared module state. A single AccountData listener is
-  // installed per client (in ensureModuleState); each hook instance only
-  // registers a local setter and unregisters it on unmount / client change.
-  useEffect(() => {
-    const state = ensureModuleState(mx);
-    setReminders(state.latest);
-    state.listeners.add(setReminders);
-    return () => {
-      state.listeners.delete(setReminders);
-    };
-  }, [mx]);
 
   const addReminder = useCallback(
-    (r: Reminder) => enqueueReminderWrite(mx, (current) => [...current, r]),
+    (r: Reminder) => remindersStore.enqueueWrite(mx, (current) => [...current, r]),
     [mx],
   );
 
   const removeReminder = useCallback(
     (eventId: string, timestamp: number) =>
-      enqueueReminderWrite(mx, (current) =>
+      remindersStore.enqueueWrite(mx, (current) =>
         current.filter((r) => !(r.eventId === eventId && r.timestamp === timestamp)),
       ),
     [mx],
   );
 
-  const getReminders = useCallback(() => ensureModuleState(mx).latest, [mx]);
+  const getReminders = useCallback(() => remindersStore.getLatest(mx), [mx]);
 
   return { reminders, addReminder, removeReminder, getReminders };
 }

src/app/hooks/useUserNotes.ts

@@ -1,111 +1,37 @@
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback } from 'react';
-import { ClientEvent, ClientEventHandlerMap, MatrixClient } from 'matrix-js-sdk';
 import { useMatrixClient } from './useMatrixClient';
-import { getAccountData, setAccountData } from '../utils/accountData';
+import { createAccountDataListStore } from './createAccountDataListStore';
 
 const NOTES_KEY = 'io.lotus.user_notes';
 export const USER_NOTE_MAX_LENGTH = 500;
 
 type UserNotesContent = Record<string, string>;
 
-function readNotes(mx: MatrixClient): UserNotesContent {
+// Shared, concurrency-safe store. See createAccountDataListStore for why the
-  return getAccountData<UserNotesContent>(mx, NOTES_KEY) ?? {};
+// snapshot + write queue must be module-scoped (writes are serialized to avoid
-}
+// lost updates, since setAccountData replaces the whole content with no merge).
-
-// Module-scoped serialization state.
 //
-// useUserNotes() can be mounted by many components at once, so a per-instance
+// Notes are a flat Record<userId, note> rather than a wrapped list, so `read`
-// latest/queue would only serialize writes within one instance. Notes for
+// and `write` pass the content through unchanged.
-// different users saved from different instances (before the server echo lands)
+const notesStore = createAccountDataListStore<UserNotesContent, UserNotesContent>({
-// would each compute from a stale snapshot and clobber each other, since
+  eventType: NOTES_KEY,
-// setAccountData replaces the whole record with no server merge. We therefore
+  read: (content) => content ?? {},
-// keep a single shared latest record + write queue, keyed off the active client.
+  write: (notes) => notes,
-type UserNotesModuleState = {
-  mx: MatrixClient;
-  latest: UserNotesContent;
-  writeQueue: Promise<unknown>;
-  listeners: Set<(record: UserNotesContent) => void>;
-  onAccountData: ClientEventHandlerMap[ClientEvent.AccountData];
-};
-
-let moduleState: UserNotesModuleState | null = null;
-
-// Lazily initialize the shared state for the given client. On a client change
-// (login/logout swaps the MatrixClient) we tear down the old subscription and
-// re-initialize against the new client so we never leak or double-subscribe.
-function ensureModuleState(mx: MatrixClient): UserNotesModuleState {
-  if (moduleState && moduleState.mx === mx) {
-    return moduleState;
-  }
-
-  if (moduleState) {
-    moduleState.mx.removeListener(ClientEvent.AccountData, moduleState.onAccountData);
-  }
-
-  const state: UserNotesModuleState = {
-    mx,
-    latest: readNotes(mx),
-    writeQueue: Promise.resolve(),
-    listeners: new Set(),
-    // Reassigned below once `state` is captured.
-    onAccountData: () => undefined,
-  };
-
-  state.onAccountData = (evt) => {
-    if (evt.getType() === NOTES_KEY) {
-      const record = evt.getContent<UserNotesContent>() ?? {};
-      state.latest = record;
-      state.listeners.forEach((listener) => listener(record));
-    }
-  };
-
-  mx.on(ClientEvent.AccountData, state.onAccountData);
-  moduleState = state;
-  return state;
-}
-
-function enqueueNotesWrite(
-  mx: MatrixClient,
-  compute: (current: UserNotesContent) => UserNotesContent,
-): Promise<void> {
-  const state = ensureModuleState(mx);
-  const run = state.writeQueue.then(async () => {
-    const next = compute(state.latest);
-    state.latest = next;
-    state.listeners.forEach((listener) => listener(next));
-    await setAccountData(mx, NOTES_KEY, next);
 });
-  // Keep the chain alive even if one write rejects, but propagate the
-  // rejection to this caller so it can react (e.g. retry).
-  state.writeQueue = run.catch(() => undefined);
-  return run;
-}
 
 export function useUserNotes(): {
   getNote: (userId: string) => string;
   setNote: (userId: string, note: string) => Promise<void>;
 } {
   const mx = useMatrixClient();
-  const [notes, setNotes] = useState<UserNotesContent>(() => ensureModuleState(mx).latest);
+  const notes = notesStore.useValue(mx);
-
-  // Subscribe to the shared module state. A single AccountData listener is
-  // installed per client (in ensureModuleState); each hook instance only
-  // registers a local setter and unregisters it on unmount / client change.
-  useEffect(() => {
-    const state = ensureModuleState(mx);
-    setNotes(state.latest);
-    state.listeners.add(setNotes);
-    return () => {
-      state.listeners.delete(setNotes);
-    };
-  }, [mx]);
 
   const getNote = useCallback((userId: string) => notes[userId] ?? '', [notes]);
 
   const setNote = useCallback(
     (userId: string, note: string) => {
       const trimmed = note.trim().slice(0, USER_NOTE_MAX_LENGTH);
-      return enqueueNotesWrite(mx, (current) => {
+      return notesStore.enqueueWrite(mx, (current) => {
         const updated = { ...current };
         if (trimmed) {
           updated[userId] = trimmed;