Altinity / Altinity/altinity-sql-browser

Split dashboard-viewer-session.ts: a 1857-line file whose 1354-line closure holds every concern

Aperta
#453 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

enhancement
Lingua principale
TypeScript
Stelle
8
Fork
2
Merge medio
1h 34m
PR unite (30g)
6

Descrizione

Found while planning #447 phase 1. Deferred: doing it now would relocate code that #447 phase 1 deletes.

What

src/dashboard/application/dashboard-viewer-session.ts is 1857 lines, of which
1354 are a single factory closurecreateDashboardViewerSession starts at line 503 and runs
to the end of the file. tests/unit/dashboard-viewer-session.test.ts is 3441 lines.

The closure holds ~65 members with no internal boundary: tile runtime construction, the parameter
analysis, target resolution, the option-source/provider wave engine, published-state assembly, the
execution pool, the commit API, and the session lifecycle all capture the same scope.

That has three concrete costs:

  • Review surface. Any change to one concern reads as a change to the whole session.
  • The 90% branch floor (tests/vitest.config.ts, perFile: true, 100/95/90/100) is enforced
    over the whole 1857-line file, so a defensive branch in one concern can only be covered through
    the public session API, via a fixture that sets up every other concern too. That is a large part
    of why the spec is 3441 lines.
  • Closure capture instead of parameters. Members read tiles, filters, analysis,
    documentRef and friends directly from scope, so there is no stated contract between concerns.

Why after #447, not before

#447 phase 1 removes the curated-filter/provider infrastructure, which accounts for roughly
500 lines (~27%) of this file:

Region Lines What
FilterSourceRuntime 405-424 provider record
filterInitialActive / filterDefaultValue 458-468 persisted-default readers
sourceBackedParams, filterSources 610-640 provider index
curatedControls, staticFilterDiagnostics, mergeControls 730-807 provider/control merge
optionsSignaturerunFilterSourceWave 1105-1466 the whole option-source wave engine

Splitting first would mean carefully extracting code that is about to be deleted, and would put two
large diffs on the same file back to back. After phase 1 the file is ~1350 lines with a clearer set
of responsibilities, and the extraction boundaries below fall out of what remains.

Proposed split (post-#447)

File Contents
dashboard-viewer-session.ts public facade + lifecycle: start/refresh/refreshTile/syncDocument/destroy/setTileSearch/cancelTile/setGridRenderMode/setDashboardStyle/preflight
viewer-tile-runtime.ts TileRuntime, buildTileRuntime, runTile, tileResultMeta, markTextAndErrorTiles, runnableTiles, isRunnableTileRuntime, runPool, supersede
viewer-variable-runtime.ts variable runtime records, rawValues/activeMap/committedRootValues/prepareBatch, the commit API (setFilter/applyFilter/applyFilters/clearFilter/clearAllFilters/resetFilters), commitAndRerun/runAffectedWave/reserveAffected
viewer-state.ts ViewerTileState/ViewerFilterState/DashboardViewState + buildState/publish
dashboard-viewer-session.types.ts DashboardViewerDeps, ViewerExecutor, ViewerConnection, ViewerReadRequest (type-only seam contract, per ADR-0002 phase 0)

tests/unit/dashboard-viewer-session.test.ts splits alongside, one spec per new module.

Acceptance

  • No behaviour change; the existing session public API is byte-identical to callers
    (src/ui/dashboard.ts is the only consumer).
  • Each new file independently meets 100/95/90/100. This is the real work — extracted
    collaborators must take explicit inputs rather than capture closure scope, and each one must be
    reachable from its own spec without standing up the whole session.
  • npm run check:arch still passes: src/dashboard/application may not import src/ui,
    src/editor, src/application, src/state.ts, src/net, src/dashboard/ui
    (build/check-boundaries.mjs), and the new siblings inherit that rule.
  • No new runtime dependency; no seam changes.

Non-goals

  • Changing the session's public API or the published DashboardViewState shape.
  • Splitting src/ui/dashboard.ts (2641 lines) — related, but a separate call.

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Attendi la fase 1 di #447, quindi leggi src/dashboard/application/dashboard-viewer-session.ts e tests/unit/dashboard-viewer-session.test.ts insieme ai confini proposti per runtime e stato. Inizia eseguendo gli unit test esistenti e verificando le impostazioni della coverage in tests/vitest.config.ts. Il lavoro è completato quando i moduli sono suddivisi con input espliciti, il comportamento e le API pubbliche sono invariati, ogni nuovo file soddisfa la coverage e npm run check:arch passa.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
typescript
Ambito
frontend, testing-qa
Tipo di issue
Refactoring
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
42/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.