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) <noreply@anthropic.com>
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -517,10 +517,9 @@ const Bookshelf: React.FC<BookshelfProps> = ({
|
||||
// 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,
|
||||
|
||||
@@ -77,7 +77,7 @@ export abstract class BaseAppService implements AppService {
|
||||
abstract selectFiles(name: string, extensions: string[]): Promise<string[]>;
|
||||
abstract saveFile(
|
||||
filename: string,
|
||||
content: string | ArrayBuffer,
|
||||
content: string | ArrayBuffer | null,
|
||||
options?: {
|
||||
filePath?: string;
|
||||
mimeType?: string;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<boolean> {
|
||||
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;
|
||||
|
||||
@@ -128,9 +128,12 @@ export interface AppService {
|
||||
* the current session.
|
||||
*/
|
||||
allowPathsInScopes?(paths: string[], isDirectory: boolean): Promise<void>;
|
||||
// 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;
|
||||
|
||||
Reference in New Issue
Block a user