From ded64159b6b44ea32935c71e8ae15ded4faab270 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Wed, 20 May 2026 19:29:46 +0800 Subject: [PATCH] fix(send): library-clobber + perf: lazy-load conversion deps (#4238) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(send): dynamic-import the conversion fallback so /library stays lean conversionWorker.ts value-imported convertToEpub for the no-Worker fallback path. That pulled mammoth, @mozilla/readability, DOMPurify and @zip.js/zip.js into the main bundle — eagerly loaded on /library via useInboxDrainer's static import of conversionWorker. Switch the fallback to `await import('./convertToEpub')`. The worker entry still value-imports convertToEpub for its own chunk; the main-thread fallback only loads the heavy deps when Workers are actually unavailable or fail. Measured on the production web build: - before: /library eagerly loads the 634KB conversion chunk - after: the 634KB chunk + its two ~627KB duplicates are all lazy Co-Authored-By: Claude Opus 4.7 (1M context) * fix(send): never clobber the library when /send writes before it has loaded The /send page (and the inbox drainer when it races the library page's load) called `useLibraryStore.updateBooks(envConfig, [book])` while the store still held the empty initial library — `libraryLoaded: false`. The merge ran against `[]`, so `saveLibraryBooks` persisted just the new book as the *entire* library and sync pushed the clobbered copy to every device. Two-layer fix: 1. Harden `updateBooks`: if `libraryLoaded` is false, load the real library from disk first, then merge — `updateBooks` is now self- protecting against any future caller that forgets the load step. 2. Gate `useInboxDrainer` on `libraryLoaded`. The hook now subscribes to the flag and starts draining the moment the library finishes loading, instead of running the first pass against an empty in-memory copy. Adds a regression test that fails without the store change. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../src/__tests__/store/library-store.test.ts | 35 +++++++++++++++++++ apps/readest-app/src/hooks/useInboxDrainer.ts | 8 +++-- .../send/conversion/conversionWorker.ts | 16 +++++++-- apps/readest-app/src/store/libraryStore.ts | 17 ++++++++- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/apps/readest-app/src/__tests__/store/library-store.test.ts b/apps/readest-app/src/__tests__/store/library-store.test.ts index c4187984..1d24e261 100644 --- a/apps/readest-app/src/__tests__/store/library-store.test.ts +++ b/apps/readest-app/src/__tests__/store/library-store.test.ts @@ -167,6 +167,7 @@ describe('libraryStore', () => { describe('updateBooks', () => { test('persists by default', async () => { + useLibraryStore.getState().setLibrary([]); const saveLibraryBooks = vi.fn().mockResolvedValue(undefined); const envConfig = makeEnvConfig({ saveLibraryBooks }); @@ -175,7 +176,40 @@ describe('libraryStore', () => { expect(saveLibraryBooks).toHaveBeenCalledTimes(1); }); + test('loads the library from disk first when not yet loaded — never clobbers', async () => { + // Reproduces the /send page bug: a caller adds a book without having + // loaded the library, and the merge runs against an empty in-memory + // copy. updateBooks must self-protect by loading from disk first. + const existing = [makeBook({ hash: 'old1' }), makeBook({ hash: 'old2' })]; + const loadLibraryBooks = vi.fn().mockResolvedValue(existing); + const saveLibraryBooks = vi.fn().mockResolvedValue(undefined); + const envConfig = makeEnvConfig({ loadLibraryBooks, saveLibraryBooks }); + + // Store starts empty + libraryLoaded:false (the dangerous state). + await useLibraryStore.getState().updateBooks(envConfig, [makeBook({ hash: 'new1' })]); + + expect(loadLibraryBooks).toHaveBeenCalledTimes(1); + const saved = saveLibraryBooks.mock.calls[0]?.[0]; + expect(saved.map((b: Book) => b.hash).sort()).toEqual(['new1', 'old1', 'old2']); + const state = useLibraryStore.getState(); + expect(state.library).toHaveLength(3); + expect(state.libraryLoaded).toBe(true); + }); + + test('does not re-load when the library is already loaded', async () => { + useLibraryStore.getState().setLibrary([makeBook({ hash: 'a' })]); + const loadLibraryBooks = vi.fn().mockResolvedValue([]); + const saveLibraryBooks = vi.fn().mockResolvedValue(undefined); + const envConfig = makeEnvConfig({ loadLibraryBooks, saveLibraryBooks }); + + await useLibraryStore.getState().updateBooks(envConfig, [makeBook({ hash: 'b' })]); + + expect(loadLibraryBooks).not.toHaveBeenCalled(); + expect(useLibraryStore.getState().library).toHaveLength(2); + }); + test('skips persistence when skipSave: true', async () => { + useLibraryStore.getState().setLibrary([]); const saveLibraryBooks = vi.fn().mockResolvedValue(undefined); const envConfig = makeEnvConfig({ saveLibraryBooks }); @@ -187,6 +221,7 @@ describe('libraryStore', () => { }); test('still updates store state when skipSave: true', async () => { + useLibraryStore.getState().setLibrary([]); const saveLibraryBooks = vi.fn().mockResolvedValue(undefined); const envConfig = makeEnvConfig({ saveLibraryBooks }); diff --git a/apps/readest-app/src/hooks/useInboxDrainer.ts b/apps/readest-app/src/hooks/useInboxDrainer.ts index 78616d54..0a87b7f9 100644 --- a/apps/readest-app/src/hooks/useInboxDrainer.ts +++ b/apps/readest-app/src/hooks/useInboxDrainer.ts @@ -41,6 +41,7 @@ export function useInboxDrainer(): void { const { envConfig, appService } = useEnv(); const { user } = useAuth(); const { settings } = useSettingsStore(); + const libraryLoaded = useLibraryStore((s) => s.libraryLoaded); const runningRef = useRef(false); const lastDrainAtRef = useRef(0); @@ -162,7 +163,10 @@ export function useInboxDrainer(): void { }, [user, appService, settings, envConfig]); useEffect(() => { - if (!user) return; + // Hold off until the library has loaded — otherwise the very first + // drained item would force `updateBooks` to fall back to its own disk + // load. Once `libraryLoaded` flips true this effect re-runs. + if (!user || !libraryLoaded) return; void runDrain(); const interval = setInterval(() => void runDrain(), DRAIN_INTERVAL_MS); const onFocus = () => void runDrain(); @@ -171,7 +175,7 @@ export function useInboxDrainer(): void { clearInterval(interval); window.removeEventListener('focus', onFocus); }; - }, [user, runDrain]); + }, [user, libraryLoaded, runDrain]); } /** diff --git a/apps/readest-app/src/services/send/conversion/conversionWorker.ts b/apps/readest-app/src/services/send/conversion/conversionWorker.ts index 5e1b8a05..4515d3da 100644 --- a/apps/readest-app/src/services/send/conversion/conversionWorker.ts +++ b/apps/readest-app/src/services/send/conversion/conversionWorker.ts @@ -1,4 +1,4 @@ -import { convertToEpub, type ConvertInput } from './convertToEpub'; +import type { ConvertInput } from './convertToEpub'; import type { ConvertedBook } from './types'; import type { ConversionWorkerRequest, @@ -60,6 +60,16 @@ async function convertInWorker(input: ConvertInput, timeoutMs: number): Promise< }); } +/** + * Main-thread fallback. Dynamic-imported so the heavy conversion deps + * (mammoth, Readability, DOMPurify, zip.js) never load on the main thread + * unless the Worker path is actually unavailable. + */ +async function convertOnMainThread(input: ConvertInput): Promise { + const { convertToEpub } = await import('./convertToEpub'); + return convertToEpub(input); +} + /** * Convert a document to EPUB off the main thread, falling back to in-thread * conversion when Web Workers are unavailable or the worker fails. @@ -69,13 +79,13 @@ export async function convertToEpubWithWorker( timeoutMs: number = DEFAULT_TIMEOUT_MS, ): Promise { if (typeof Worker === 'undefined') { - return convertToEpub(input); + return convertOnMainThread(input); } try { return await convertInWorker(input, timeoutMs); } catch (error) { console.warn('Conversion worker failed, falling back to main thread:', error); - return convertToEpub(input); + return convertOnMainThread(input); } } diff --git a/apps/readest-app/src/store/libraryStore.ts b/apps/readest-app/src/store/libraryStore.ts index 5bd1d653..90e2b264 100644 --- a/apps/readest-app/src/store/libraryStore.ts +++ b/apps/readest-app/src/store/libraryStore.ts @@ -148,7 +148,22 @@ export const useLibraryStore = create((set, get) => ({ ) => { if (!books?.length) return; - const { library, refreshGroups } = get(); + // Hardening: if a caller (e.g. /send, the inbox drainer) hits us before + // `setLibrary` has populated the store, merging against the empty + // in-memory array would persist `books` as the *entire* library and + // clobber whatever is on disk. Load the real library first. + let { library } = get(); + const { libraryLoaded, refreshGroups } = get(); + if (!libraryLoaded) { + const appService = await envConfig.getAppService(); + library = await appService.loadLibraryBooks(); + set({ + library, + libraryLoaded: true, + hashIndex: buildHashIndex(library), + visibleLibrary: library.filter((b) => !b.deletedAt), + }); + } const newLibrary = Array.from(new Map([...library, ...books].map((b) => [b.hash, b])).values()); set({