Altinity / Altinity/altinity-sql-browser
Split dashboard-viewer-session.ts: a 1857-line file whose 1354-line closure holds every concern
Dieses Issue hat noch niemand übernommen.
- Vorherrschende Sprache
- TypeScript
- Sterne
- 8
- Forks
- 2
- Ø Merge
- 1 Std. 34 Min.
- Gemergte PRs (30 T.)
- 6
Beschreibung
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.
Beitragsleitfaden
Erste Schritte
- Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
- Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
- Forke das Repository und arbeite in einem Branch.
- Öffne einen Pull Request, der die Issue-Nummer nennt.
Rechercherichtung
Warte auf Phase 1 von #447 und lies dann src/dashboard/application/dashboard-viewer-session.ts und tests/unit/dashboard-viewer-session.test.ts zusammen mit den vorgeschlagenen Runtime- und State-Grenzen. Führe zunächst die vorhandenen Unit-Tests aus und prüfe die Coverage-Einstellungen in tests/vitest.config.ts. Abgeschlossen ist die Aufgabe, wenn die Module mit expliziten Eingaben aufgeteilt sind, Verhalten und öffentliche APIs unverändert sind, jede neue Datei die Coverage-Anforderungen erfüllt und npm run check:arch erfolgreich ist.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- typescript
- Bereich
- frontend, testing-qa
- Issue-Typ
- Refactoring
- Schwierigkeit
- 5/5
- Geschätzter Aufwand
- Über eine Woche
- Aktivitätsstatus
- Ruhig
- Klarheit
- Klar beschrieben
- Anfängerfreundlichkeit
- 42/100