fix(send): library-clobber + perf: lazy-load conversion deps (#4238)
* 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) <noreply@anthropic.com>
* 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) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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 });
|
||||
|
||||
|
||||
@@ -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]);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<ConvertedBook> {
|
||||
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<ConvertedBook> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -148,7 +148,22 @@ export const useLibraryStore = create<LibraryState>((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({
|
||||
|
||||
Reference in New Issue
Block a user