From eba258e61199c69910b56d6da2f2b4cc35c141f0 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Sun, 12 Jul 2026 11:49:18 +0200 Subject: [PATCH] refactor(highway): make the `highway` global explicit before the module flip (R3c) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 73 bare `highway.x` references -> `window.highway.x`, across app.js and 10 other files. Provably a NO-OP today. It is the precondition for flipping highway.js to a module. ━━━ WHY THIS HAS TO LAND FIRST ━━━ highway.js is a CLASSIC script. Its top-level `const highway = createHighway()` therefore creates a GLOBAL LEXICAL BINDING — visible as a bare name to every other classic script AND to every ES module. 73 call sites quietly rely on that. The moment highway.js becomes a module, that binding is gone. `const` in a module is module-scoped, not global. Every one of those 73 sites becomes a ReferenceError, and the flip is impossible until they say what they mean. `window.highway = highway` is already set, to the same object, on the same line. So this is an identity rewrite — verified in the browser below. ━━━ THE REWRITE BIT ME THREE TIMES. REGEX IS NOT ENOUGH FOR THIS. ━━━ 1. A SHADOWED LOCAL. capabilities/note-detection.js does `const highway = window.highway`. Its 9 bare uses are LOCAL and already correct; a blind rewrite would have emitted `const window.highway = window.highway`. Excluded. 2. HALF-CONVERTED GUARDS — the dangerous one. Six sites read `typeof highway !== 'undefined' && highway && typeof highway.setTime === 'function'`. The regex converted the CONSEQUENT and left the TEST, which is WORSE than not touching them: after the flip `typeof highway` is 'undefined', so each guard is PERMANENTLY FALSE and the code behind it silently never runs. transport.js's was the seek->setTime sync: the chart clock would have quietly desynced after every seek, with nothing failing. All six now test window.highway. 3. TWO MORE BARE REFERENCES, found by Codex [P2] and confirmed by an AST scan: app.js:3114 and :3176 use `highway && typeof window.highway.getSections === 'function'`. My grep searched for `typeof highway`, not `highway &&`. After the flip these throw, the catch swallows it, and the editor silently falls back to a ±4s edit window and arrangement 0. Regex missed a shadow, a half-conversion, and two bare reads. The final check is an AST pass that resolves scopes and reports every `highway` identifier not bound locally. It now reports ZERO. VERIFIED. A/B against origin/main in two browsers, 15 probes, IDENTICAL, zero page errors: window.highway is the same object as the bare global, the whole API surface resolves, a real song plays, the chart clock advances, getPerf().drawMs > 0 — and `seek syncs chart` passes, which is the exact guard I nearly broke in (2). node 1045, pytest 2416, ESLint 0, Codex 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- static/app.js | 40 ++++++++++++++--------------- static/js/count-in.js | 14 +++++----- static/js/highway-colors.js | 2 +- static/js/juce-audio.js | 4 +-- static/js/player-controls.js | 4 +-- static/js/resume-session.js | 6 ++--- static/js/section-practice.js | 4 +-- static/js/transport.js | 6 ++--- static/js/viz.js | 38 +++++++++++++-------------- static/v3/player-chrome.js | 14 +++++----- static/v3/venue-scene-3d.js | 2 +- tests/js/loop_restart.test.js | 5 ++++ tests/js/song_event_payload.test.js | 10 ++++++++ tests/js/speed_reset.test.js | 5 ++++ 14 files changed, 87 insertions(+), 67 deletions(-) diff --git a/static/app.js b/static/app.js index c5a016ec..9350924a 100644 --- a/static/app.js +++ b/static/app.js @@ -860,10 +860,10 @@ async function showScreen(id) { const hadPlayableSong = !!audio.src || !!window._juceAudioUrl || S.isPlaying; // Snapshot where we were so leaving the player — especially by accident // — is recoverable instead of dumping the user back at bar 1 next time. - // Must run BEFORE highway.stop()/audio unload, while getSongInfo() and + // Must run BEFORE window.highway.stop()/audio unload, while getSongInfo() and // the position (stopTime) are still live. if (hadPlayableSong) _snapshotResumeSession(stopTime); - highway.stop(); + window.highway.stop(); // Cancel any queued seeks, in-flight shim closures, AND active // count-in timers before stopping playback so none of these paths // can mutate the torn-down session (mirrors the same triple reset @@ -1049,7 +1049,7 @@ async function loadSettings() { const demucsEl = document.getElementById('demucs-server-url'); if (demucsEl) demucsEl.value = data.demucs_server_url || ''; const leftyEl = document.getElementById('setting-lefty'); - if (leftyEl) leftyEl.checked = highway.getLefty(); + if (leftyEl) leftyEl.checked = window.highway.getLefty(); const autoplayExitEl = document.getElementById('setting-autoplay-exit'); if (autoplayExitEl) autoplayExitEl.checked = _autoplayExitEnabled(); const showUpNextEl = document.getElementById('setting-show-upnext'); @@ -1435,7 +1435,7 @@ function setAvOffsetMs(ms, skipPersist) { // the audio-aligned chart time so plugins (note detection, etc.) // keep scoring against the real chart clock regardless of visual // calibration. - if (typeof highway !== 'undefined' && highway?.setAvOffset) highway.setAvOffset(_avOffsetMs); + if (window.highway?.setAvOffset) window.highway.setAvOffset(_avOffsetMs); // Sync any visible Settings slider const avSlider = document.getElementById('setting-av-offset'); if (avSlider) { @@ -1771,7 +1771,7 @@ window._juceAudioUrl = null; window.jucePlayer = jucePlayer; // ── Engine start/stop → re-route song audio (HTML5 ⇄ JUCE) ────────────────── -// window._juceMode is otherwise decided once, at song-load time (highway.js), +// window._juceMode is otherwise decided once, at song-load time (window.highway.js), // from isAudioRunning(). If the JUCE audio engine is started or stopped *after* // a song is already loaded (e.g. the user presses CHAIN / AMP), that decision // goes stale: the song stays on the HTML5