From 48825e919a2d169d1b0fc12c37f08b7233b90e89 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:54:39 +0200 Subject: [PATCH] fix: stop rewriting the mail, contacts and calendar caches on every state change zustand's JSON storage serialises the whole partialized slice and writes it to AsyncStorage on every set(), including ones that only flip `loading`. Opening one unread message rewrote the mail cache about five times. `createPersistStorage` now coalesces writes per store: - a slice whose fields are the same references as the last one is dropped without serialising; the email and contacts slices are built through `memoizeSlice`, so a set() that only touches UI flags hands over the same slice - other changes wait up to 1 s and only the newest is serialised, and JSON identical to the stored row, or to the row just hydrated from, isn't written again - pending writes are flushed when the app leaves the foreground and when an account is signed out - the account registry is still written straight away, since the headless push task reads it from AsyncStorage - "Refresh cached data" removes the rows through the stores, so their storage forgets the old JSON and writes the re-fetched data back --- src/lib/clear-cached-data.ts | 6 + src/stores/__tests__/contacts-store.test.ts | 11 ++ .../__tests__/email-cache-persist.test.ts | 7 + src/stores/__tests__/persist-storage.test.ts | 174 +++++++++++++++++- src/stores/account-store.ts | 4 +- src/stores/auth-store.ts | 4 + src/stores/contacts-store.ts | 19 +- src/stores/email-store.ts | 23 ++- src/stores/persist-storage.ts | 123 ++++++++++++- src/test-setup.ts | 1 + 10 files changed, 349 insertions(+), 23 deletions(-) diff --git a/src/lib/clear-cached-data.ts b/src/lib/clear-cached-data.ts index 67b99de1..5ecca916 100644 --- a/src/lib/clear-cached-data.ts +++ b/src/lib/clear-cached-data.ts @@ -26,6 +26,12 @@ export const CACHE_STORAGE_KEYS = [ * sessions are preserved. */ export async function clearCachedData(): Promise { + // Through the stores first: their storage drops any pending write and + // forgets what it last wrote, so the re-fetched data is written back even + // where it matches the old cache. + for (const store of [useEmailStore, useContactsStore, useCalendarStore]) { + store.persist.clearStorage(); + } try { await AsyncStorage.multiRemove(CACHE_STORAGE_KEYS); } catch { diff --git a/src/stores/__tests__/contacts-store.test.ts b/src/stores/__tests__/contacts-store.test.ts index 5dc7de91..f6a3f0af 100644 --- a/src/stores/__tests__/contacts-store.test.ts +++ b/src/stores/__tests__/contacts-store.test.ts @@ -450,6 +450,17 @@ describe('contacts-store', () => { }); }); +describe('persistence', () => { + it('hands the storage the same slice while the cards are unchanged', () => { + const partialize = useContactsStore.persist.getOptions().partialize!; + useContactsStore.setState({ contacts: [card('c1', { media: { p: { kind: 'photo' } } } as never)] }); + + const slice = partialize(useContactsStore.getState()) as { contacts: ContactCard[] }; + expect(slice.contacts[0]).not.toHaveProperty('media'); + expect(partialize({ ...useContactsStore.getState(), loading: true })).toBe(slice); + }); +}); + describe('helpers', () => { it('cleanGroupMembers strips id, uid and urn:uuid variants', () => { const contacts = [ diff --git a/src/stores/__tests__/email-cache-persist.test.ts b/src/stores/__tests__/email-cache-persist.test.ts index aa4795a2..bb64eac1 100644 --- a/src/stores/__tests__/email-cache-persist.test.ts +++ b/src/stores/__tests__/email-cache-persist.test.ts @@ -128,6 +128,13 @@ describe('email-store persistence', () => { expect(JSON.stringify(persisted).match(/"id":"live0"/g)).toHaveLength(1); }); + it('hands the storage the same slice when only UI flags change', () => { + const state = viewState({ emails: rows('live', 3), mailboxSnapshots: {} }); + const slice = partialize!(state); + expect(partialize!({ ...state, loading: true, error: 'offline' })).toBe(slice); + expect(partialize!({ ...state, emails: rows('live', 4) })).not.toBe(slice); + }); + it('keeps the base view, not search results, for the open folder', () => { const base = { emails: rows('base', 2), total: 2, queryState: 'q1' }; const persisted = partialize!(viewState({ diff --git a/src/stores/__tests__/persist-storage.test.ts b/src/stores/__tests__/persist-storage.test.ts index cc7eeb9f..752e886a 100644 --- a/src/stores/__tests__/persist-storage.test.ts +++ b/src/stores/__tests__/persist-storage.test.ts @@ -1,13 +1,36 @@ -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { create } from 'zustand'; import { persist } from 'zustand/middleware'; import AsyncStorage from '@react-native-async-storage/async-storage'; -import { createPersistStorage } from '../persist-storage'; -const getItem = AsyncStorage.getItem as unknown as ReturnType; +const appState = vi.hoisted(() => ({ listeners: [] as Array<(state: string) => void> })); +vi.mock('react-native', () => ({ + AppState: { + currentState: 'active', + addEventListener: (_type: string, cb: (state: string) => void) => { + appState.listeners.push(cb); + return { remove: () => undefined }; + }, + }, +})); + +import { + createPersistStorage, + flushPersistedWrites, + memoizeSlice, + PERSIST_WRITE_DELAY_MS, +} from '../persist-storage'; + +const getItem = vi.mocked(AsyncStorage.getItem); +const setItem = vi.mocked(AsyncStorage.setItem); + +let seq = 0; +const freshName = () => `persist-test-${++seq}`; +const writesTo = (name: string) => setItem.mock.calls.filter(([key]) => key === name).map(([, json]) => json); afterEach(() => { vi.restoreAllMocks(); + vi.useRealTimers(); }); describe('createPersistStorage', () => { @@ -43,4 +66,149 @@ describe('createPersistStorage', () => { await vi.waitFor(() => expect(useStore.persist.hasHydrated()).toBe(true)); expect(useStore.getState().n).toBe(0); }); + + describe('writes', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + it('coalesces changes into one write of the newest slice', async () => { + const storage = createPersistStorage<{ n: number }>(); + const name = freshName(); + storage.setItem(name, { state: { n: 1 }, version: 0 }); + storage.setItem(name, { state: { n: 2 }, version: 0 }); + storage.setItem(name, { state: { n: 3 }, version: 0 }); + + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS - 1); + expect(writesTo(name)).toEqual([]); + await vi.advanceTimersByTimeAsync(1); + expect(writesTo(name)).toEqual([JSON.stringify({ state: { n: 3 }, version: 0 })]); + }); + + it('writes no later than the delay after the first change, however busy the store is', async () => { + const storage = createPersistStorage<{ n: number }>(); + const name = freshName(); + for (let n = 0; n < 10; n++) { + storage.setItem(name, { state: { n }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS / 4); + } + expect(writesTo(name).length).toBeGreaterThanOrEqual(2); + }); + + it('drops a slice whose fields are the same references as the last one', async () => { + const storage = createPersistStorage<{ list: number[] }>(); + const name = freshName(); + const list = [1, 2, 3]; + storage.setItem(name, { state: { list }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + const stringify = vi.spyOn(JSON, 'stringify'); + + storage.setItem(name, { state: { list }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + + expect(stringify).not.toHaveBeenCalled(); + expect(writesTo(name)).toHaveLength(1); + }); + + it('does not rewrite JSON identical to what is stored', async () => { + const storage = createPersistStorage<{ list: number[] }>(); + const name = freshName(); + storage.setItem(name, { state: { list: [1, 2] }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + storage.setItem(name, { state: { list: [1, 2] }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + expect(writesTo(name)).toHaveLength(1); + }); + + it('does not rewrite a slice that matches the row it hydrated from', async () => { + const name = freshName(); + const row = JSON.stringify({ state: { list: [1, 2] }, version: 0 }); + await AsyncStorage.setItem(name, row); + setItem.mockClear(); + const storage = createPersistStorage<{ list: number[] }>(); + + await storage.getItem(name); + storage.setItem(name, { state: { list: [1, 2] }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + + expect(writesTo(name)).toEqual([]); + }); + + it('writes pending slices as soon as the app leaves the foreground', async () => { + const storage = createPersistStorage<{ n: number }>(); + const name = freshName(); + storage.setItem(name, { state: { n: 1 }, version: 0 }); + expect(appState.listeners).toHaveLength(1); + + appState.listeners[0]('background'); + await vi.advanceTimersByTimeAsync(0); + + expect(writesTo(name)).toEqual([JSON.stringify({ state: { n: 1 }, version: 0 })]); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + expect(writesTo(name)).toHaveLength(1); + }); + + it('writes straight away with writeDelayMs 0', async () => { + const storage = createPersistStorage<{ n: number }>({ writeDelayMs: 0 }); + const name = freshName(); + storage.setItem(name, { state: { n: 1 }, version: 0 }); + await vi.advanceTimersByTimeAsync(0); + expect(writesTo(name)).toHaveLength(1); + }); + + it('drops the pending write on remove and writes the same slice again afterwards', async () => { + const storage = createPersistStorage<{ n: number }>(); + const name = freshName(); + storage.setItem(name, { state: { n: 1 }, version: 0 }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + storage.setItem(name, { state: { n: 2 }, version: 0 }); + + await storage.removeItem(name); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + expect(writesTo(name)).toHaveLength(1); + expect(await AsyncStorage.getItem(name)).toBeNull(); + + storage.setItem(name, { state: { n: 1 }, version: 0 }); + await flushPersistedWrites(); + expect(writesTo(name)).toHaveLength(2); + }); + + it('keeps a store that only flips loading from writing', async () => { + const name = freshName(); + type State = { items: string[]; loading: boolean }; + const useStore = create()( + persist((): State => ({ items: [], loading: false }), { + name, + storage: createPersistStorage(), + partialize: (state) => ({ items: state.items }), + }), + ); + await vi.waitFor(() => expect(useStore.persist.hasHydrated()).toBe(true)); + + useStore.setState({ items: ['a'] }); + useStore.setState({ loading: true }); + useStore.setState({ loading: false }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + useStore.setState({ loading: true }); + useStore.setState({ loading: false }); + await vi.advanceTimersByTimeAsync(PERSIST_WRITE_DELAY_MS); + + expect(writesTo(name)).toEqual([JSON.stringify({ state: { items: ['a'] }, version: 0 })]); + }); + }); +}); + +describe('memoizeSlice', () => { + it('returns the previous slice until an input changes', () => { + const build = vi.fn((s: { list: number[]; loading: boolean }) => ({ doubled: s.list.map((n) => n * 2) })); + const partialize = memoizeSlice((s: { list: number[]; loading: boolean }) => [s.list], build); + const list = [1, 2]; + + const first = partialize({ list, loading: false }); + expect(partialize({ list, loading: true })).toBe(first); + expect(build).toHaveBeenCalledTimes(1); + + expect(partialize({ list: [1, 2, 3], loading: true })).toEqual({ doubled: [2, 4, 6] }); + expect(build).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/stores/account-store.ts b/src/stores/account-store.ts index 79da0351..e147754b 100644 --- a/src/stores/account-store.ts +++ b/src/stores/account-store.ts @@ -169,7 +169,9 @@ export const useAccountStore = create()( }), { name: 'account-registry', - storage: createPersistStorage(), + // Tiny, rarely written, and read straight from AsyncStorage by the + // headless push task, so it isn't held back like the caches. + storage: createPersistStorage({ writeDelayMs: 0 }), partialize: (state) => ({ accounts: state.accounts, activeAccountId: state.activeAccountId, diff --git a/src/stores/auth-store.ts b/src/stores/auth-store.ts index 6738499c..423c5ef7 100644 --- a/src/stores/auth-store.ts +++ b/src/stores/auth-store.ts @@ -8,6 +8,7 @@ import { useEmailStore } from './email-store'; import { useContactsStore } from './contacts-store'; import { useCalendarStore } from './calendar-store'; import { useFilterStore } from './filter-store'; +import { flushPersistedWrites } from './persist-storage'; import { generateAccountId } from '../lib/account-utils'; import { runWebmailHandoff, redeemPairingCode, HandoffCancelledError, HandoffError, type HandoffResult } from '../lib/oauth'; import { discoverOAuthMetadata, loginWithPkce, probeWebmail, revokeRefreshToken } from '../lib/oauth-native'; @@ -95,6 +96,8 @@ function clearAllFeatureStores(): void { useContactsStore.getState().reset(); useCalendarStore.getState().reset(); useFilterStore.getState().clearState(); + // Cache writes are held back briefly; get the signed-out data off disk now. + void flushPersistedWrites(); } // Drop the named account from the email cache, then reset the (per-session, @@ -113,6 +116,7 @@ function clearAccountFeatureStores(accountId: string | null): void { useContactsStore.getState().reset(); useCalendarStore.getState().reset(); useFilterStore.getState().clearState(); + void flushPersistedWrites(); } function refetchFeatureStores(): void { diff --git a/src/stores/contacts-store.ts b/src/stores/contacts-store.ts index 883139b6..a0767a78 100644 --- a/src/stores/contacts-store.ts +++ b/src/stores/contacts-store.ts @@ -1,7 +1,7 @@ import { create } from 'zustand'; import { persist } from 'zustand/middleware'; import AsyncStorage from '@react-native-async-storage/async-storage'; -import { createPersistStorage } from './persist-storage'; +import { createPersistStorage, memoizeSlice } from './persist-storage'; import type { ContactCard, AddressBook, StateChange, EmailAddress } from '../api/types'; import { getAddressBooks as fetchPrimaryAddressBooks, @@ -822,13 +822,16 @@ export const useContactsStore = create()( // ready and replaces the cached data. name: 'contacts-cache', storage: createPersistStorage(), - partialize: (state) => ({ - addressBooks: state.addressBooks, - // Photos are inline base64 blobs; a few hundred of them blow past - // Android AsyncStorage's write cap and the cache silently stops - // updating. They are re-hydrated from the server on refresh. - contacts: state.contacts.map(({ media: _media, ...rest }) => rest), - }), + partialize: memoizeSlice( + (state: ContactsState) => [state.addressBooks, state.contacts], + (state) => ({ + addressBooks: state.addressBooks, + // Photos are inline base64 blobs; a few hundred of them blow past + // Android AsyncStorage's write cap and the cache silently stops + // updating. They are re-hydrated from the server on refresh. + contacts: state.contacts.map(({ media: _media, ...rest }) => rest), + }), + ), }, ), ); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index 977289a0..f83e34c6 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -1,6 +1,6 @@ import { create } from 'zustand'; import { persist } from 'zustand/middleware'; -import { createPersistStorage } from './persist-storage'; +import { createPersistStorage, memoizeSlice } from './persist-storage'; import { boundEmailCache, type PersistedEmailCache } from './email-cache-persist'; import type { Email, Mailbox, StateChange } from '../api/types'; import { jmapClient } from '../api/jmap-client'; @@ -1714,12 +1714,21 @@ export const useEmailStore = create()( }, // The active view is stored the way an account switch tucks it away, // so its folder is in the row once and `merge` rebuilds `emails` from - // it. Bounded so Android can still read the row back. - partialize: (state): PersistedEmailCache => boundEmailCache({ - accountSnapshots: state.accountSnapshots, - activeAccountId: state.activeAccountId, - ...snapshotFromActive(state), - }), + // it. Bounded so Android can still read the row back. Memoised on + // everything `snapshotFromActive` reads, so a set() that only flips + // `loading` or `error` isn't written. + partialize: memoizeSlice( + (state: EmailState) => [ + state.accountSnapshots, state.activeAccountId, state.mailboxes, state.mailboxState, + state.emailStates, state.currentMailboxId, state.mailboxSnapshots, state.emails, + state.totalEmails, state.queryState, state.searchQuery, state.filters, + ], + (state): PersistedEmailCache => boundEmailCache({ + accountSnapshots: state.accountSnapshots, + activeAccountId: state.activeAccountId, + ...snapshotFromActive(state), + }), + ), merge: (persisted, current) => { if (!persisted) return current; const cache = persisted as PersistedEmailCache; diff --git a/src/stores/persist-storage.ts b/src/stores/persist-storage.ts index ac1fe778..3a956f7e 100644 --- a/src/stores/persist-storage.ts +++ b/src/stores/persist-storage.ts @@ -6,22 +6,137 @@ // marks a store hydrated when the read succeeds, so a rejected read left // `persist.hasHydrated()` false for good and session restore waiting on it. // Now the store starts empty and its next write replaces the bad row. +// +// `persist` calls setItem with the whole partialized slice on every set(), +// including one that only flips `loading`. Writes are coalesced instead: a +// slice whose fields are the same references as the last one is dropped, the +// rest wait up to PERSIST_WRITE_DELAY_MS and only the newest is serialised, +// and JSON identical to what is stored isn't written again. Pending writes +// go out as soon as the app leaves the foreground. +import { AppState } from 'react-native'; import AsyncStorage from '@react-native-async-storage/async-storage'; import type { PersistStorage, StorageValue } from 'zustand/middleware'; -export function createPersistStorage(): PersistStorage { +/** How long a store's write waits to take in further changes. */ +export const PERSIST_WRITE_DELAY_MS = 1000; + +interface Slot { + /** The last slice the store handed over, written or not. */ + latest?: StorageValue; + /** The slice waiting to be written. */ + pending?: StorageValue; + timer?: ReturnType; + /** The JSON last written or read back. */ + stored?: string; +} + +const slots = new Map(); + +function slotFor(name: string): Slot { + let slot = slots.get(name); + if (!slot) { + slot = {}; + slots.set(name, slot); + } + return slot; +} + +// Same version and every field the same reference: nothing new to persist. +function sameSlice(a: StorageValue, b: StorageValue): boolean { + if (a.version !== b.version) return false; + if (a.state === b.state) return true; + const x = a.state as Record; + const y = b.state as Record; + const keys = Object.keys(x); + return keys.length === Object.keys(y).length && keys.every((k) => Object.is(x[k], y[k])); +} + +async function flush(name: string): Promise { + const slot = slots.get(name); + if (!slot?.pending) return; + clearTimeout(slot.timer); + slot.timer = undefined; + const value = slot.pending; + slot.pending = undefined; + try { + const json = JSON.stringify(value); + if (json === slot.stored) return; + slot.stored = json; + await AsyncStorage.setItem(name, json); + } catch (err) { + slot.stored = undefined; + console.warn(`[persist] could not save '${name}'`, err); + } +} + +/** Writes every store's pending slice now. */ +export async function flushPersistedWrites(): Promise { + await Promise.all([...slots.keys()].map(flush)); +} + +let flushesOnBackground = false; + +/** + * `writeDelayMs: 0` writes each changed slice straight away (still skipping + * unchanged ones), for small stores that other code reads from AsyncStorage. + */ +export function createPersistStorage( + { writeDelayMs = PERSIST_WRITE_DELAY_MS }: { writeDelayMs?: number } = {}, +): PersistStorage { + if (!flushesOnBackground) { + flushesOnBackground = true; + AppState.addEventListener('change', (next) => { + if (next !== 'active') void flushPersistedWrites(); + }); + } return { getItem: async (name) => { try { const raw = await AsyncStorage.getItem(name); - return raw === null ? null : (JSON.parse(raw) as StorageValue); + if (raw === null) return null; + const value = JSON.parse(raw) as StorageValue; + slotFor(name).stored = raw; + return value; } catch (err) { console.warn(`[persist] could not load '${name}', starting empty`, err); return null; } }, - setItem: (name, value) => AsyncStorage.setItem(name, JSON.stringify(value)), - removeItem: (name) => AsyncStorage.removeItem(name), + setItem: (name, value) => { + const slot = slotFor(name); + if (slot.latest && sameSlice(slot.latest, value)) return; + slot.latest = value; + slot.pending = value; + if (writeDelayMs === 0) void flush(name); + else slot.timer ??= setTimeout(() => void flush(name), writeDelayMs); + }, + // Forgets the pending write and the stored JSON too, so the next write + // lands even if it matches what was there before. + removeItem: (name) => { + clearTimeout(slots.get(name)?.timer); + slots.delete(name); + return AsyncStorage.removeItem(name).catch((err) => { + console.warn(`[persist] could not remove '${name}'`, err); + }); + }, + }; +} + +/** + * Memoises a `partialize` that builds new objects (a mapped list, a folded + * view) on the store fields it reads. A set() that leaves those alone then + * hands the storage the same slice, which is dropped without serialising. + */ +export function memoizeSlice( + inputs: (state: T) => readonly unknown[], + build: (state: T) => P, +): (state: T) => P { + let last: { inputs: readonly unknown[]; slice: P } | undefined; + return (state) => { + const next = inputs(state); + if (last && next.every((v, i) => Object.is(v, last!.inputs[i]))) return last.slice; + last = { inputs: next, slice: build(state) }; + return last.slice; }; } diff --git a/src/test-setup.ts b/src/test-setup.ts index 9aedef8a..2f81bb3a 100644 --- a/src/test-setup.ts +++ b/src/test-setup.ts @@ -56,6 +56,7 @@ vi.mock('react-native', () => { }, Linking: { openURL: async () => undefined }, Appearance: { getColorScheme: () => 'dark', addChangeListener: () => ({ remove: () => undefined }) }, + AppState: { currentState: 'active', addEventListener: () => ({ remove: () => undefined }) }, }; });