diff --git a/src/app/hooks/useMessageTranslation.ts b/src/app/hooks/useMessageTranslation.ts index 872ced76f..7f9eb91ad 100644 --- a/src/app/hooks/useMessageTranslation.ts +++ b/src/app/hooks/useMessageTranslation.ts @@ -1,13 +1,15 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; -import { useAtom } from 'jotai'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useAtom, useAtomValue, useSetAtom } from 'jotai'; import { settingsAtom } from '../state/settings'; import { useSetting } from '../state/hooks/settings'; import { translationCacheAtom, + translationEntryAtomFamily, msgTranslationActiveAtomFamily, makeCacheKey, - findTranslation, addTranslation, + wasAutoTranslateTried, + markAutoTranslateTried, } from '../state/translation'; import { chromeTranslationEngine as engine } from '../utils/translation/chromeEngine'; import { sameLanguage } from '../utils/translation/langUtils'; @@ -47,12 +49,17 @@ export const useMessageTranslation = (eventId: string, text: string): MsgTransla const [targetLang] = useSetting(settingsAtom, 'translateTargetLang'); const [autoTranslate] = useSetting(settingsAtom, 'autoTranslate'); const [active, setActive] = useAtom(msgTranslationActiveAtomFamily(eventId)); - const [cache, setCache] = useAtom(translationCacheAtom); + // Write-only: reading the whole cache array here (via useAtom) would + // resubscribe this message to every other message's writes. [Gitea #39] + const setCache = useSetAtom(translationCacheAtom); // The key includes a fingerprint of `text`, so an edit (same event id, new - // body) misses the old entry and re-translates instead of showing stale text. - const key = makeCacheKey(eventId, targetLang, text); - const cached = findTranslation(cache, key); + // body) misses the old entry and re-translates instead of showing stale + // text. Memoised so the O(len) fingerprint isn't recomputed every render. + const key = useMemo(() => makeCacheKey(eventId, targetLang, text), [eventId, targetLang, text]); + // Subscribes to only this message's cache slot, so translating one message + // no longer re-renders every mounted message. [Gitea #39] + const cached = useAtomValue(translationEntryAtomFamily(key)); const [status, setStatus] = useState('idle'); const [translated, setTranslated] = useState(undefined); @@ -74,13 +81,13 @@ export const useMessageTranslation = (eventId: string, text: string): MsgTransla // Auto-translate: when enabled, flip `active` on for foreign messages whose // model is already downloaded (no gesture/download needed). Messages needing // a first-time download keep the manual chip (which provides the gesture). - // Store the text we last auto-tried so an edit (new text) re-triggers auto - // detection instead of being suppressed by a one-shot flag. - const autoTriedText = useRef(undefined); + // The "already tried" guard is hoisted to module scope (keyed by eventId) so + // it survives unmount — virtualised scrolling used to re-run detection every + // time the same message remounted with a per-hook-instance ref. [Gitea #39] useEffect(() => { if (!supported || !autoTranslate || active || cached || !eventId) return; - if (!text.trim() || autoTriedText.current === text) return; - autoTriedText.current = text; + if (!text.trim() || wasAutoTranslateTried(eventId, text)) return; + markAutoTranslateTried(eventId, text); let alive = true; (async () => { const from = await engine.detect(text); diff --git a/src/app/state/translation.test.ts b/src/app/state/translation.test.ts index 85ca3d46f..4b7090085 100644 --- a/src/app/state/translation.test.ts +++ b/src/app/state/translation.test.ts @@ -1,16 +1,32 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; +import { createStore } from 'jotai'; // The module evaluates atomWithStorage(..., { getOnInit: true }) which reads // localStorage at load time. node has none — install a no-op mock, then import // dynamically (a static import would hoist above the mock). +const localStorageBacking = new Map(); (globalThis as { localStorage?: unknown }).localStorage = { - getItem: () => null, - setItem: () => undefined, - removeItem: () => undefined, + getItem: (k: string) => localStorageBacking.get(k) ?? null, + setItem: (k: string, v: string) => { + localStorageBacking.set(k, v); + }, + removeItem: (k: string) => { + localStorageBacking.delete(k); + }, }; -const { addTranslation, findTranslation, makeCacheKey } = await import('./translation'); +const { + addTranslation, + findTranslation, + makeCacheKey, + translationCacheAtom, + translationEntryAtomFamily, + msgTranslationActiveAtomFamily, + wasAutoTranslateTried, + markAutoTranslateTried, + clearTranslationCache, +} = await import('./translation'); const entry = (key: string, translated = `t-${key}`, fromLang = 'de') => ({ key, @@ -66,3 +82,90 @@ test('findTranslation: returns match or undefined', () => { assert.equal(findTranslation(list, 'b')?.key, 'b'); assert.equal(findTranslation(list, 'z'), undefined); }); + +// [Gitea #39] Keyed cache subscription: a message only re-renders for its own +// entry, not the whole array. +test('translationEntryAtomFamily: looks up only its own key, unaffected by others', () => { + const store = createStore(); + store.set(translationCacheAtom, () => [entry('a'), entry('b')]); + + assert.equal(store.get(translationEntryAtomFamily('a'))?.translated, 't-a'); + assert.equal(store.get(translationEntryAtomFamily('z')), undefined); +}); + +test('translationEntryAtomFamily: does not notify a subscriber when an unrelated key changes', () => { + const store = createStore(); + store.set(translationCacheAtom, () => [entry('a')]); + + let notifications = 0; + const unsub = store.sub(translationEntryAtomFamily('a'), () => { + notifications += 1; + }); + // atomWithStorage's `onMount` resyncs the base atom from storage as soon as + // it gets its first subscriber, which — via the JSON round trip — produces + // a structurally-equal-but-different-reference array, firing exactly one + // notification unrelated to what this test checks. Drain it before + // asserting so we're isolating the behaviour under test. + notifications = 0; + try { + // Add an unrelated entry — 'a's object reference in the array is + // preserved by addTranslation, so selectAtom's Object.is bails. + store.set(translationCacheAtom, (prev) => addTranslation(prev, entry('other'))); + assert.equal(notifications, 0); + + // Changing 'a' itself does notify. + store.set(translationCacheAtom, (prev) => addTranslation(prev, entry('a', 'new-a'))); + assert.equal(notifications, 1); + } finally { + unsub(); + } +}); + +test('msgTranslationActiveAtomFamily: FIFO-capped at 500 members', () => { + const store = createStore(); + // Prime "e0" with non-default state. + store.set(msgTranslationActiveAtomFamily('e0'), true); + assert.equal(store.get(msgTranslationActiveAtomFamily('e0')), true); + + // Create 500 more distinct members so the cap evicts "e0" (oldest-created); + // real usage is one member per mounted message regardless of translation. + for (let i = 1; i <= 500; i += 1) { + msgTranslationActiveAtomFamily(`e${i}`); + } + + // Evicted members are unmounted (`.remove()`d): re-accessing "e0" creates a + // fresh atom, so it comes back at its default (inactive) value — evidence + // the family doesn't grow without bound as the timeline is scrolled. + assert.equal(store.get(msgTranslationActiveAtomFamily('e0')), false); +}); + +test('translationEntryAtomFamily: stays usable well past the FIFO cap', () => { + const store = createStore(); + store.set(translationCacheAtom, () => [entry('k0')]); + for (let i = 1; i <= 500; i += 1) { + translationEntryAtomFamily(`k${i}`); + } + // Unlike the active family, this family is *derived* from the persisted + // cache array, so a re-created (evicted-then-reaccessed) atom still finds + // the entry — eviction here is a memory-hygiene concern, not a data-loss + // one. Confirms the cap doesn't corrupt lookups either way. + assert.equal(store.get(translationEntryAtomFamily('k0'))?.key, 'k0'); + assert.equal(store.get(translationEntryAtomFamily('k250')), undefined); +}); + +test('clearTranslationCache: wipes storage, live family members, and the auto-translate guard', () => { + const store = createStore(); + localStorageBacking.set('cinny_translation_cache_v1', '[{"key":"x"}]'); + + store.set(msgTranslationActiveAtomFamily('ev1'), true); + markAutoTranslateTried('ev1', 'hola'); + assert.equal(wasAutoTranslateTried('ev1', 'hola'), true); + + clearTranslationCache(); + + assert.equal(localStorageBacking.has('cinny_translation_cache_v1'), false); + // Family member was `.remove()`d, so re-accessing it yields a fresh atom + // back at its default value. + assert.equal(store.get(msgTranslationActiveAtomFamily('ev1')), false); + assert.equal(wasAutoTranslateTried('ev1', 'hola'), false); +}); diff --git a/src/app/state/translation.ts b/src/app/state/translation.ts index b5858f80f..e59997f56 100644 --- a/src/app/state/translation.ts +++ b/src/app/state/translation.ts @@ -1,5 +1,5 @@ import { atom } from 'jotai'; -import { atomWithStorage, createJSONStorage, atomFamily } from 'jotai/utils'; +import { atomWithStorage, createJSONStorage, atomFamily, selectAtom } from 'jotai/utils'; import { normalizeLang } from '../utils/translation/langUtils'; export type CachedTranslation = { @@ -73,15 +73,75 @@ export const addTranslation = ( return [entry, ...withoutDupe].slice(0, max); }; +// [Gitea #39] Every `atomFamily` here is capped with simple FIFO eviction: +// a message mounts one entry in each family regardless of whether it's ever +// translated, so scrolling a long-lived session used to grow both families +// (and, for the active family, forever — it was never `.remove()`d at all). +// The cap is generous relative to what's ever rendered at once, so evicting +// the oldest-*created* member (not oldest-used) doesn't touch on-screen +// messages in practice; a false eviction just means that message's atom is +// recreated at its default value next access, which is harmless here (a +// re-derived cache lookup / a re-collapsed "active" toggle). +const FAMILY_CAP = 500; + // Per-event "show translation" toggle, shared between the message menu action // (which flips it) and the inline body render (which reacts to it). Keyed by // event id so each message has its own independent state. -export const msgTranslationActiveAtomFamily = atomFamily((_eventId: string) => atom(false)); +const activeFamilyOrder: string[] = []; +export const msgTranslationActiveAtomFamily = atomFamily((_eventId: string) => { + activeFamilyOrder.push(_eventId); + if (activeFamilyOrder.length > FAMILY_CAP) { + const oldest = activeFamilyOrder.shift(); + if (oldest !== undefined) msgTranslationActiveAtomFamily.remove(oldest); + } + return atom(false); +}); /** - * Wipe the persisted translation cache. Called on logout — cached entries are - * decrypted message plaintext, so they must not survive a session on a shared - * device (mirrors the search-index wipe). + * Per-`(eventId, targetLang, textFingerprint)` cache slot, derived from the + * shared `translationCacheAtom` array via `selectAtom` with `Object.is` + * equality. `addTranslation` preserves the object reference of every entry it + * doesn't touch, so a message only re-renders when *its own* entry changes — + * fixing the render storm where translating one message re-rendered every + * mounted message subscribed to the whole array. + */ +const entryFamilyOrder: string[] = []; +export const translationEntryAtomFamily = atomFamily((key: string) => { + entryFamilyOrder.push(key); + if (entryFamilyOrder.length > FAMILY_CAP) { + const oldest = entryFamilyOrder.shift(); + if (oldest !== undefined) translationEntryAtomFamily.remove(oldest); + } + return selectAtom(translationCacheAtom, (list) => findTranslation(list, key), Object.is); +}); + +// Auto-translate "already tried" guard, hoisted to module scope and keyed by +// event id (previously a `useRef` per hook instance, so virtualised scrolling +// re-ran `engine.detect` every time the same message remounted). Stores the +// text last auto-tried so an edit — same event id, new body — still +// re-triggers detection instead of being suppressed by a stale entry. +const AUTO_TRIED_CAP = 500; +const autoTriedTextByEvent = new Map(); + +export const wasAutoTranslateTried = (eventId: string, text: string): boolean => + autoTriedTextByEvent.get(eventId) === text; + +export const markAutoTranslateTried = (eventId: string, text: string): void => { + // Re-insert so the Map's iteration (insertion) order tracks recency for + // the FIFO cap below. + autoTriedTextByEvent.delete(eventId); + autoTriedTextByEvent.set(eventId, text); + if (autoTriedTextByEvent.size > AUTO_TRIED_CAP) { + const oldest = autoTriedTextByEvent.keys().next().value; + if (oldest !== undefined) autoTriedTextByEvent.delete(oldest); + } +}; + +/** + * Wipe the persisted translation cache, every live family member, and the + * auto-translate guard. Called on logout — all of this is derived from + * decrypted message plaintext, so none of it must survive a session on a + * shared device (mirrors the search-index wipe). */ export const clearTranslationCache = (): void => { try { @@ -89,4 +149,7 @@ export const clearTranslationCache = (): void => { } catch { /* localStorage unavailable — nothing to clear */ } + activeFamilyOrder.splice(0).forEach((eventId) => msgTranslationActiveAtomFamily.remove(eventId)); + entryFamilyOrder.splice(0).forEach((key) => translationEntryAtomFamily.remove(key)); + autoTriedTextByEvent.clear(); };