From e4bb9fc4b791cf166daff41bea8d49751f8ca208 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 2 Jun 2026 22:49:47 +0800 Subject: [PATCH] refactor(share): make saveFile content nullable for path-based shares (#4424) The book "Send" flow had to pass `new ArrayBuffer(0)` to saveFile purely to satisfy the type-checker: the content arg is ignored on the native share path when `options.filePath` points at an already-on-disk file. Widen saveFile's content parameter to `string | ArrayBuffer | null` across the AppService contract so callers can hand off a file by path without buffering it into memory, and pass `null` from the Send flow instead of a throwaway empty buffer. Co-authored-by: Claude Opus 4.8 (1M context) --- .../services/native-app-service-share.test.ts | 21 +++++++++++++++++++ .../src/app/library/components/Bookshelf.tsx | 7 +++---- apps/readest-app/src/services/appService.ts | 2 +- .../src/services/nativeAppService.ts | 6 +++--- .../src/services/nodeAppService.ts | 4 ++-- .../readest-app/src/services/webAppService.ts | 9 +++++--- apps/readest-app/src/types/system.ts | 5 ++++- 7 files changed, 40 insertions(+), 14 deletions(-) diff --git a/apps/readest-app/src/__tests__/services/native-app-service-share.test.ts b/apps/readest-app/src/__tests__/services/native-app-service-share.test.ts index 1bb8ee09..0d503c2f 100644 --- a/apps/readest-app/src/__tests__/services/native-app-service-share.test.ts +++ b/apps/readest-app/src/__tests__/services/native-app-service-share.test.ts @@ -107,4 +107,25 @@ describe('NativeAppService.saveFile share gating', () => { expect(shareFileMock).not.toHaveBeenCalled(); expect(saveDialogMock).toHaveBeenCalledTimes(1); }); + + // The book "Send" flow hands an already-on-disk file straight to the share + // sheet via `filePath` and passes `null` content so nothing gets re-buffered + // into memory. The file at `filePath` must be shared verbatim without any + // write happening first. + test('shares the file at filePath without buffering when content is null', async () => { + const service = await loadServiceWithOS('macos'); + await service.saveFile('book.epub', null, { + share: true, + mimeType: 'application/epub+zip', + filePath: '/abs/path/book.epub', + }); + expect(shareFileMock).toHaveBeenCalledTimes(1); + expect(shareFileMock).toHaveBeenCalledWith( + '/abs/path/book.epub', + expect.objectContaining({ mimeType: 'application/epub+zip' }), + ); + expect(writeFileMock).not.toHaveBeenCalled(); + expect(writeTextFileMock).not.toHaveBeenCalled(); + expect(saveDialogMock).not.toHaveBeenCalled(); + }); }); diff --git a/apps/readest-app/src/app/library/components/Bookshelf.tsx b/apps/readest-app/src/app/library/components/Bookshelf.tsx index 02098877..359c0864 100644 --- a/apps/readest-app/src/app/library/components/Bookshelf.tsx +++ b/apps/readest-app/src/app/library/components/Bookshelf.tsx @@ -517,10 +517,9 @@ const Bookshelf: React.FC = ({ // whole epub/pdf into memory just to have saveFile write it back // to disk. const absoluteFilePath = await appService.resolveFilePath(path, base); - // saveFile's binary branch wants an ArrayBuffer; with `filePath` - // set the content arg is ignored on the native share path, but - // we still pass an empty buffer so the type-checker is happy. - await appService.saveFile(shareFilename, new ArrayBuffer(0), { + // `null` content: there's nothing to write — the file already lives at + // `filePath`, which the native share path reads directly. + await appService.saveFile(shareFilename, null, { share: true, mimeType, filePath: absoluteFilePath, diff --git a/apps/readest-app/src/services/appService.ts b/apps/readest-app/src/services/appService.ts index a08c2550..c3d829a2 100644 --- a/apps/readest-app/src/services/appService.ts +++ b/apps/readest-app/src/services/appService.ts @@ -77,7 +77,7 @@ export abstract class BaseAppService implements AppService { abstract selectFiles(name: string, extensions: string[]): Promise; abstract saveFile( filename: string, - content: string | ArrayBuffer, + content: string | ArrayBuffer | null, options?: { filePath?: string; mimeType?: string; diff --git a/apps/readest-app/src/services/nativeAppService.ts b/apps/readest-app/src/services/nativeAppService.ts index 65a5ca16..0447b763 100644 --- a/apps/readest-app/src/services/nativeAppService.ts +++ b/apps/readest-app/src/services/nativeAppService.ts @@ -633,7 +633,7 @@ export class NativeAppService extends BaseAppService { async saveFile( filename: string, - content: string | ArrayBuffer, + content: string | ArrayBuffer | null, options?: { filePath?: string; mimeType?: string; @@ -655,7 +655,7 @@ export class NativeAppService extends BaseAppService { shareablePath = await this.resolveFilePath(filename, 'Temp'); if (typeof content === 'string') { await writeTextFile(shareablePath, content); - } else { + } else if (content) { await writeFile(shareablePath, new Uint8Array(content)); } } @@ -688,7 +688,7 @@ export class NativeAppService extends BaseAppService { if (typeof content === 'string') { await writeTextFile(filePath, content); - } else { + } else if (content) { await writeFile(filePath, new Uint8Array(content)); } return true; diff --git a/apps/readest-app/src/services/nodeAppService.ts b/apps/readest-app/src/services/nodeAppService.ts index 410acb4c..3585dc86 100644 --- a/apps/readest-app/src/services/nodeAppService.ts +++ b/apps/readest-app/src/services/nodeAppService.ts @@ -386,7 +386,7 @@ export class NodeAppService extends BaseAppService { async saveFile( _filename: string, - content: string | ArrayBuffer, + content: string | ArrayBuffer | null, options?: { filePath?: string; mimeType?: string; @@ -399,7 +399,7 @@ export class NodeAppService extends BaseAppService { await fsp.mkdir(nodePath.dirname(filepath), { recursive: true }); if (typeof content === 'string') { await fsp.writeFile(filepath, content, 'utf-8'); - } else { + } else if (content) { await fsp.writeFile(filepath, Buffer.from(content)); } return true; diff --git a/apps/readest-app/src/services/webAppService.ts b/apps/readest-app/src/services/webAppService.ts index 0a10cd87..302ee53b 100644 --- a/apps/readest-app/src/services/webAppService.ts +++ b/apps/readest-app/src/services/webAppService.ts @@ -333,7 +333,7 @@ export class WebAppService extends BaseAppService { async saveFile( filename: string, - content: string | ArrayBuffer, + content: string | ArrayBuffer | null, options?: { filePath?: string; mimeType?: string; @@ -344,6 +344,9 @@ export class WebAppService extends BaseAppService { }, ): Promise { const mimeType = options?.mimeType || 'application/octet-stream'; + // Web has no filesystem path to read from, so `null` content (only the + // native-only "Send" flow passes it) degrades to an empty body. + const body = content ?? ''; if ( options?.share && typeof navigator !== 'undefined' && @@ -351,7 +354,7 @@ export class WebAppService extends BaseAppService { ) { let shareData: ShareData | null = null; try { - const file = new File([content], filename, { type: mimeType }); + const file = new File([body], filename, { type: mimeType }); const candidate: ShareData = { files: [file], title: filename }; if (typeof navigator.canShare !== 'function' || navigator.canShare(candidate)) { shareData = candidate; @@ -377,7 +380,7 @@ export class WebAppService extends BaseAppService { } } try { - const blob = new Blob([content], { type: mimeType }); + const blob = new Blob([body], { type: mimeType }); const url = URL.createObjectURL(blob); const a = document.createElement('a'); a.href = url; diff --git a/apps/readest-app/src/types/system.ts b/apps/readest-app/src/types/system.ts index c4903f4b..2143aee2 100644 --- a/apps/readest-app/src/types/system.ts +++ b/apps/readest-app/src/types/system.ts @@ -128,9 +128,12 @@ export interface AppService { * the current session. */ allowPathsInScopes?(paths: string[], isDirectory: boolean): Promise; + // Pass `null` for `content` when `options.filePath` already points to the + // file on disk you want to save/share — the native share path reads it + // directly instead of buffering an in-memory copy. saveFile( filename: string, - content: string | ArrayBuffer, + content: string | ArrayBuffer | null, options?: { filePath?: string; mimeType?: string;