Altinity / Altinity/altinity-sql-browser
Split dashboard-viewer-session.ts: a 1857-line file whose 1354-line closure holds every concern
Nessuno ha ancora preso questa issue.
- 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 closure — createDashboardViewerSession 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,
documentRefand 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 |
optionsSignature … runFilterSourceWave |
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.tsis 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:archstill passes:src/dashboard/applicationmay not importsrc/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
DashboardViewStateshape. - Splitting
src/ui/dashboard.ts(2641 lines) — related, but a separate call.
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- 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