mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 12:21:49 +00:00
8bec8d2466
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) <noreply@anthropic.com>
73 lines
3.7 KiB
JavaScript
73 lines
3.7 KiB
JavaScript
// Regression: the play/pause button must not be reset to "Play" when an
|
|
// in-flight togglePlay() audio.play() is rejected *because the engine reroute
|
|
// (HTML5 -> JUCE) deliberately paused the <audio> element*. Playback continues
|
|
// on the JUCE transport, so the button must stay "Pause" (isPlaying true).
|
|
//
|
|
// Bug: first song after a fresh load on desktop — the reroute's audio.pause()
|
|
// aborts autoplay's play(); togglePlay's catch then flipped the button to Play
|
|
// while the song kept playing, so it took two clicks to actually pause.
|
|
//
|
|
// Same isolation strategy as autoplay_exit.test.js: extract togglePlay() from
|
|
// app.js by brace-matching and run it in a vm sandbox with stubbed deps.
|
|
|
|
const { test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
const vm = require('node:vm');
|
|
|
|
const { extractFunction } = require('./test_utils');
|
|
|
|
const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'transport.js');
|
|
const SRC = fs.readFileSync(APP_JS, 'utf8');
|
|
const TOGGLE_PLAY_SRC = extractFunction(SRC, 'async function togglePlay(');
|
|
|
|
// Drive togglePlay() from the not-playing state with an HTML5 audio.play() that
|
|
// rejects, optionally with a reroute in progress. Returns the observed button
|
|
// states and the final isPlaying flag.
|
|
async function runTogglePlayRejecting({ rerouteInProgress }) {
|
|
const buttonStates = [];
|
|
const sandbox = {
|
|
console: { log() {}, warn() {}, error() {} },
|
|
// not-playing -> togglePlay takes the HTML5 play branch.
|
|
// isPlaying / lastAudioTime moved onto the shared player-state container
|
|
// (static/js/player-state.js) so a carved module can WRITE them — an imported
|
|
// binding is read-only. Same values, same assertions, one indirection.
|
|
S: { isPlaying: false, lastAudioTime: 0 },
|
|
_audioSeekGen: 0,
|
|
_playAttemptGen: 0,
|
|
setPlayButtonState(v) { buttonStates.push(v); },
|
|
audio: {
|
|
// Reject like the browser does when a pending play() is interrupted
|
|
// by a pause() (the reroute's deliberate audio.pause()).
|
|
play: () => Promise.reject(new DOMException('aborted by pause', 'AbortError')),
|
|
pause() {},
|
|
},
|
|
jucePlayer: { play: () => Promise.resolve(true), pause: () => Promise.resolve() },
|
|
window: {
|
|
_juceMode: false,
|
|
_juceRerouteInProgress: rerouteInProgress ? 1 : 0,
|
|
feedBack: { isPlaying: false, emit() {} },
|
|
},
|
|
};
|
|
sandbox.globalThis = sandbox;
|
|
vm.createContext(sandbox);
|
|
vm.runInContext(TOGGLE_PLAY_SRC, sandbox, { filename: 'app.js#togglePlay' });
|
|
await vm.runInContext('togglePlay()', sandbox);
|
|
return { buttonStates, isPlaying: sandbox.S.isPlaying };
|
|
}
|
|
|
|
test('reroute-aborted play() leaves the button on Pause (isPlaying stays true)', async () => {
|
|
const { buttonStates, isPlaying } = await runTogglePlayRejecting({ rerouteInProgress: true });
|
|
// Optimistic flip to Pause happened; the reroute guard must prevent the
|
|
// catch from flipping it back to Play.
|
|
assert.deepEqual(buttonStates, [true], 'button should only have been set to Pause, never reset to Play');
|
|
assert.equal(isPlaying, true, 'isPlaying must stay true — the JUCE transport owns playback');
|
|
});
|
|
|
|
test('a genuine play() rejection (no reroute) still resets the button to Play', async () => {
|
|
const { buttonStates, isPlaying } = await runTogglePlayRejecting({ rerouteInProgress: false });
|
|
assert.deepEqual(buttonStates, [true, false], 'button set to Pause then correctly reset to Play on real failure');
|
|
assert.equal(isPlaying, false, 'isPlaying must reflect the failed start');
|
|
});
|