Skip to content

Settings split pass 6: hoist import-history logic into a hook (SettingsView 8.2k) - #152

Merged
AdamXweb merged 1 commit into
mainfrom
refactor/settings-import-history
Aug 3, 2026
Merged

Settings split pass 6: hoist import-history logic into a hook (SettingsView 8.2k)#152
AdamXweb merged 1 commit into
mainfrom
refactor/settings-import-history

Conversation

@adamXbot

@adamXbot adamXbot commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Pass 6 — Import History, stage 1 (logic)

SettingsView.tsx 9,404 → 8,215 (−1,189), with zero JSX changes.

Base note: this branches on top of #151 (pass 5), which hasn't merged yet, so its two commits appear here until it does. The PR targets main directly — that is the thing that went wrong in #150, and basing on main means merging this lands everything in main regardless of which PR goes first. Merge #151 first if you want the smaller diff.

Why this is one commit and not the whole section

Import History is the largest remaining section: ~1,100 lines of logic plus 1,299 lines of JSX. Unlike the activity log, its logic isn't contiguous — the AI debug-log fetchers and the entire backup/restore flow sit inside the range, so this lifts four separate ranges rather than one. Splitting logic from markup keeps each half reviewable; the JSX move is the next pass.

What made a wholesale move safe

Of the ~50 component-scope bindings the section touches, exactly five cross the boundary: the import-queue provider, the router/searchParams pair used for deep-link handling, and showToast with its translator. Those become the hook's inputs. Everything else was read only by Import History.

Getting that number right took care — a naive identifier sweep reports 49 "leaks," but error, status, t, s and has are matching catch (error), res.status, and words inside comments. Checking actual line numbers showed nearly every hit was the declaration itself or the handler definition.

Two judgment calls:

  • deleteTarget is returned rather than kept private. The delete-confirm modal renders as a page-level overlay at the bottom of SettingsView, not inside the section. Same feature, different place in the tree — returning one binding beats relocating a modal and changing DOM order.
  • The countdown ticker moved into the hook. It lived a thousand lines from the rest of the subsystem, but it exists only to re-render queued-row countdowns. Moving it beat re-exporting setNowTick so an outside effect could drive it.

Verification

  • JSX untouched — all eight diff hunks land above the first return ( at line 3702, so this reviews as "did the logic move intact?" without a markup diff.
  • typecheck, lint, 441 unit tests, full Playwright suite 45 passed including the settings structural net.

Two things a reviewer should look at

  • The hook call sits just after showToast, not where the state block used to be. showToast is a const arrow, so calling the hook at the old position was a TDZ error (tsc caught it). Moving the call is the honest fix — a lazy (msg) => showToast(msg) wrapper would have hidden the ordering dependency instead of stating it.
  • The first cut returned all 51 bindings mechanically; 13 were dead at the call site and are now private to the hook. Worth a glance that nothing wanted was dropped — setNowTick was in that batch, which is how the stranded ticker got noticed.

Next

Stage 2 moves the 1,299-line JSX into ImportHistorySection.tsx, verified with the same before/after HTML diff used in #151.

🤖 Generated with Claude Code

Import History was ~1,100 lines of state, effects and handlers living
directly in SettingsView, interleaved with unrelated code: the AI
debug-log fetchers and the whole backup/restore flow sat in the middle of
it. Moving it out means lifting four separate ranges, not one.

The dependency analysis is what made this safe to do wholesale. Of the
~50 component-scope bindings the section touches, exactly five cross the
boundary — the import-queue provider, the router/searchParams pair used
for deep links, and showToast with its translator. Those five become the
hook's inputs; everything else was read only by Import History.

`deleteTarget` is returned rather than kept private because the
delete-confirm modal renders as a page-level overlay at the bottom of
SettingsView rather than inside the section. Same feature, different
place in the tree — worth returning one binding instead of relocating a
modal and changing DOM order.

The queued-row countdown ticker moved too. It lived a thousand lines away
from the rest of the subsystem but only exists to re-render queue
countdowns, so it went into the hook rather than re-exporting
`setNowTick` to drive it from outside.

As with the activity log, the JSX is untouched: hook fields are renamed
back on destructure, and all eight diff hunks land above the first
`return (` at line 3702. The 1,299-line markup move is the next pass.

Two things worth noting for review:

- The hook call sits just after `showToast` rather than where the state
  block used to be. `showToast` is a const arrow, so calling the hook at
  the old position was a TDZ error. Moving the call is the honest fix; a
  lazy `(msg) => showToast(msg)` wrapper would have hidden it.
- The first cut returned all 51 bindings mechanically. Thirteen were dead
  at the call site, so they are private to the hook now.

SettingsView 9,404 -> 8,215.

Verified: typecheck, lint, 441 unit tests, full Playwright suite 45
passed including the settings structural net.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@adamXbot
adamXbot requested a review from AdamXweb as a code owner August 2, 2026 08:02
@AdamXweb
AdamXweb merged commit 458ae47 into main Aug 3, 2026
13 checks passed
@AdamXweb
AdamXweb deleted the refactor/settings-import-history branch August 3, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants