From 8bec8d2466f04cfbdc042cfe46dadf8e86f91879 Mon Sep 17 00:00:00 2001 From: Byron Gamatos Date: Sat, 11 Jul 2026 23:26:35 +0200 Subject: [PATCH] =?UTF-8?q?refactor(app):=20carve=20the=20playback=20trans?= =?UTF-8?q?port=20out=20of=20app.js=20=E2=80=94=20and=20RETIRE=208=20host?= =?UTF-8?q?=20hooks=20(R3a)=20(#894)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit static/js/transport.js (377) — bodies VERBATIM. app.js 6,643 → 6,316. THIS IS THE FIRST CARVE THAT SUBTRACTS HOOKS INSTEAD OF ADDING THEM. Every carve before this one added host hooks: a module pulled out of app.js still had to call back into it. But four modules were all reaching through the seam for the SAME handful of names — _audioSeek, _audioTime, setPlayButtonState, _songEventPayload, jucePlayer. Those names have an owner, and it isn't app.js. Give them one, and the consumers import them directly: count-in.js 5 hooks -> 0 (host import deleted) juce-audio.js 4 hooks -> 0 (host import deleted) loops.js 6 hooks -> 4 section-practice.js 10 hooks -> 7 ---------------------------------------------------------- configureHost() 20 hooks -> 12 A hook is a cycle you agreed to live with. An import is a dependency you actually have. Prefer the import whenever the name has a real owner. _audioSeekGen now stays PRIVATE. It has exactly one writer — _resetAudioSeekState(), which moved with it — so readers get audioSeekGen() and nobody outside can desync it. Strictly better than the hook it replaces, which handed out a getter and left the writer behind in app.js. THE SCAN HAD A HOLE, AND IT BIT. Picking the carve by dependency closure over app.js's own top-level decls said this cluster was downward-closed. It wasn't: _currentPlaybackSnapshot reads loopA/loopB — which live in ./js/loops.js, and loops.js imports transport. The scan saw nothing, because loopA STOPPED BEING an app.js decl the moment loops.js was carved out. Any dependency scan of a partly-carved monolith has to resolve the imports too, or it will confidently hand you a cycle. Added that pass; it found exactly one back-edge, and _currentPlaybackSnapshot stays in app.js (as does restartCurrentSong, which calls _cancelCountIn). app.js is the root — it imports both sides for free. TESTS. Four harnesses retargeted (play_button_reroute_guard, song_event_payload, song_seek -> transport.js; playback_app_adapter SPLIT, since _installPlaybackTransportAdapter stayed behind). The two CENSUS tests — "≥8 song:* emit sites", "every seek callsite passes a reason" — now scan app.js AND every static/js/*.js, not one file. Pointed at a single file, their count silently shrinks as code leaves, which reads as "someone deleted an emit" or, worse, passes while genuinely missing sites. Both bite-tested: stripping a _songEventPayload() from an emit and adding a reason-less _audioSeek() each fail the suite. VERIFIED. A/B against origin/main, real song, real playback: song:play payload is exactly {audioT, chartT, perfNow, time}; song:seek carries reason "seek-by" with finite from/to; all five song:* events fire; seekBy advances the clock; restartCurrentSong returns to zero; the play button's aria-pressed tracks state. IDENTICAL on all 21 probes, zero page errors. pytest 2396, node 1040/1040, host contract 2/2, ESLint 0 (no-cycle clean), Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) --- static/app.js | 353 +------------------ static/js/count-in.js | 26 +- static/js/juce-audio.js | 42 +-- static/js/loops.js | 9 +- static/js/section-practice.js | 23 +- static/js/transport.js | 377 +++++++++++++++++++++ tests/js/play_button_reroute_guard.test.js | 2 +- tests/js/playback_app_adapter.test.js | 8 +- tests/js/song_event_payload.test.js | 17 +- tests/js/song_seek.test.js | 19 +- 10 files changed, 478 insertions(+), 398 deletions(-) create mode 100644 static/js/transport.js diff --git a/static/app.js b/static/app.js index 2cbecba..bda30e3 100644 --- a/static/app.js +++ b/static/app.js @@ -127,35 +127,18 @@ import { toggleSectionPracticePopover, } from './js/section-practice.js'; import { configureHost } from './js/host.js'; +// The playback transport. These used to BE app.js — they are imported back now, and the +// four modules that reached for them through the host seam import them directly instead. +import { + setPlayButtonState, jucePlayer, _audioTime, _audioDuration, _songEventPayload, + _markPlaybackPaused, _markPlaybackResumed, _emitPlaybackStopped, _emitSongPositionChanged, + _waitForSongReady, _resetAudioSeekState, _audioSeek, togglePlay, seekBy, audioSeekGen, +} from './js/transport.js'; // Demo analytics — real impl set by demo.js; no-op in normal builds window.feedBackDemoTrack = window.feedBackDemoTrack ?? null; -// Sync the play/pause button's icon and accessible state in one place so -// screen readers, tooltips, and aria-pressed stay aligned with playback. -// Updates the existing child's src in place rather than rewriting -// innerHTML, so any future children (fallback label, loading spinner, …) -// survive state changes. -function setPlayButtonState(isPlaying) { - const btn = document.getElementById('btn-play'); - if (!btn) return; - const label = isPlaying ? 'Pause' : 'Play'; - const icon = isPlaying ? 'pause' : 'play'; - let img = btn.querySelector('img.button-icon-svg'); - if (!img) { - img = document.createElement('img'); - img.className = 'button-icon-svg'; - img.alt = ''; - img.setAttribute('aria-hidden', 'true'); - btn.appendChild(img); - } - img.src = `/static/svg/${icon}.svg`; - btn.setAttribute('aria-label', label); - btn.setAttribute('aria-pressed', isPlaying ? 'true' : 'false'); - btn.title = label; -} - // ── Global keyboard shortcuts ───────────────────────────────────────────── // // `/` focuses the active screen's search input (Library / Favorites); @@ -3502,20 +3485,6 @@ function retuneSong(filename, title, tuning, target) { }; } -// ── Player ─────────────────────────────────────────────────────────────── -// `audio` now lives in ./js/audio-el.js so carved-out modules can reach the -// player without importing app.js back (which would close a cycle). Same -// element, same handle, same lookup — just imported instead of declared here. -let _lastSongPositionEventAt = 0; - -function _emitSongPositionChanged(time, duration) { - const now = Date.now(); - if (now - _lastSongPositionEventAt < 250) return; - _lastSongPositionEventAt = now; - const payload = (typeof _songEventPayload === 'function') ? _songEventPayload() : { time }; - window.feedBack.emit('song:position-changed', Object.assign(payload, { duration })); -} - function _applyPreservePitch(el) { if (!el) return; if ('preservesPitch' in el) el.preservesPitch = true; @@ -3529,94 +3498,6 @@ _applyPreservePitch(audio); // through the JUCE backing track player instead of the HTML5