mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-10 18:59:56 +00:00
refactor(app): carve the song session out of app.js — playSong, showScreen, closeCurrentSong (R3d) (#921)
36 declarations + the 4 autoplay/auto-exit gate statements. 359 lines. app.js 3,772 -> 3,242. Bodies VERBATIM. ━━━ THIS WAS "THE UNCUTTABLE HEART", AND IT IS 359 LINES ━━━ At the start of this epic, seeding a dependency closure from count-in, from loops, from section-practice, or from the JUCE seek shim all returned the SAME 178-function, 3,360-line set. playSong and showScreen called each other; everything called them; nothing could be cut anywhere. The conclusion — correct at the time — was that NO closure-based carve could touch it at any seed, and the answer was a host seam. That was true THEN. Every slice taken out since (transport, loops, count-in, section-practice, the library, the edit modal, settings) removed edges, and the strongly-connected component DISSOLVED. This closure is 36 declarations with an interface width of FOUR. The lesson is not that the seam was wrong — the seam is what MADE this possible, by letting the carves proceed against a cyclic core instead of stalling on it. The lesson is to RE-MEASURE. An SCC is a fact about a graph at a moment, not a property of the code. ━━━ THE BUG NO SCAN COULD SEE, AND THE A/B DID ━━━ First cut passed every gate — no-undef clean, no-cycle clean, 1045/1045, pytest green — and THREW IN THE BROWSER: "Assignment to constant variable." window.feedBack.holdAutoplay / holdAutoExit and their two event handlers are TOP-LEVEL STATEMENTS, not declarations. They WRITE this cluster's state (_autoplayHeld, _autoExitTimer, …), and an imported binding is READ-ONLY — so left behind in app.js, every one threw the instant the module existed. A dependency scan that walks DECLARATIONS cannot see them. Mine didn't. This is the same blind spot that nearly shipped a dead library A-Z rail (#896): app.js keeps its public API in top-level statements, and a call-graph is blind to every one of them. The extractor now finds them by construction — any top-level statement that WRITES a moved binding comes with the carve — and the gate statements live beside the machinery they drive, which is where they belonged anyway. ━━━ ZERO OUTSIDE WRITES, BY MOVING THE BOUNDARY RATHER THAN BUILDING MACHINERY ━━━ The autoplay scalars and the wake-lock state were written from outside the cluster, which would have forced a setter or a state container. But the writers — _releaseAutoplay, _acquireWakeLock — plainly belong here. Pulling them in left ZERO outside writes, so every export is a plain import. Same move as settings (#920): measure the writers before you reach for a container. VERIFIED. A/B against origin/main in two browsers, IDENTICAL, zero page errors — including the autoplay gate driven end to end: a plugin HOLDS autoplay, the song loads but does not start, the RELEASE fires it, and a stale release is a no-op. That is the exact machinery that was throwing. node 1045, pytest 2425, ESLint 0 (no-cycle clean), host contract 2/2, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
84fe29688c
commit
545e569ad6
@@ -18,7 +18,14 @@ const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
|
||||
// auto-exit machinery around it (_clearAutoExit, holdAutoExit, _resolvePlayerOrigin)
|
||||
// stayed in app.js.
|
||||
const CONTROLS_JS = path.join(__dirname, '..', '..', 'static', 'js', 'player-controls.js');
|
||||
const SRC = fs.readFileSync(APP_JS, 'utf8');
|
||||
// R3d: the song session (showScreen / playSong / closeCurrentSong, and the autoplay hold and
|
||||
// auto-exit timer they own) was carved out of app.js into static/js/session.js. This file slices
|
||||
// functions from BOTH — `_resultsOverlayVisible` is still in app.js; `_releaseAutoplay` and
|
||||
// `_resolvePlayerOrigin` moved. Read both and strip `export`, exactly as CONTROLS_SRC already
|
||||
// does, rather than re-pinning each extraction at whichever file currently holds it.
|
||||
const SESSION_JS = path.join(__dirname, '..', '..', 'static', 'js', 'session.js');
|
||||
const SRC = fs.readFileSync(APP_JS, 'utf8')
|
||||
+ '\n' + fs.readFileSync(SESSION_JS, 'utf8').replace(/^export /gm, '');
|
||||
// the module is ESM; these sandboxes evaluate plain script text
|
||||
const CONTROLS_SRC = fs.readFileSync(CONTROLS_JS, 'utf8').replace(/^export /gm, '');
|
||||
|
||||
|
||||
@@ -12,6 +12,11 @@ const vm = require('node:vm');
|
||||
|
||||
const { extractFunction } = require('./test_utils');
|
||||
|
||||
// R3d: closeCurrentSong (and showScreen and playSong, the mutual recursion they form) moved to
|
||||
// static/js/session.js. Bodies unchanged — only the file. The WINDOW CONTRACT stays in app.js,
|
||||
// which is the whole point of it: app.js is the only place that publishes names for the markup's
|
||||
// onclick= handlers to resolve against.
|
||||
const SESSION_JS = path.join(__dirname, '..', '..', 'static', 'js', 'session.js');
|
||||
const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
|
||||
|
||||
function buildSandbox({ playerOriginScreen = 'home' } = {}) {
|
||||
@@ -66,13 +71,13 @@ function loadClose(sandbox, src) {
|
||||
}
|
||||
|
||||
test('closeCurrentSong is exported on window and window.feedBack', () => {
|
||||
const src = fs.readFileSync(APP_JS, 'utf8');
|
||||
const src = fs.readFileSync(APP_JS, 'utf8'); // the contract lives in app.js
|
||||
assert.match(src, /window\.closeCurrentSong\s*=\s*closeCurrentSong/);
|
||||
assert.match(src, /window\.feedBack\.closeCurrentSong\s*=\s*closeCurrentSong/);
|
||||
});
|
||||
|
||||
test('closeCurrentSong uses _playerOriginScreen when set', async () => {
|
||||
const src = fs.readFileSync(APP_JS, 'utf8');
|
||||
const src = fs.readFileSync(SESSION_JS, 'utf8');
|
||||
const sandbox = buildSandbox({ playerOriginScreen: 'favorites' });
|
||||
loadClose(sandbox, src);
|
||||
await sandbox.__closeCurrentSong();
|
||||
@@ -87,7 +92,7 @@ test('closeCurrentSong uses _playerOriginScreen when set', async () => {
|
||||
});
|
||||
|
||||
test('closeCurrentSong falls back to home when origin missing', async () => {
|
||||
const src = fs.readFileSync(APP_JS, 'utf8');
|
||||
const src = fs.readFileSync(SESSION_JS, 'utf8');
|
||||
const sandbox = buildSandbox({ playerOriginScreen: null });
|
||||
loadClose(sandbox, src);
|
||||
await sandbox.__closeCurrentSong();
|
||||
@@ -96,7 +101,7 @@ test('closeCurrentSong falls back to home when origin missing', async () => {
|
||||
});
|
||||
|
||||
test('closeCurrentSong falls back to home when origin is empty string', async () => {
|
||||
const src = fs.readFileSync(APP_JS, 'utf8');
|
||||
const src = fs.readFileSync(SESSION_JS, 'utf8');
|
||||
const sandbox = buildSandbox({ playerOriginScreen: '' });
|
||||
loadClose(sandbox, src);
|
||||
await sandbox.__closeCurrentSong();
|
||||
|
||||
@@ -4,7 +4,8 @@ const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const vm = require('node:vm');
|
||||
|
||||
const APP_JS = path.join(__dirname, '..', '..', 'static', 'app.js');
|
||||
// R3d: playSong moved to static/js/session.js with showScreen and closeCurrentSong.
|
||||
const APP_JS = path.join(__dirname, '..', '..', 'static', 'js', 'session.js');
|
||||
// The speed controls were carved out into static/js/player-controls.js (R3a); playSong,
|
||||
// which resets them on a new song, stayed in app.js. This test spans both.
|
||||
const CONTROLS_JS = path.join(__dirname, '..', '..', 'static', 'js', 'player-controls.js');
|
||||
|
||||
@@ -488,8 +488,14 @@ test('gate: does not claim a hold when the feature is off', async () => {
|
||||
assert.equal(holds, 0);
|
||||
});
|
||||
|
||||
test('the autoplay gate is a generic core hook with a fail-open backstop (app.js)', () => {
|
||||
const appSrc = fs.readFileSync(APP_JS, 'utf8');
|
||||
test('the autoplay gate is a generic core hook with a fail-open backstop', () => {
|
||||
// R3d: the gate SPANS two files now. window.feedBack.holdAutoplay is the public hook and
|
||||
// stays on app.js's window contract; the machinery it drives (_autoplayHeld,
|
||||
// _clearAutoplayHold, the backstop) moved to static/js/session.js with playSong. Read both —
|
||||
// re-pinning at one would silently stop checking half the gate.
|
||||
const SESSION_JS = path.join(__dirname, '..', '..', 'static', 'js', 'session.js');
|
||||
const appSrc = fs.readFileSync(APP_JS, 'utf8')
|
||||
+ '\n' + fs.readFileSync(SESSION_JS, 'utf8').replace(/^export /gm, '');
|
||||
assert.match(appSrc, /window\.feedBack\.holdAutoplay = function/);
|
||||
assert.match(appSrc, /AUTOPLAY_HOLD_BACKSTOP_MS/); // fail-open: never strand the song
|
||||
assert.match(appSrc, /if \(_autoplayHeld\) \{ _autoplayStart = start;/); // a gated start is stashed
|
||||
|
||||
Reference in New Issue
Block a user