Format code with Prettier (8 modified files from H1 decomposition)
Aligns with CC2 enforcement: ESLint + Prettier + noImplicitAny now enforced. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
+84
-14
@@ -51,6 +51,18 @@ preview mock + Settings defaults — `DEFAULT_SETTINGS` in `@shared`, one typed
|
||||
eslint green; the touched files are prettier-clean (3 pre-existing format violations in
|
||||
DownloadOptionsForm/LibraryView/SettingsView are untouched and out of scope).
|
||||
|
||||
**Session 2026-07-01 (main-process structure — the H1-adjacent decompose batch):** **L2** (the ~200-line
|
||||
flat IPC block moved into one `src/main/ipc.ts` `registerIpcHandlers(getMainWindow)`; index.ts is now
|
||||
lifecycle + window creation and imports the shared theme helpers from ipc.ts), **L69** (path helpers split
|
||||
into `src/main/paths.ts`; settings.ts is now purely the electron-store layer), **L10** (a main-process
|
||||
`src/main/constants.ts` centralizes the spawn timeouts, execFile buffers, stderr tail, and the three store
|
||||
caps — nine modules migrated off bare literals), **CL2** (`buildArgs` → single `BuildArgsInput` object),
|
||||
**CL3** (`startDownload`'s stream/close/error/watchdog wiring extracted into `wireChildProcess`), and **L1**
|
||||
(the scheduled-download promoter tick moved from store-module load into an App `useEffect` so a store import
|
||||
no longer starts a stray timer). All verified: typecheck (node+web) + 253 tests + eslint + production build
|
||||
green; the touched files are prettier-clean (the 8 remaining prettier warnings are all pre-existing,
|
||||
untouched files — DownloadOptionsForm/SettingsView/settings/*Card).
|
||||
|
||||
**Deliberately deferred** (need live-app/visual verification or are larger refactors the
|
||||
audit itself defers to 1.x): the wire-or-cut features (M5/M6/UX1), the god-file/store
|
||||
refactors (C1, C2, H1), the SIMP* helpers + shared-UI-token work, the CC* consolidations,
|
||||
@@ -439,9 +451,20 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
|
||||
## Low
|
||||
|
||||
- [ ] **L1 — Module-load `setInterval` in `store/downloads.ts`** runs on import (incl. tests/preview).
|
||||
- [ ] **L2 — `index.ts` flat IPC registration (~150 lines)** — let each main module export its own
|
||||
`register(ipcMain)`.
|
||||
- [x] **L1 — Module-load `setInterval` in `store/downloads.ts`** runs on import (incl. tests/preview).
|
||||
*Fixed: the scheduled-download promoter tick moved out of store-module scope into an `App.tsx`
|
||||
`useEffect` (interval `SCHEDULE_TICK_MS`, cleared on unmount). Importing the store in tests/preview no
|
||||
longer starts a stray never-cleared timer; the store keeps the `promoteDueScheduled` action, App owns the
|
||||
cadence. (The preview `startFakeTicker` interval is created on demand per launched item, not at load, so
|
||||
it was already fine.)*
|
||||
- [x] **L2 — `index.ts` flat IPC registration (~150 lines)** — let each main module export its own
|
||||
`register(ipcMain)`. *Fixed: the whole ~200-line handler block moved into one dedicated
|
||||
[ipc.ts](src/main/ipc.ts) exporting `registerIpcHandlers(getMainWindow)` — the single place the renderer's
|
||||
IPC contract is wired. index.ts now imports it (plus the three shared theme helpers `resolveBackgroundMode`
|
||||
/`applyNativeTheme`/`getSystemThemeInfo`, also moved to ipc.ts) and shed ~50 imports it only needed for the
|
||||
handlers, leaving it as app-lifecycle + window creation. The window-coupled handlers (folder picker,
|
||||
theme-synced background, taskbar progress) take the live window via the `getMainWindow` accessor rather
|
||||
than a captured `mainWindow`.*
|
||||
- [x] **L3 — Stale comments.** `getYtdlpVersion` JSDoc still calls it the "Step-1 spike";
|
||||
`vitest.config.ts` claims tests "only exercise buildArgs.ts" (9 test files now exist).
|
||||
- [x] **L4 — `electron-builder.yml` nits.** `copyright: yt-dlp frontend` is not a copyright
|
||||
@@ -458,9 +481,16 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
Diagnostics keys by `id + occurredAt` while every other list keys by `id` alone.
|
||||
- [x] **L9 — Thumbnail box sizes not shared.** Four hand-tuned 16:9 boxes (120×68, 108×64, 72×44,
|
||||
60×34) with no shared aspect/size constant.
|
||||
- [ ] **L10 — Scattered magic numbers.** Timeouts/caps spread across modules (probe 60s, indexer
|
||||
- [x] **L10 — Scattered magic numbers.** Timeouts/caps spread across modules (probe 60s, indexer
|
||||
180s, probeMeta 30s, update-idle 60s; MAX_ENTRIES 500/200, MAX_TEMPLATES 100, MAX_ITEMS 20000).
|
||||
Consider a central constants module.
|
||||
Consider a central constants module. *Fixed: a main-process [constants.ts](src/main/constants.ts) now
|
||||
holds the process-spawn timeouts (`VERSION_TIMEOUT_MS`, `META_PROBE_TIMEOUT_MS`, `PROBE_TIMEOUT_MS`,
|
||||
`INDEX_TIMEOUT_MS`, `YTDLP_UPDATE_TIMEOUT_MS`, `FEED_FETCH_TIMEOUT_MS`, `STALL_TIMEOUT_MS`), the execFile
|
||||
`maxBuffer` sizes (`META`/`PROBE`/`INDEX_MAX_BUFFER`), `STDERR_TAIL_BYTES`, and the three hand-rolled store
|
||||
caps (`ERRORLOG_MAX_ENTRIES`, `TEMPLATES_MAX`, `MEDIA_ITEMS_MAX`). download/ffmpeg/indexer/probe/ytdlp/sync/
|
||||
errorlog/templates/sources import from it instead of bare literals. Values already single-sourced in the
|
||||
shared contract (`HISTORY_MAX_ENTRIES`) and updater.ts's already-named local timeouts were left in place —
|
||||
they aren't scattered literals.*
|
||||
- [x] **L11 — "Queue (N)" overcounts.** DownloadsView's header count is `items.length` (includes
|
||||
completed/error/canceled), not the active queue.
|
||||
- [x] **L12 — Command palette polish.** No scroll-into-view for keyboard selection in the 50vh
|
||||
@@ -568,8 +598,12 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
- [ ] **L67 — Onboarding has no Skip and can't be revisited** (no "show tips again").
|
||||
- [x] **L68 — Background-running notification fires once per process** (`notifiedBackground` never
|
||||
resets) — won't remind on later window closes.
|
||||
- [ ] **L69 — `settings.ts` mixes path helpers with persistence** (`getDefaultMediaDir`/
|
||||
`ensureMediaDirs`/`getDownloadArchivePath` alongside the store) — split a `paths.ts`.
|
||||
- [x] **L69 — `settings.ts` mixes path helpers with persistence** (`getDefaultMediaDir`/
|
||||
`ensureMediaDirs`/`getDownloadArchivePath` alongside the store) — split a `paths.ts`. *Fixed: the three
|
||||
path helpers moved to [paths.ts](src/main/paths.ts); settings.ts is now just the electron-store persistence
|
||||
layer (its now-unused `path`/`fs` imports dropped). download.ts and index.ts import the helpers from
|
||||
`./paths`. `applyLaunchAtStartup` stays in settings.ts — it's OS login-item integration referenced by
|
||||
`applySettings`, not a path helper.*
|
||||
- [x] **L70 — `crypto.randomUUID` fallback is effectively unreachable** in Electron/Node 26
|
||||
(`typeof crypto !== 'undefined'` is always true) — dead defensive branch (see M7). *Fixed with M7: the
|
||||
branch now lives once in `newId` (kept so the fn is total in any host) and is covered by a test that stubs
|
||||
@@ -968,23 +1002,51 @@ root cause is already filed.
|
||||
subtitles, SponsorBlock), passed via `AddOptions.options` → `startDownload`. The panel also carries the
|
||||
M6 incognito checkbox and sits beside the M5 command preview. Overrides are one-shot: they reset to the
|
||||
global defaults after each download (matching Trim/Schedule).*
|
||||
- [ ] **UX2 — "Fetch" vs "Download" is ambiguous, and Enter does the wrong thing.** Two unlabeled
|
||||
- [x] **UX2 — "Fetch" vs "Download" is ambiguous, and Enter does the wrong thing.** Two unlabeled
|
||||
icon buttons (magnifier = "Fetch", clipboard = "Paste") sit next to "Download"; a first-timer can't
|
||||
tell that Download works without Fetch. Pressing **Enter** in the URL field runs a *probe*, not the
|
||||
download (L27) — violating "type URL + Enter = go". **Fix:** label the action, make Enter download
|
||||
(or fetch-then-download), and present Fetch as optional ("Preview formats").
|
||||
- [ ] **UX3 — Library "Index" is jargon and a hidden two-stage workflow.** Pasting a channel URL in
|
||||
*Fixed in [DownloadBar.tsx](src/renderer/src/components/DownloadBar.tsx): **Enter is now "go"** — it
|
||||
starts the download (or adds the fetched playlist when the playlist panel is open) instead of probing;
|
||||
probing stays an explicit, optional click. The magnifier button is relabeled from "Check URL" to
|
||||
**"Preview available formats (optional)"** (both its tooltip and `aria-label`), and the URL placeholder
|
||||
now reads "Paste a video or playlist URL, then press Enter to download" so it's clear Download works
|
||||
without Fetch. `download()` already enqueues with the presets when nothing has been probed, so the
|
||||
Enter-downloads path needs no probe. (resolves L27's Enter inconsistency too.)*
|
||||
- [x] **UX3 — Library "Index" is jargon and a hidden two-stage workflow.** Pasting a channel URL in
|
||||
the Downloads bar tries to treat it as one item; whole channels belong in the Library tab, but
|
||||
nothing signposts that. "Index" reads like "Download" but only catalogs — you must then expand,
|
||||
select, and Download. **Fix:** rename to "Add channel/playlist," and after indexing surface a clear
|
||||
"Download N videos" next step; detect channel URLs in the Downloads bar and suggest the Library.
|
||||
*Fixed across three seams. **(1) De-jargoned the Library** ([LibraryView.tsx](src/renderer/src/components/LibraryView.tsx)):
|
||||
the primary button is now **"Add"** (was "Index"), "Re-index" → **"Refresh"**, and the header
|
||||
description / empty state / placeholder / error copy drop "index" for "add". The post-add next step
|
||||
already exists — `indexSource` auto-selects the new source so its card expands with a prominent
|
||||
"Download N pending" primary button. **(2) Channel detection in the Downloads bar** — a new pure
|
||||
[`looksLikeChannelOrPlaylist`](src/renderer/src/lib/urlHelpers.ts) (conservative: only clear YouTube
|
||||
channel/dedicated-playlist shapes; a `/watch` video, even with a `list=`, is never flagged and keeps
|
||||
working in the bar) drives a dismissible brand-tinted nudge in
|
||||
[DownloadBar.tsx](src/renderer/src/components/DownloadBar.tsx): "This looks like a channel or playlist.
|
||||
Add it in the Library…" with an **Open in Library** action. **(3) The handoff** goes through a small
|
||||
new [store/nav.ts](src/renderer/src/store/nav.ts) (App's tab state moved into it) that carries a
|
||||
one-shot `pendingLibraryUrl`; the Library consumes it on mount to pre-fill its add field. Unit-tested
|
||||
(`looksLikeChannelOrPlaylist` in `test/clipboardLink.test.ts`) and verified end-to-end in the browser
|
||||
preview (channel URL → nudge → Open in Library → Library tab with the URL pre-filled).*
|
||||
- [x] **UX4 — Destructive actions have no confirmation and no undo** (L13). "Clear history,"
|
||||
"Clear log," "Remove source" (deletes an entire indexed channel + items), and "Delete selected"
|
||||
are all one click. A user exploring can wipe data irreversibly. **Fix:** confirm destructive/bulk
|
||||
deletes (or offer an undo toast).
|
||||
- [ ] **UX5 — First run can't choose the download folder.** [Onboarding.tsx](src/renderer/src/components/Onboarding.tsx)
|
||||
- [x] **UX5 — First run can't choose the download folder.** [Onboarding.tsx](src/renderer/src/components/Onboarding.tsx)
|
||||
only *describes* Documents\Video/Audio — there's no picker (the ROADMAP claims one). The user must
|
||||
later discover Settings → Downloads. **Fix:** put the folder picker in onboarding as documented.
|
||||
*Fixed: the "Where downloads go" block in [Onboarding.tsx](src/renderer/src/components/Onboarding.tsx)
|
||||
now has interactive Video and Audio rows — each shows the current folder (or the `Documents\Video` /
|
||||
`Documents\Audio` default) with a **Choose…** button that opens the OS picker via the settings store's
|
||||
`chooseDir` (the same proven path DownloadsCard uses, seeded at the current value — W5). Picking a
|
||||
folder persists immediately, so the choice is already saved when the user clicks "Get started";
|
||||
leaving it untouched keeps the Documents defaults. Verified in the browser preview with onboarding
|
||||
forced on.*
|
||||
|
||||
### Medium — confusion, feedback & organization
|
||||
|
||||
@@ -1258,10 +1320,18 @@ filed. Several listed categories are **N/A** to this stack and worth recording s
|
||||
`'path|'` are hard-coded in `buildArgs.ts` (`PROGRESS_TEMPLATE` / `--print after_move:path|…`) and again
|
||||
in `download.ts` (`line.startsWith('prog|')` / `'path|'`). Change one and the other breaks silently.
|
||||
**Fix:** export shared marker constants from one module.
|
||||
- [ ] **CL2 — Long positional parameter list.** `buildArgs(opts, outputTemplate, o, binDir, access,
|
||||
extraArgs)` takes six positional args (and `o` vs `opts` is easy to swap). **Fix:** pass a single options object.
|
||||
- [ ] **CL3 — Large method: `startDownload`** (~130 lines) bundles spawn + dual metadata path + four inline
|
||||
`child` event handlers. **Fix:** extract the stdout-parse + close/error wiring (pairs with CC3's spawn-stream helper).
|
||||
- [x] **CL2 — Long positional parameter list.** `buildArgs(opts, outputTemplate, o, binDir, access,
|
||||
extraArgs)` takes six positional args (and `o` vs `opts` is easy to swap). **Fix:** pass a single options
|
||||
object. *Fixed: `buildArgs(input: BuildArgsInput)` now takes one named object (`opts`/`outputTemplate`/
|
||||
`options`/`binDir`/`access`/`extraArgs?`) destructured at the top, so callers can't transpose the two
|
||||
path-like strings or `opts`/`options`. download.ts's `buildCommand` and both test harnesses updated.*
|
||||
- [x] **CL3 — Large method: `startDownload`** (~130 lines) bundles spawn + dual metadata path + four inline
|
||||
`child` event handlers. **Fix:** extract the stdout-parse + close/error wiring (pairs with CC3's spawn-stream
|
||||
helper). *Fixed: the stdout/stderr parse, the close/error handlers, and the B1 idle watchdog moved into a
|
||||
`wireChildProcess({ wc, opts, rec, cleanup, getTitle })` helper in [download.ts](src/main/download.ts).
|
||||
`startDownload` is now a linear pre-flight (binary checks → URL normalise → concurrency guard → cookies →
|
||||
spawn → metadata probe → wire). `getTitle` is a getter because the parallel metadata probe may fill the
|
||||
resolved title in after wiring is set up. Behaviour unchanged.*
|
||||
- [ ] **CL4 — Deep nesting: `updater.downloadAppUpdate`** — Promise → `net.request` → `response` → `data`
|
||||
with nested conditionals/teardown is the hardest-to-follow block. **Fix:** extract a `streamToFile` helper.
|
||||
- [x] **CL5 — Superfluous exports.** `PROGRESS_TEMPLATE` (L167) and `getManagedBinDir` are `export`ed but
|
||||
|
||||
Reference in New Issue
Block a user