refactor(highway): make the highway global explicit before the module flip (R3c) (#912)

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) <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos
2026-07-12 11:56:07 +02:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 23ecddc721
commit d9fa6d3f55
14 changed files with 87 additions and 67 deletions
+7 -7
View File
@@ -60,7 +60,7 @@ const _CREDITS_HOLD_MS = 3000;
// Backstop: the overlay's primary dismiss is song:play, but playback can fail
// to start without emitting it (HTML5 autoplay rejection, JUCE start failure,
// a count-in handoff that never plays). This hard cap guarantees the credits
// never linger over the highway. Generous enough to outlast a normal count-in.
// never linger over the window.highway. Generous enough to outlast a normal count-in.
const _CREDITS_MAX_MS = 12000;
export function _cancelCountIn() {
_countInGen++;
@@ -107,7 +107,7 @@ function _creditLineLabel(role) {
return key.charAt(0).toUpperCase() + key.slice(1) + ' by';
}
// Show the feedpak contributor credits over the highway. `authors` is the
// Show the feedpak contributor credits over the window.highway. `authors` is the
// sanitized [{name, role}] list from window.feedBack.currentSong.authors.
// Anchored to the lower third (bottom-center) so it never collides with the
// vertically-centered count-in number, and pointer-events-none so it never
@@ -196,7 +196,7 @@ export async function startCountIn(opts = {}) {
return;
}
S.lastAudioTime = loopA;
highway.setTime(loopA);
window.highway.setTime(loopA);
if (window.feedBack) {
window.feedBack.emit('loop:restart', { loopA, loopB, time: loopA });
}
@@ -217,7 +217,7 @@ export async function startCountIn(opts = {}) {
// Ease out quad
const eased = 1 - (1 - t) * (1 - t);
const currentT = fromTime + (toTime - fromTime) * eased;
highway.setTime(currentT);
window.highway.setTime(currentT);
if (t < 1) {
_countInRaf = requestAnimationFrame(rewindStep);
} else {
@@ -262,7 +262,7 @@ export async function startCountIn(opts = {}) {
// marker for "new iteration starts at A", not the actual
// audio position.
S.lastAudioTime = r.to;
highway.setTime(r.to);
window.highway.setTime(r.to);
window.feedBack.emit('loop:restart', { loopA, loopB, time: loopA });
beginCount();
});
@@ -271,7 +271,7 @@ export async function startCountIn(opts = {}) {
_countInRaf = requestAnimationFrame(rewindStep);
function beginCount() {
const bpm = highway.getBPM(loopA);
const bpm = window.highway.getBPM(loopA);
const beatInterval = 60 / bpm;
let count = 0;
@@ -339,7 +339,7 @@ export async function startSongCountIn() {
}
if (gen !== _countInGen) return; // teardown during pause
const startT = S.lastAudioTime || 0;
let bpm = highway.getBPM(startT);
let bpm = window.highway.getBPM(startT);
// Pre-chart / malformed-tempo fallback: 4 beats at 120 BPM (500 ms each).
if (!Number.isFinite(bpm) || bpm <= 0) bpm = 120;
const beatInterval = 60 / bpm;
+1 -1
View File
@@ -155,7 +155,7 @@ function _hwcSlotKeysForChart(sc, isBass) {
return ['low8', 'low7', 'lowE', 'A', 'D', 'G', 'B', 'highE'];
}
// Current arrangement shape (string count + bass-vs-guitar) from the 2D highway.
// Current arrangement shape (string count + bass-vs-guitar) from the 2D window.highway.
function _hwcChartShape() {
let sc = 6, arr = '';
try { sc = window.highway?.getStringCount?.() || 6; } catch (_) {}
+2 -2
View File
@@ -111,7 +111,7 @@ import { S } from './player-state.js';
return false;
}
}
// highway.js's initial song-load routing consults this for the same
// window.highway.js's initial song-load routing consults this for the same
// feedpak-under-exclusive decision the watcher makes below.
window._juceOutputIsExclusive = _outputIsExclusive;
// Returns true when window._currentSongAudio no longer references the exact
@@ -383,7 +383,7 @@ import { S } from './player-state.js';
// the stem mixer / WebAudio path keeps working. Sloppak stem URLs
// are never routable (per-stem mix can't ride a single transport).
if (!songAudio || (!songAudio.juceEligible && !songAudio.feedpakFullMix)) return;
// Don't race highway.js's own initial song-load routing: it owns
// Don't race window.highway.js's own initial song-load routing: it owns
// _juceMode until _juceRoutingPromise settles. Re-running our switch
// concurrently would double-call loadBackingTrack for the same URL.
if (window._highwayJuceRoutingPending) return;
+2 -2
View File
@@ -160,7 +160,7 @@ export function _resetPlaybackSpeedForNewSong() {
//
// Debounced trailing-edge (300ms) so dragging the slider — which fires
// oninput per pixel — doesn't flood the server with concurrent writes
// to config.json. highway.setMastery() still fires every oninput so
// to config.json. window.highway.setMastery() still fires every oninput so
// the chart re-filters in real time; only disk persistence waits.
let _masteryPersistTimer = null;
function _persistMastery(pct) {
@@ -209,7 +209,7 @@ export function _applyMastery(v, opts = {}) {
// unlike #mastery-label above, whose markup carries no trailing unit.
const setLabel = document.getElementById('setting-highway-speed-val');
if (setLabel) setLabel.textContent = pct;
highway.setMastery(pct / 100);
window.highway.setMastery(pct / 100);
if (!opts.skipPersist) _persistMastery(pct);
}
// Reflect phrase-data availability on the slider after every `ready`.
+3 -3
View File
@@ -31,12 +31,12 @@ const _RESUME_END_GUARD_S = 5; // ignore basically-finished
let _resumePillDismissed = false; // per-session: user waved off the current snapshot
// Snapshot the live session. Called from showScreen()'s teardown before
// highway.stop()/audio unload, while getSongInfo() + position are still valid.
// window.highway.stop()/audio unload, while getSongInfo() + position are still valid.
export function _snapshotResumeSession(position) {
try {
if (!host.currentFilename()) return;
const si = (window.highway && typeof highway.getSongInfo === 'function')
? (highway.getSongInfo() || {}) : {};
const si = (window.highway && typeof window.highway.getSongInfo === 'function')
? (window.highway.getSongInfo() || {}) : {};
const dur = Number(si.duration) || 0;
const pos = Number(position) || 0;
// Only worth resuming a song you were genuinely mid-way through — not a
+2 -2
View File
@@ -38,7 +38,7 @@ export function _sectionPracticeBarContains(el) {
}
// ── Section Practice Bar ────────────────────────────────────────────────
// One-click looping over song section markers (highway.getSections —
// One-click looping over song section markers (window.highway.getSections —
// same array as 3D highway bundle.sections / "Now / Up Next").
// Reuses setLoop() so manual A/B controls and saved loops stay canonical.
let _sectionPracticeRanges = [];
@@ -127,7 +127,7 @@ export function _resetSectionPracticeLog() {
}
function _sectionPracticeHighway() {
return window.highway || (typeof highway !== 'undefined' ? highway : null);
return window.highway || null;
}
function _sectionPracticeDuration() {
+3 -3
View File
@@ -172,7 +172,7 @@ export function _songEventPayload() {
return {
time: audioT,
audioT,
chartT: highway.getTime(),
chartT: window.highway.getTime(),
perfNow: performance.now(),
};
}
@@ -289,8 +289,8 @@ export async function _audioSeek(s, reason) {
// _audioSeek resolves (e.g. the auto-resume song:play in
// changeArrangement) sees an in-sync chartT via _songEventPayload.
// Without this, chartT lags by one 60Hz tick after a seek.
if (typeof highway !== 'undefined' && highway && typeof highway.setTime === 'function') {
highway.setTime(to);
if (window.highway && typeof window.highway.setTime === 'function') {
window.highway.setTime(to);
}
window.feedBack.emit('song:seek', { from, to, reason: reason || null });
return { completed: true, from, to };
+19 -19
View File
@@ -62,7 +62,7 @@ function _hasPromotedFlag() {
// Pending nag: queued during _populateVizPicker, fired on the first
// `song:ready` (so the toast lands when the user actually opens the
// player, not at page load when they're still in the library).
// `song:ready` is emitted by highway.js via window.feedBack.emit(), so
// `song:ready` is emitted by window.highway.js via window.feedBack.emit(), so
// subscribe through the same EventTarget. window.feedBack is created in
// this same file before _populateVizPicker is reachable, so the global
// is guaranteed to exist by the time this listener registers — but guard
@@ -326,7 +326,7 @@ export async function _populateVizPicker(plugins) {
// plugin options — _autoMatchViz saw no candidates and left the
// default active. Now that plugins are registered, re-evaluate
// against whatever song is currently loaded (a no-op when no song
// has been loaded yet, since highway.getSongInfo() returns {}).
// has been loaded yet, since window.highway.getSongInfo() returns {}).
if (sel.value === 'auto') _autoMatchViz();
}
@@ -356,7 +356,7 @@ function _noteVizAutoMatch(id, matched) {
}
function _installVizRenderer(renderer, id, source = 'user-select') {
highway.setRenderer(_tagVizRenderer(renderer, id));
window.highway.setRenderer(_tagVizRenderer(renderer, id));
// Drop any stale notation-view hint now that we have a resolved renderer id.
// This is also the path used by _autoMatchViz() after it resolves 'auto' to
// a real plugin id, so the null passed at evaluation start is corrected here.
@@ -377,7 +377,7 @@ export function setViz(id) {
try { localStorage.setItem('vizSelection', 'default'); } catch (_) {}
const sel = document.getElementById('viz-picker');
if (sel) sel.value = 'default';
highway.setRenderer(null);
window.highway.setRenderer(null);
_syncVenueVizPlayerClass('default');
if (window.v3VenueScene3d && typeof window.v3VenueScene3d.syncViz === 'function') {
window.v3VenueScene3d.syncViz('default');
@@ -399,7 +399,7 @@ export function setViz(id) {
try { localStorage.setItem('vizSelection', id || 'default'); } catch (_) {}
const _sel = document.getElementById('viz-picker');
if (_sel) _sel.value = 'default';
highway.setRenderer(null);
window.highway.setRenderer(null);
_syncVenueVizPlayerClass('default');
if (window.v3VenueScene3d && typeof window.v3VenueScene3d.syncViz === 'function') {
window.v3VenueScene3d.syncViz('default');
@@ -483,7 +483,7 @@ export function setViz(id) {
fallbackToDefault();
return;
}
// Validate shape — highway.setRenderer will itself fall back to
// Validate shape — window.highway.setRenderer will itself fall back to
// default on a bad renderer, but without this check the UI and
// localStorage would still advertise the broken selection.
if (!renderer || typeof renderer.draw !== 'function') {
@@ -503,7 +503,7 @@ export function setViz(id) {
// Auto mode: evaluate each registered viz factory's static
// `matchesArrangement(songInfo)` predicate and install the first
// matching renderer. No match → fall back to the built-in 2D highway.
// matching renderer. No match → fall back to the built-in 2D window.highway.
//
// vizSelection stays 'auto' across invocations so the next song:ready
// re-evaluates. An explicit picker choice overrides Auto by persisting
@@ -530,10 +530,10 @@ function _setAutoVizLabel(resolvedText) {
let _cancelPendingAutoLabel = null;
// One-shot (per song) hint shown when a notation-only arrangement falls back
// to the built-in 2D highway. Such arrangements carry no wire notes
// to the built-in 2D window.highway. Such arrangements carry no wire notes
// (sloppak-spec §5.3: `file:` may be omitted when `notation:` is present), so
// the default renderer draws an empty board — without this the user is left
// staring at a silently blank highway. Core ships no notation view; point at
// staring at a silently blank window.highway. Core ships no notation view; point at
// the viz picker instead.
let _notationHintShownFor = null;
function _showNotationViewHint(arrangementIndex, activeVizId) {
@@ -579,8 +579,8 @@ function _dropStaleNotationHint(activeVizId) {
const curFilename = (window.feedBack && window.feedBack.currentSong
&& window.feedBack.currentSong.filename) || '';
if (stale.dataset.filename !== curFilename) { stale.remove(); return; }
const songInfo = (typeof highway !== 'undefined' && typeof highway.getSongInfo === 'function')
? (highway.getSongInfo() || {}) : {};
const songInfo = (typeof window.highway?.getSongInfo === 'function')
? (window.highway.getSongInfo() || {}) : {};
const curArrIdx = songInfo.arrangement_index != null ? String(songInfo.arrangement_index) : null;
if (curArrIdx !== null && stale.dataset.arrangementIndex !== undefined
&& stale.dataset.arrangementIndex !== curArrIdx) {
@@ -593,8 +593,8 @@ function _dropStaleNotationHint(activeVizId) {
export function _maybeShowNotationViewHint(activeVizId) {
_dropStaleNotationHint(activeVizId);
const songInfo = (typeof highway !== 'undefined' && typeof highway.getSongInfo === 'function')
? (highway.getSongInfo() || {}) : {};
const songInfo = (typeof window.highway?.getSongInfo === 'function')
? (window.highway.getSongInfo() || {}) : {};
const activeArr = Array.isArray(songInfo.arrangements)
? songInfo.arrangements.find(a => a.index === songInfo.arrangement_index)
: null;
@@ -641,8 +641,8 @@ export function _autoMatchViz() {
// Reset label at evaluation start so a stale resolved label never persists
// if the song changes or the picker re-evaluates with a different outcome.
_setAutoVizLabel(null);
const songInfo = (typeof highway !== 'undefined' && typeof highway.getSongInfo === 'function')
? (highway.getSongInfo() || {}) : {};
const songInfo = (typeof window.highway?.getSongInfo === 'function')
? (window.highway.getSongInfo() || {}) : {};
// Only update the label when a real song is loaded. Before the first
// song_info frame, getSongInfo() returns {} — leaving the reset state
// ("Auto (match arrangement)") is correct; we haven't evaluated yet.
@@ -714,17 +714,17 @@ export function _autoMatchViz() {
_noteVizAutoMatch(id, true);
return;
}
// No match — restore the built-in 2D highway. setRenderer(null) is
// No match — restore the built-in 2D window.highway. setRenderer(null) is
// a no-op when the default is already active. If the previous Auto
// pick was a WebGL renderer, highway.setRenderer() handles the
// pick was a WebGL renderer, window.highway.setRenderer() handles the
// context-type change by replacing the canvas element (cloneNode +
// replaceWith) so the default 2D renderer's getContext('2d') always
// succeeds — no canvas-lock limitation here.
highway.setRenderer(null);
window.highway.setRenderer(null);
_notifyVizDomain('default', 'auto-match');
_noteVizAutoMatch('default', false);
// Update the label so the user can see Auto resolved to the built-in
// highway. Read from the DOM rather than hard-coding the name so a
// window.highway. Read from the DOM rather than hard-coding the name so a
// future rename of the default entry is automatically reflected.
if (hasSong) {
const defaultOpt = Array.from(sel.options).find(o => o.value === 'default');