refactor(reader): harden saveConfig updatedAt refresh (#4189)

saveConfig refreshed config.updatedAt by mutating the config object in
place. That only worked because every reader view shares one config
object reference, and it bypassed Zustand change-detection entirely.

Refresh updatedAt via an immutable setConfig store update instead, so it
no longer depends on callers sharing the same reference, notifies
subscribers, and never mutates the caller-provided object. Sync behavior
is unchanged.

Refs #4184

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-05-17 01:56:56 +08:00
committed by GitHub
parent 411d3ad687
commit 40b7c2c15e
2 changed files with 77 additions and 4 deletions
@@ -400,5 +400,73 @@ describe('bookDataStore', () => {
expect(saveBookConfig).not.toHaveBeenCalled();
expect(saveLibraryBooks).not.toHaveBeenCalled();
});
test('refreshes config.updatedAt in the store with a fresh reference', async () => {
const saveBookConfig = vi.fn().mockResolvedValue(undefined);
const saveLibraryBooks = vi.fn().mockResolvedValue(undefined);
const envConfig = makeEnvConfig({ saveBookConfig, saveLibraryBooks });
useLibraryStore.getState().setLibrary([makeLibraryBook({ hash: 'h1' })]);
const data = makeBookData('h1', { progress: [42, 100] });
useBookDataStore.setState({ booksData: { h1: data } });
const before = useBookDataStore.getState().getConfig('h1');
await useBookDataStore.getState().saveConfig(envConfig, 'h1', data.config!, FAKE_SETTINGS);
const after = useBookDataStore.getState().getConfig('h1');
// New reference so Zustand change-detection fires for selector subscribers.
expect(after).not.toBe(before);
expect(after!.updatedAt).toBeGreaterThan(1000);
});
test('refreshes the store config even when the caller passes a separate object', async () => {
const saveBookConfig = vi.fn().mockResolvedValue(undefined);
const saveLibraryBooks = vi.fn().mockResolvedValue(undefined);
const envConfig = makeEnvConfig({ saveBookConfig, saveLibraryBooks });
useLibraryStore.getState().setLibrary([makeLibraryBook({ hash: 'h1' })]);
const data = makeBookData('h1', { progress: [42, 100] });
useBookDataStore.setState({ booksData: { h1: data } });
// Caller passes a freshly-built config, not the shared store object.
const detachedConfig: BookConfig = { ...data.config! };
await useBookDataStore.getState().saveConfig(envConfig, 'h1', detachedConfig, FAKE_SETTINGS);
expect(useBookDataStore.getState().getConfig('h1')!.updatedAt).toBeGreaterThan(1000);
});
test('does not mutate the caller-provided config object', async () => {
const saveBookConfig = vi.fn().mockResolvedValue(undefined);
const saveLibraryBooks = vi.fn().mockResolvedValue(undefined);
const envConfig = makeEnvConfig({ saveBookConfig, saveLibraryBooks });
useLibraryStore.getState().setLibrary([makeLibraryBook({ hash: 'h1' })]);
const data = makeBookData('h1', { progress: [42, 100] });
useBookDataStore.setState({ booksData: { h1: data } });
const detachedConfig: BookConfig = { ...data.config!, updatedAt: 1000 };
await useBookDataStore.getState().saveConfig(envConfig, 'h1', detachedConfig, FAKE_SETTINGS);
expect(detachedConfig.updatedAt).toBe(1000);
});
test('persists a config carrying the refreshed updatedAt', async () => {
const saveBookConfig = vi.fn().mockResolvedValue(undefined);
const saveLibraryBooks = vi.fn().mockResolvedValue(undefined);
const envConfig = makeEnvConfig({ saveBookConfig, saveLibraryBooks });
useLibraryStore.getState().setLibrary([makeLibraryBook({ hash: 'h1' })]);
const data = makeBookData('h1', { progress: [42, 100] });
useBookDataStore.setState({ booksData: { h1: data } });
await useBookDataStore.getState().saveConfig(envConfig, 'h1', data.config!, FAKE_SETTINGS);
const persistedConfig = saveBookConfig.mock.calls[0]![1] as BookConfig;
expect(persistedConfig.updatedAt).toBeGreaterThan(1000);
});
});
});
+9 -4
View File
@@ -86,18 +86,23 @@ export const useBookDataStore = create<BookDataState>((set, get) => ({
// progress and timestamps. We do NOT mutate the existing book object or
// the existing library array — Zustand subscribers see fresh references
// and the visibleLibrary cache stays in sync via setLibrary's full update.
const now = Date.now();
const original = library[idx]!;
const updatedBook: Book = {
...original,
progress: config.progress,
updatedAt: Date.now(),
downloadedAt: original.downloadedAt || Date.now(),
updatedAt: now,
downloadedAt: original.downloadedAt || now,
};
const newLibrary = [updatedBook, ...library.slice(0, idx), ...library.slice(idx + 1)];
setLibrary(newLibrary);
config.updatedAt = Date.now();
await appService.saveBookConfig(updatedBook, config, settings);
// Refresh updatedAt immutably via the store rather than mutating the
// caller-provided object. This notifies Zustand subscribers and works
// regardless of whether the caller passed the shared store config.
get().setConfig(bookKey, { updatedAt: now });
const configToSave = { ...config, updatedAt: now };
await appService.saveBookConfig(updatedBook, configToSave, settings);
await appService.saveLibraryBooks(useLibraryStore.getState().library);
},
updateBooknotes: (key: string, booknotes: BookNote[]) => {