From 4eb97b86f96b0d11455592907f9d1bac441bab0a Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Sat, 21 Mar 2026 08:43:31 +0100 Subject: [PATCH] fix: address PR review comments - Wrap readFile in try/catch, return 404 for missing attachment files - Remove unused CollaboratorPopover import from NoteCard - Make darkMode a proper $state rune for reactivity (add svelte-runes polyfill for Vitest so $state works in unit tests) - Add error toasts for failed share link toggle (non-2xx responses) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/lib/__mocks__/svelte-runes.ts | 11 ++++++++++ src/lib/components/NoteCard.svelte | 1 - src/lib/components/ShareDialog.svelte | 22 +++++++++++-------- src/lib/utils/theme.svelte.ts | 2 +- .../attachment/[attachmentId]/+server.ts | 20 ++++++++++------- vitest.config.ts | 1 + 6 files changed, 38 insertions(+), 19 deletions(-) create mode 100644 src/lib/__mocks__/svelte-runes.ts diff --git a/src/lib/__mocks__/svelte-runes.ts b/src/lib/__mocks__/svelte-runes.ts new file mode 100644 index 0000000..05f5d94 --- /dev/null +++ b/src/lib/__mocks__/svelte-runes.ts @@ -0,0 +1,11 @@ +/** + * Polyfill Svelte 5 runes for Vitest. + * The Svelte compiler transforms these in .svelte and .svelte.ts files, + * but Vitest doesn't run the Svelte compiler, so we provide plain-JS stubs. + */ +/* eslint-disable @typescript-eslint/no-explicit-any */ + +(globalThis as any).$state = (initial: T): T => initial; +(globalThis as any).$derived = (value: T): T => value; +(globalThis as any).$effect = (_fn: () => void): void => {}; +(globalThis as any).$props = (): any => ({}); diff --git a/src/lib/components/NoteCard.svelte b/src/lib/components/NoteCard.svelte index 8b17b2b..db4b19b 100644 --- a/src/lib/components/NoteCard.svelte +++ b/src/lib/components/NoteCard.svelte @@ -4,7 +4,6 @@ import { renderMarkdown } from '$lib/utils/markdown.js'; import { togglePin, trashNote, archiveNote, unarchiveNote, restoreNote, deleteNote, leaveNote, currentFilter } from '$lib/stores/notes.js'; import ImageLightbox from './ImageLightbox.svelte'; - import CollaboratorPopover from './CollaboratorPopover.svelte'; import SharingIndicator from './SharingIndicator.svelte'; import type { Note } from '$lib/types/index.js'; import Undo2 from 'lucide-svelte/icons/undo-2'; diff --git a/src/lib/components/ShareDialog.svelte b/src/lib/components/ShareDialog.svelte index f7b73ce..d23cada 100644 --- a/src/lib/components/ShareDialog.svelte +++ b/src/lib/components/ShareDialog.svelte @@ -43,19 +43,23 @@ try { if (publicShareToken) { const res = await fetch(`/api/notes/${noteId}/share`, { method: 'DELETE' }); - if (res.ok) { - publicShareToken = null; - onShareUpdate(null); - showToast('Public link removed', 'success'); + if (!res.ok) { + showToast('Failed to remove share link', 'error'); + return; } + publicShareToken = null; + onShareUpdate(null); + showToast('Public link removed', 'success'); } else { const res = await fetch(`/api/notes/${noteId}/share`, { method: 'POST' }); - if (res.ok) { - const data = await res.json(); - publicShareToken = data.token; - onShareUpdate(data.token); - showToast('Public link created', 'success'); + if (!res.ok) { + showToast('Failed to create share link', 'error'); + return; } + const data = await res.json(); + publicShareToken = data.token; + onShareUpdate(data.token); + showToast('Public link created', 'success'); } } catch { showToast('Failed to update share link', 'error'); diff --git a/src/lib/utils/theme.svelte.ts b/src/lib/utils/theme.svelte.ts index c28e022..c40ddb4 100644 --- a/src/lib/utils/theme.svelte.ts +++ b/src/lib/utils/theme.svelte.ts @@ -3,7 +3,7 @@ import { browser } from '$app/environment'; const LIGHT_THEME_COLOR = '#f0e6d3'; const DARK_THEME_COLOR = '#1a1715'; -let darkMode = false; +let darkMode = $state(false); export function getIsDarkMode(): boolean { return darkMode; diff --git a/src/routes/api/shared/[token]/attachment/[attachmentId]/+server.ts b/src/routes/api/shared/[token]/attachment/[attachmentId]/+server.ts index ef4fbc2..418c6a3 100644 --- a/src/routes/api/shared/[token]/attachment/[attachmentId]/+server.ts +++ b/src/routes/api/shared/[token]/attachment/[attachmentId]/+server.ts @@ -26,12 +26,16 @@ export const GET: RequestHandler = async ({ params, url, getClientAddress }) => const filePath = wantThumb && attachment.thumbnailPath ? attachment.thumbnailPath : attachment.path; const mimeType = wantThumb && attachment.thumbnailPath ? 'image/webp' : attachment.mimeType; - const buffer = await readFile(filePath); - return new Response(buffer, { - headers: { - 'Content-Type': mimeType, - 'Content-Disposition': `inline; filename="${attachment.filename}"`, - 'Cache-Control': 'public, max-age=3600' - } - }); + try { + const buffer = await readFile(filePath); + return new Response(buffer, { + headers: { + 'Content-Type': mimeType, + 'Content-Disposition': `inline; filename="${attachment.filename}"`, + 'Cache-Control': 'public, max-age=3600' + } + }); + } catch { + throw error(404, 'Attachment not found'); + } }; diff --git a/vitest.config.ts b/vitest.config.ts index dd92d89..21b52f0 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -11,6 +11,7 @@ export default defineConfig({ test: { include: ['src/**/*.test.ts'], environment: 'node', + setupFiles: ['src/lib/__mocks__/svelte-runes.ts'], coverage: { provider: 'v8', reporter: ['text', 'html', 'lcov', 'json-summary'],