From 2ff60c3a40892002e2a18523991799b9f5c30331 Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Wed, 9 Sep 2026 18:56:49 -0600 Subject: [PATCH] chore: act on the unambiguous findings from the architecture review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four of the nine findings in #81 needed no judgement call, so they are done here. The rest — the layering leak into the database schema, the two oversized modules, the test-server footgun and the comment density — are questions about structure and house style, and they are left in the issue to be weighed rather than decided in a cleanup commit. Runtime dependencies were all declared as dev dependencies, and `dependencies` was empty. arctic signs users in; drizzle-orm and @libsql/client reach the database. It worked only because everything is bundled at build time — it breaks on any install with `--omit=dev`, and it quietly files every advisory against those three as a development-only risk. The lockfile is regenerated so `npm ci` still resolves. Two exports were dead. `RATE_LIMIT` was never read: the real ceiling lives on the Cloudflare binding, so the constant was a copy of configuration that could drift from it silently and nothing would notice. `loadShareSettings` arrived with the share feature and was never wired up — My List reads those columns inline — so it and the suite that only exercised it both go. The clipboard fallback existed three times, and its wording had already started to drift between the copies. What is genuinely shared is the write and the sentence said when a browser refuses it; what differs is how each caller gets the text in front of the reader, so that part is a callback. The share button needs it: unlike the two panels it has no field on screen until the clipboard has actually refused, so revealing one is part of its fallback. `worker-configuration.d.ts` is marked generated. It is wrangler output — about twenty lines of this project's bindings inside ~14,800 lines of pinned workerd typings — and committing it is correct, because CI has no generate step and would fail the type-check without it. Marking it keeps it out of diffs and out of the language breakdown; it is not refactored. --- .gitattributes | 6 +++- package-lock.json | 38 +++----------------- package.json | 8 +++-- src/lib/components/media/CalendarFeed.svelte | 23 +++--------- src/lib/components/media/ShareButton.svelte | 25 +++++++------ src/lib/components/media/ShareList.svelte | 22 +++--------- src/lib/forms/feedback.ts | 26 ++++++++++++++ src/lib/server/rate-limit.ts | 3 -- src/lib/server/share.spec.ts | 21 ++--------- src/lib/server/share.ts | 15 -------- 10 files changed, 64 insertions(+), 123 deletions(-) diff --git a/.gitattributes b/.gitattributes index a9640ea..09cbf2d 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,4 +3,8 @@ # `wrangler types --check`, and therefore the build, fail spuriously. * text=auto eol=lf *.png binary -*.svg text eol=lf \ No newline at end of file +*.svg text eol=lf +# Wrangler regenerates this in full; it is ~14,800 lines of pinned workerd +# runtime typings around ~20 lines of this project's own bindings. Marking it +# generated keeps it out of diffs and out of the language breakdown. +worker-configuration.d.ts linguist-generated=true diff --git a/package-lock.json b/package-lock.json index 2db7e08..fc06380 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,11 +7,15 @@ "": { "name": "nextsode", "version": "0.0.1", + "dependencies": { + "@libsql/client": "^0.17.3", + "arctic": "^3.7.0", + "drizzle-orm": "^0.45.2" + }, "devDependencies": { "@eslint/js": "^10.0.1", "@fontsource-variable/outfit": "^5.3.0", "@fontsource-variable/work-sans": "^5.3.0", - "@libsql/client": "^0.17.3", "@playwright/test": "^1.60.0", "@sveltejs/adapter-cloudflare": "^7.2.8", "@sveltejs/kit": "^2.63.0", @@ -19,9 +23,7 @@ "@tailwindcss/vite": "^4.3.0", "@types/node": "^24", "@vitest/browser-playwright": "^4.1.8", - "arctic": "^3.7.0", "drizzle-kit": "^0.31.10", - "drizzle-orm": "^0.45.2", "eslint": "^10.4.1", "eslint-config-prettier": "^10.1.8", "eslint-plugin-svelte": "^3.19.0", @@ -1533,7 +1535,6 @@ "version": "0.17.4", "resolved": "https://registry.npmjs.org/@libsql/client/-/client-0.17.4.tgz", "integrity": "sha512-lYayFWasDV78A+TjlEhr6ubb3odBV6OHjb+wdp8VQcyWWAEIjuwbCHaraEUS4m4yWoo0BvZo96It4VdzZRmRWw==", - "dev": true, "license": "MIT", "dependencies": { "@libsql/core": "^0.17.4", @@ -1547,7 +1548,6 @@ "version": "0.17.4", "resolved": "https://registry.npmjs.org/@libsql/core/-/core-0.17.4.tgz", "integrity": "sha512-LqF9gIvnJ38nmAH1y/ChizHqDO/MO1wLgA96XrraulEEbqXxLjleSH92YWTolbuJKgPUmGu4aJk9W3UnAcxLOQ==", - "dev": true, "license": "MIT", "dependencies": { "js-base64": "^3.7.5" @@ -1560,7 +1560,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1574,7 +1573,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1585,7 +1583,6 @@ "version": "0.10.0", "resolved": "https://registry.npmjs.org/@libsql/hrana-client/-/hrana-client-0.10.0.tgz", "integrity": "sha512-OoA4EMqRAC7kn7V2P6EQqRcpZf2W+AjsNIyCizBg339Tq/aMC7sRnzs3SklderhmQWAqEzvv8A2vhxVmWpkVvw==", - "dev": true, "license": "MIT", "dependencies": { "@libsql/isomorphic-ws": "^0.1.5", @@ -1596,7 +1593,6 @@ "version": "0.1.5", "resolved": "https://registry.npmjs.org/@libsql/isomorphic-ws/-/isomorphic-ws-0.1.5.tgz", "integrity": "sha512-DtLWIH29onUYR00i0GlQ3UdcTRC6EP4u9w/h9LxpUZJWRMARk6dQwZ6Jkd+QdwVpuAOrdxt18v0K2uIYR3fwFg==", - "dev": true, "license": "MIT", "dependencies": { "@types/ws": "^8.5.4", @@ -1610,7 +1606,6 @@ "cpu": [ "arm" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1624,7 +1619,6 @@ "cpu": [ "arm" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1638,7 +1632,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1652,7 +1645,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1666,7 +1658,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1680,7 +1671,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1694,7 +1684,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1724,7 +1713,6 @@ "version": "0.0.4", "resolved": "https://registry.npmjs.org/@neon-rs/load/-/load-0.0.4.tgz", "integrity": "sha512-kTPhdZyTQxB+2wpiRcFWrDcejc4JI6tkPuS7UZCG4l6Zvc5kU/gGQ/ozvHTh1XR5tS+UlfAfGuPajjzQjCiHCw==", - "dev": true, "license": "MIT" }, "node_modules/@oslojs/asn1": { @@ -1732,7 +1720,6 @@ "resolved": "https://registry.npmjs.org/@oslojs/asn1/-/asn1-1.0.0.tgz", "integrity": "sha512-zw/wn0sj0j0QKbIXfIlnEcTviaCzYOY3V5rAyjR6YtOByFtJiT574+8p9Wlach0lZH9fddD4yb9laEAIl4vXQA==", "deprecated": "Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.", - "dev": true, "license": "MIT", "dependencies": { "@oslojs/binary": "1.0.0" @@ -1743,7 +1730,6 @@ "resolved": "https://registry.npmjs.org/@oslojs/binary/-/binary-1.0.0.tgz", "integrity": "sha512-9RCU6OwXU6p67H4NODbuxv2S3eenuQ4/WFLrsq+K/k682xrznH5EVWA7N4VFk9VYVcbFtKqur5YQQZc0ySGhsQ==", "deprecated": "Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.", - "dev": true, "license": "MIT" }, "node_modules/@oslojs/crypto": { @@ -1751,7 +1737,6 @@ "resolved": "https://registry.npmjs.org/@oslojs/crypto/-/crypto-1.0.1.tgz", "integrity": "sha512-7n08G8nWjAr/Yu3vu9zzrd0L9XnrJfpMioQcvCMxBIiF5orECHe5/3J0jmXRVvgfqMm/+4oxlQ+Sq39COYLcNQ==", "deprecated": "Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.", - "dev": true, "license": "MIT", "dependencies": { "@oslojs/asn1": "1.0.0", @@ -1762,7 +1747,6 @@ "version": "1.1.0", "resolved": "https://registry.npmjs.org/@oslojs/encoding/-/encoding-1.1.0.tgz", "integrity": "sha512-70wQhgYmndg4GCPxPPxPGevRKqTIJ2Nh4OkiMWmDAVYsTQ+Ta7Sq+rPevXyXGdzr30/qZBnyOalCszoMxlyldQ==", - "dev": true, "license": "MIT" }, "node_modules/@oslojs/jwt": { @@ -1770,7 +1754,6 @@ "resolved": "https://registry.npmjs.org/@oslojs/jwt/-/jwt-0.2.0.tgz", "integrity": "sha512-bLE7BtHrURedCn4Mco3ma9L4Y1GR2SMBuIvjWr7rmQ4/W/4Jy70TIAgZ+0nIlk0xHz1vNP8x8DCns45Sb2XRbg==", "deprecated": "Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.", - "dev": true, "license": "MIT", "dependencies": { "@oslojs/encoding": "0.4.1" @@ -1780,7 +1763,6 @@ "version": "0.4.1", "resolved": "https://registry.npmjs.org/@oslojs/encoding/-/encoding-0.4.1.tgz", "integrity": "sha512-hkjo6MuIK/kQR5CrGNdAPZhS01ZCXuWDRJ187zh6qqF2+yMHZpD9fAYpX8q2bOO6Ryhl3XpCT6kUX76N8hhm4Q==", - "dev": true, "license": "MIT" }, "node_modules/@oxc-project/types": { @@ -2616,7 +2598,6 @@ "version": "24.13.3", "resolved": "https://registry.npmjs.org/@types/node/-/node-24.13.3.tgz", "integrity": "sha512-Dh8vAsV36ig5wa9OX4pXvMc9D3Veibfw2wix0CUwYODLD8nkj9UsLjASr49nPg+2eKzxhBV+v7L8pXvT4e639Q==", - "dev": true, "license": "MIT", "dependencies": { "undici-types": "~7.18.0" @@ -2633,7 +2614,6 @@ "version": "8.18.1", "resolved": "https://registry.npmjs.org/@types/ws/-/ws-8.18.1.tgz", "integrity": "sha512-ThVF6DCVhA8kUGy+aazFQ4kXQ7E1Ty7A3ypFOe0IcJV8O/M511G99AW24irKrW56Wt44yG9+ij8FaqoBGkuBXg==", - "dev": true, "license": "MIT", "dependencies": { "@types/node": "*" @@ -3074,7 +3054,6 @@ "resolved": "https://registry.npmjs.org/arctic/-/arctic-3.7.0.tgz", "integrity": "sha512-ZMQ+f6VazDgUJOd+qNV+H7GohNSYal1mVjm5kEaZfE2Ifb7Ss70w+Q7xpJC87qZDkMZIXYf0pTIYZA0OPasSbw==", "deprecated": "Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.", - "dev": true, "license": "MIT", "dependencies": { "@oslojs/crypto": "1.0.1", @@ -3273,7 +3252,6 @@ "version": "2.0.2", "resolved": "https://registry.npmjs.org/detect-libc/-/detect-libc-2.0.2.tgz", "integrity": "sha512-UX6sGumvvqSaXgdKGUsgZWqcUyIXZ/vZTrlRT/iobiKhGL0zL4d3osHj3uqllWJK+i+sixDS/3COVEOFbupFyw==", - "dev": true, "license": "Apache-2.0", "engines": { "node": ">=8" @@ -3306,7 +3284,6 @@ "version": "0.45.2", "resolved": "https://registry.npmjs.org/drizzle-orm/-/drizzle-orm-0.45.2.tgz", "integrity": "sha512-kY0BSaTNYWnoDMVoyY8uxmyHjpJW1geOmBMdSSicKo9CIIWkSxMIj2rkeSR51b8KAPB7m+qysjuHme5nKP+E5Q==", - "dev": true, "license": "Apache-2.0", "peerDependencies": { "@aws-sdk/client-rds-data": ">=3", @@ -4002,7 +3979,6 @@ "version": "3.9.1", "resolved": "https://registry.npmjs.org/js-base64/-/js-base64-3.9.1.tgz", "integrity": "sha512-U73qptcvf/HIOauFOmqT3a0mDUp0MYlfd15oqoe9kqZt5XhiXVb+HG09sLvI9PQ9tZIBFS4nlErai8zbWazP0g==", - "dev": true, "license": "BSD-3-Clause" }, "node_modules/json-buffer": { @@ -4077,7 +4053,6 @@ "wasm32", "arm" ], - "dev": true, "license": "MIT", "os": [ "darwin", @@ -4970,7 +4945,6 @@ "version": "2.7.0", "resolved": "https://registry.npmjs.org/promise-limit/-/promise-limit-2.7.0.tgz", "integrity": "sha512-7nJ6v5lnJsXwGprnGXga4wx6d1POjvi5Qmf1ivTRxTjH4Z/9Czja/UCMLVmB9N93GeWOU93XaFaEt6jbuoagNw==", - "dev": true, "license": "ISC" }, "node_modules/punycode": { @@ -5554,7 +5528,6 @@ "version": "7.18.2", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", - "dev": true, "license": "MIT" }, "node_modules/unenv": { @@ -5907,7 +5880,6 @@ "version": "8.21.1", "resolved": "https://registry.npmjs.org/ws/-/ws-8.21.1.tgz", "integrity": "sha512-+0NTnW77fFN/DjQi6k/Sq/Yvk4Sgajw7urW8V+asjXnRgDs9gyGkdb7EzgfhA4goXsRIZKE28fzIXBHEzhuiWw==", - "dev": true, "license": "MIT", "engines": { "node": ">=10.0.0" diff --git a/package.json b/package.json index 706bd31..50acace 100644 --- a/package.json +++ b/package.json @@ -22,11 +22,15 @@ "test": "npm run test:unit -- --run && npm run test:e2e", "test:e2e": "playwright install && playwright test" }, + "dependencies": { + "@libsql/client": "^0.17.3", + "arctic": "^3.7.0", + "drizzle-orm": "^0.45.2" + }, "devDependencies": { "@eslint/js": "^10.0.1", "@fontsource-variable/outfit": "^5.3.0", "@fontsource-variable/work-sans": "^5.3.0", - "@libsql/client": "^0.17.3", "@playwright/test": "^1.60.0", "@sveltejs/adapter-cloudflare": "^7.2.8", "@sveltejs/kit": "^2.63.0", @@ -34,9 +38,7 @@ "@tailwindcss/vite": "^4.3.0", "@types/node": "^24", "@vitest/browser-playwright": "^4.1.8", - "arctic": "^3.7.0", "drizzle-kit": "^0.31.10", - "drizzle-orm": "^0.45.2", "eslint": "^10.4.1", "eslint-config-prettier": "^10.1.8", "eslint-plugin-svelte": "^3.19.0", diff --git a/src/lib/components/media/CalendarFeed.svelte b/src/lib/components/media/CalendarFeed.svelte index 643871e..8497e51 100644 --- a/src/lib/components/media/CalendarFeed.svelte +++ b/src/lib/components/media/CalendarFeed.svelte @@ -2,7 +2,7 @@ import { enhance } from '$app/forms'; import Icon from '$lib/components/ui/Icon.svelte'; import { toasts } from '$lib/stores/toasts.svelte'; - import { absorb } from '$lib/forms/feedback'; + import { absorb, copyToClipboard } from '$lib/forms/feedback'; import { subscribeLinks } from '$lib/domain/calendar'; import type { SubmitFunction } from '@sveltejs/kit'; @@ -37,23 +37,10 @@ }; }; - /** - * Copy, or fall back to selecting the text. - * - * `navigator.clipboard` needs a secure context and a permission that can be - * refused. When it is not there, leaving the URL selected turns the failure - * into one keystroke rather than a dead button. - */ - async function copy() { - if (!url) return; - try { - await navigator.clipboard.writeText(url); - toasts.add('Calendar link copied'); - } catch { - field?.select(); - toasts.add('Press Ctrl/Cmd + C to copy the link', 'info'); - } - } + /** The field is already on screen here, so refusal only has to select it. */ + const copy = () => + url && + copyToClipboard(url, { success: 'Calendar link copied', onRefused: () => field?.select() });
import { tick } from 'svelte'; import Icon from '$lib/components/ui/Icon.svelte'; - import { toasts } from '$lib/stores/toasts.svelte'; + import { copyToClipboard } from '$lib/forms/feedback'; /** * Hand this title's public link to somebody. @@ -45,18 +45,17 @@ } } - try { - await navigator.clipboard.writeText(url); - toasts.add('Link copied'); - return; - } catch { - revealed = true; - } - - // After the input has been rendered, so there is something to select. - await tick(); - field?.select(); - toasts.add('Press Ctrl/Cmd + C to copy the link', 'info'); + // Unlike the panels, this button has no field on screen until the clipboard + // has actually refused — so revealing one is part of the fallback, and the + // tick is what gives it time to exist before it is selected. + await copyToClipboard(url, { + success: 'Link copied', + onRefused: async () => { + revealed = true; + await tick(); + field?.select(); + } + }); } diff --git a/src/lib/components/media/ShareList.svelte b/src/lib/components/media/ShareList.svelte index e7754ca..998f100 100644 --- a/src/lib/components/media/ShareList.svelte +++ b/src/lib/components/media/ShareList.svelte @@ -3,7 +3,7 @@ import { enhance } from '$app/forms'; import Icon from '$lib/components/ui/Icon.svelte'; import { toasts } from '$lib/stores/toasts.svelte'; - import { absorb } from '$lib/forms/feedback'; + import { absorb, copyToClipboard } from '$lib/forms/feedback'; import { scopeFromChoices, type ShareScope } from '$lib/domain/share'; import type { SubmitFunction } from '@sveltejs/kit'; @@ -76,23 +76,9 @@ }; }; - /** - * Copy, or fall back to selecting the text. - * - * `navigator.clipboard` needs a secure context and a permission that can be - * refused. When it is not there, leaving the URL selected turns the failure - * into one keystroke rather than a dead button. - */ - async function copy() { - if (!url) return; - try { - await navigator.clipboard.writeText(url); - toasts.add('Share link copied'); - } catch { - field?.select(); - toasts.add('Press Ctrl/Cmd + C to copy the link', 'info'); - } - } + /** The field is already on screen here, so refusal only has to select it. */ + const copy = () => + url && copyToClipboard(url, { success: 'Share link copied', onRefused: () => field?.select() });
void | Promise } +): Promise { + try { + await navigator.clipboard.writeText(text); + toasts.add(success); + } catch { + await onRefused?.(); + toasts.add('Press Ctrl/Cmd + C to copy the link', 'info'); + } +} diff --git a/src/lib/server/rate-limit.ts b/src/lib/server/rate-limit.ts index 3e553a8..6be1cfe 100644 --- a/src/lib/server/rate-limit.ts +++ b/src/lib/server/rate-limit.ts @@ -20,9 +20,6 @@ import { error, type RequestEvent } from '@sveltejs/kit'; * against a distributed caller, and nothing here should be read as one. */ -/** Requests allowed per key per window; mirrors `wrangler.jsonc`. */ -export const RATE_LIMIT = 60; - /** * Who is asking, as far as the edge can tell. * diff --git a/src/lib/server/share.spec.ts b/src/lib/server/share.spec.ts index f5d6a19..2a6d883 100644 --- a/src/lib/server/share.spec.ts +++ b/src/lib/server/share.spec.ts @@ -16,14 +16,8 @@ import { user, watchlistItem } from './db/schema'; let harness: TestDatabase; vi.mock('./db', () => ({ getDb: () => harness.db })); -const { - generateShareToken, - issueShareToken, - loadShareSettings, - loadSharedList, - revokeShareToken, - setShareScope -} = await import('./share'); +const { generateShareToken, issueShareToken, loadSharedList, revokeShareToken, setShareScope } = + await import('./share'); async function saveTitle(userId: string, over: Record = {}) { await harness.db.insert(watchlistItem).values({ @@ -219,14 +213,3 @@ describe('loadSharedList', () => { expect(list?.counts).toEqual({ toWatch: 0, watched: 0 }); }); }); - -describe('loadShareSettings', () => { - it('reports no link before one is made', async () => { - expect(await loadShareSettings('user-1')).toEqual({ token: null, scope: 'toWatch' }); - }); - - it('reports the link and what it shows', async () => { - const token = await issueShareToken('user-1', 'watched'); - expect(await loadShareSettings('user-1')).toEqual({ token, scope: 'watched' }); - }); -}); diff --git a/src/lib/server/share.ts b/src/lib/server/share.ts index 9bea3a2..1a42632 100644 --- a/src/lib/server/share.ts +++ b/src/lib/server/share.ts @@ -161,18 +161,3 @@ export async function loadSharedList(token: string): Promise items: rows.filter((row) => scopeIncludes(scope, row.watched)) }; } - -/** The current share settings for one account, for their own My List page. */ -export async function loadShareSettings(userId: string) { - const [row] = await getDb() - .select({ token: user.shareToken, scope: user.shareScope }) - .from(user) - .where(eq(user.id, userId)) - .limit(1); - - return { - token: row?.token ?? null, - // Only meaningful alongside a token; the panel reads it to tick the boxes. - scope: normalizeShareScope(row?.scope) - }; -}