mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-09-12 23:58:31 +00:00
refactor(h3d-carve-5): move H-section player-chrome bg-control to src/bg-control.js
Extracted _pc* subsystem (420 lines) from screen.js IIFE into
src/bg-control.js using a factory DI pattern (createBgControl({...})).
Exports: createBgControl({ BG_STYLE_IDS, _bgReadGlobal, _bgSubscribe,
_bgUnsubscribe, getVenueSceneOverride })
→ { _pcAcquire, _pcRelease }
screen.js: const { _pcAcquire, _pcRelease } = createBgControl({...})
Beyond-subst (2):
1. Factory wrapper (IIFE-scope closure → DI params) — factory export pattern
required because DI values are IIFE-scope, not ES module imports.
2. _venueSceneOverride → getVenueSceneOverride() (live accessor, 1 call site
in _pcSync — mutable let at screen.js:1523 must be read per-call).
DI values all defined before the createBgControl call (screen.js):
- BG_STYLE_IDS: line 1435 | _bgReadGlobal: 1825
- _bgSubscribe/_bgUnsubscribe: 1917-18 | _venueSceneOverride: 1523
First _pcAcquire caller: init() in createFactory() (~line 14850 post-cut).
Construction order correct: createBgControl call before createFactory.
Surprise declared to god before commit (outbox/h3d-cut5-surprise.json):
No bc-panel.js dependency — §8's anticipation was wrong. The
_bcCreateController call at what was ~line 8082 is in _bcSyncMode
(P-section / factory scope), not the H-section. bg-control.js has ZERO
dependency on bc-panel.js.
Tests:
- tests/js/highway_3d_bg_control.test.js (new, 13 class-killers):
stranded-caller (test 13, factory-adapted from bc-panel test 12),
construction-order (tests 11-12), DI completeness (test 2),
live-accessor enforcement (test 3), lifecycle (tests 4-7).
- plugins/highway_3d/tests/background_control.test.js: retargeted from
screen.js slice → bg-control.js factory eval; all 20 existing behaviour
tests preserved (load() uses vm.createContext + augmented return getters).
- tests/js/highway_3d_panel_controls.test.js: createBgControl stub added.
Suite: node --test tests/js/highway_3d*.test.js plugins/highway_3d/tests/*.test.js
base 222/222 (f69c544) → tip 235/235 (+13: 13 new class-killers)
plugin.json: 3.40.0 → 3.41.0
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014uZ169yfoFYArXz962g7KW
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
f69c544eea
commit
c4eebe1c9f
@@ -0,0 +1,253 @@
|
||||
// Class-killer tests for src/bg-control.js — h3d-carve-5.
|
||||
//
|
||||
// bg-control.js uses a factory export (createBgControl({DI})) because its
|
||||
// dependencies are IIFE-scope values that cannot be ES-module imports.
|
||||
// screen.js destructures { _pcAcquire, _pcRelease } from the factory result.
|
||||
//
|
||||
// Test strategy:
|
||||
// - Source-scan tests check structural invariants (critical paths, DI wiring,
|
||||
// accessor call site, tombstone).
|
||||
// - Screen.js wiring tests check the import clause and destructure form.
|
||||
// - Generic stranded-caller test (adapted from bc-panel.js test 12) checks
|
||||
// that every _pc* symbol in the bg-control.js factory return is also in
|
||||
// screen.js's createBgControl destructure — a bare _pcFoo reference in the
|
||||
// IIFE that isn't in the destructure is the same stranded-caller bug class.
|
||||
// - Construction-order test: createBgControl call must appear AFTER all DI
|
||||
// definitions in screen.js and BEFORE createFactory.
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const BG_CONTROL_JS = path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'src', 'bg-control.js');
|
||||
const SCREEN_JS = path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js');
|
||||
|
||||
let _src;
|
||||
function src() { if (!_src) _src = fs.readFileSync(BG_CONTROL_JS, 'utf8'); return _src; }
|
||||
|
||||
let _screenSrc;
|
||||
function screenSrc() { if (!_screenSrc) _screenSrc = fs.readFileSync(SCREEN_JS, 'utf8'); return _screenSrc; }
|
||||
|
||||
// ── 1. createBgControl is exported (not private) ──────────────────────────────
|
||||
|
||||
test('createBgControl is exported from bg-control.js', () => {
|
||||
// Mutation: remove `export` → screen.js import throws SyntaxError /
|
||||
// "does not provide an export" at module-graph load time → highway never
|
||||
// initialises; all 3D-Hwy users see a blank canvas.
|
||||
assert.match(src(), /^export\s+function\s+createBgControl\s*\(/m,
|
||||
'createBgControl must be a line-start export function declaration');
|
||||
});
|
||||
|
||||
// ── 2. DI params declared (all five) ─────────────────────────────────────────
|
||||
|
||||
test('createBgControl destructures all five DI params', () => {
|
||||
// Mutation: remove one DI param → that function is `undefined` inside the
|
||||
// factory → every call to e.g. _bgReadGlobal throws TypeError: not a function.
|
||||
const s = src();
|
||||
const sig = s.match(/export\s+function\s+createBgControl\s*\(\s*\{([^}]+)\}/);
|
||||
assert.ok(sig, 'createBgControl signature must use destructuring params');
|
||||
const params = sig[1];
|
||||
assert.match(params, /BG_STYLE_IDS/, 'DI must include BG_STYLE_IDS');
|
||||
assert.match(params, /_bgReadGlobal/, 'DI must include _bgReadGlobal');
|
||||
assert.match(params, /_bgSubscribe/, 'DI must include _bgSubscribe');
|
||||
assert.match(params, /_bgUnsubscribe/, 'DI must include _bgUnsubscribe');
|
||||
assert.match(params, /getVenueSceneOverride/, 'DI must include getVenueSceneOverride');
|
||||
});
|
||||
|
||||
// ── 3. getVenueSceneOverride() called as function (not captured at construction) ──
|
||||
|
||||
test('_pcSync calls getVenueSceneOverride() not _venueSceneOverride directly', () => {
|
||||
// Mutation: revert beyond-subst 2 to `!!_venueSceneOverride` → factory
|
||||
// captures the initial `false` at construction time; the accessor is never
|
||||
// called; Venue-active state is always `false` → UI never goes inert under
|
||||
// Venue; user can "pick" a background while Venue scene is active but the
|
||||
// pick goes nowhere because Venue owns the mount.
|
||||
const s = src();
|
||||
// Must call the accessor (with parens).
|
||||
assert.match(s, /getVenueSceneOverride\(\)/,
|
||||
'_pcSync must call getVenueSceneOverride() rather than capturing the var at construction');
|
||||
// Must NOT contain the raw closure variable name in executable code (bare, without call
|
||||
// parens). Strip line comments first so the comment-doc in the file header doesn't fire.
|
||||
const noLineComments = s.replace(/\/\/[^\n]*/g, '');
|
||||
assert.doesNotMatch(noLineComments, /\b_venueSceneOverride\b/,
|
||||
'bg-control.js must not reference bare _venueSceneOverride in code — use getVenueSceneOverride()');
|
||||
});
|
||||
|
||||
// ── 4. _bgSubscribe called inside _pcMount ────────────────────────────────────
|
||||
|
||||
test('_bgSubscribe is called inside _pcMount to register the settings listener', () => {
|
||||
// Mutation: remove _bgSubscribe call → control mounts but never receives
|
||||
// settings-bus events; a style change from Settings page never syncs back
|
||||
// to the in-player picker; the two UIs drift permanently.
|
||||
const s = src();
|
||||
const mountIdx = s.indexOf('function _pcMount()');
|
||||
assert.ok(mountIdx >= 0, '_pcMount must be defined in bg-control.js');
|
||||
const mountBlock = s.slice(mountIdx, mountIdx + 6000); // _bgSubscribe ~5200 chars in
|
||||
assert.match(mountBlock, /_bgSubscribe\s*\(/,
|
||||
'_bgSubscribe must be called inside _pcMount to register the listener');
|
||||
});
|
||||
|
||||
// ── 5. _bgUnsubscribe called inside _pcTeardownDom ───────────────────────────
|
||||
|
||||
test('_bgUnsubscribe is called inside _pcTeardownDom to deregister the listener', () => {
|
||||
// Mutation: remove _bgUnsubscribe call → listener closure outlives the control;
|
||||
// after release the stale closure still calls _pcSync on every settings change;
|
||||
// null refs (_pcSel etc.) throw on first setting write post-teardown.
|
||||
const s = src();
|
||||
const teardownIdx = s.indexOf('function _pcTeardownDom()');
|
||||
assert.ok(teardownIdx >= 0, '_pcTeardownDom must be defined in bg-control.js');
|
||||
const teardownBlock = s.slice(teardownIdx, teardownIdx + 500);
|
||||
assert.match(teardownBlock, /_bgUnsubscribe\s*\(/,
|
||||
'_bgUnsubscribe must be called inside _pcTeardownDom to remove the listener');
|
||||
});
|
||||
|
||||
// ── 6. _pcRelease calls _pcTeardownDom ───────────────────────────────────────
|
||||
|
||||
test('_pcRelease calls _pcTeardownDom when refcount reaches zero', () => {
|
||||
// Mutation: remove _pcTeardownDom() call from _pcRelease → DOM node is
|
||||
// never removed; the settings listener stays alive; under splitscreen each
|
||||
// renderer destroys independently but the control never disappears → orphaned
|
||||
// picker remains visible and partially interactive after 3D-Hwy is deselected.
|
||||
const s = src();
|
||||
const releaseIdx = s.indexOf('function _pcRelease()');
|
||||
assert.ok(releaseIdx >= 0, '_pcRelease must be defined in bg-control.js');
|
||||
const releaseBlock = s.slice(releaseIdx, releaseIdx + 1200); // _pcTeardownDom ~1040 chars in
|
||||
assert.match(releaseBlock, /_pcTeardownDom\s*\(\s*\)/,
|
||||
'_pcRelease must call _pcTeardownDom() when refcount reaches zero');
|
||||
});
|
||||
|
||||
// ── 7. _pcAcquire and _pcRelease returned from factory ───────────────────────
|
||||
|
||||
test('createBgControl returns { _pcAcquire, _pcRelease }', () => {
|
||||
// Mutation: remove either from return → screen.js destructure gets undefined;
|
||||
// first call to _pcAcquire / _pcRelease from init()/destroy() throws
|
||||
// TypeError: not a function → highway init crashes on every song load.
|
||||
const s = src();
|
||||
const returnMatch = s.match(/return\s*\{([^}]+)\}/);
|
||||
assert.ok(returnMatch, 'createBgControl must have a return { ... } statement');
|
||||
const returned = returnMatch[1];
|
||||
assert.match(returned, /_pcAcquire/, 'createBgControl must return _pcAcquire');
|
||||
assert.match(returned, /_pcRelease/, 'createBgControl must return _pcRelease');
|
||||
});
|
||||
|
||||
// ── 8. screen.js imports createBgControl from src/bg-control.js ──────────────
|
||||
|
||||
test('screen.js imports createBgControl from src/bg-control.js', () => {
|
||||
// Mutation: remove import → createBgControl is undefined in the IIFE;
|
||||
// the destructure const { _pcAcquire, _pcRelease } = createBgControl({...})
|
||||
// throws TypeError at module eval time → plugin never loads.
|
||||
assert.match(screenSrc(),
|
||||
/import\s+\{[^}]*createBgControl[^}]*\}\s+from\s+['"]\.\/src\/bg-control\.js['"]/,
|
||||
'screen.js must import createBgControl from ./src/bg-control.js');
|
||||
});
|
||||
|
||||
// ── 9. screen.js calls createBgControl with all five DI args ─────────────────
|
||||
|
||||
test('screen.js passes all five DI arguments to createBgControl', () => {
|
||||
// Mutation: omit one DI arg → the corresponding param is `undefined` inside
|
||||
// the factory closure; first call to it (on mount, on settings change) throws.
|
||||
const s = screenSrc();
|
||||
const callMatch = s.match(/createBgControl\s*\(\s*\{([^}]+)\}/);
|
||||
assert.ok(callMatch, 'screen.js must call createBgControl({...})');
|
||||
const args = callMatch[1];
|
||||
assert.match(args, /BG_STYLE_IDS/, 'createBgControl call must pass BG_STYLE_IDS');
|
||||
assert.match(args, /_bgReadGlobal/, 'createBgControl call must pass _bgReadGlobal');
|
||||
assert.match(args, /_bgSubscribe/, 'createBgControl call must pass _bgSubscribe');
|
||||
assert.match(args, /_bgUnsubscribe/, 'createBgControl call must pass _bgUnsubscribe');
|
||||
assert.match(args, /getVenueSceneOverride/, 'createBgControl call must pass getVenueSceneOverride');
|
||||
});
|
||||
|
||||
// ── 10. screen.js IIFE does not redefine _pcAcquire or _pcRelease ────────────
|
||||
|
||||
test('screen.js IIFE does not redeclare _pcAcquire or _pcRelease', () => {
|
||||
// Mutation: re-add `function _pcAcquire()` to the IIFE → IIFE-scope function
|
||||
// shadows the destructured import; the factory's _pcRelease holds a stale
|
||||
// closure over the old _pcRefs; refcount goes out of sync; the control
|
||||
// never unmounts.
|
||||
const s = screenSrc();
|
||||
const iife = s.replace(/^import\s+.*\n/gm, '');
|
||||
assert.doesNotMatch(iife, /function\s+_pcAcquire\s*\(/,
|
||||
'IIFE must not redeclare _pcAcquire');
|
||||
assert.doesNotMatch(iife, /function\s+_pcRelease\s*\(/,
|
||||
'IIFE must not redeclare _pcRelease');
|
||||
});
|
||||
|
||||
// ── 11. Construction order: createBgControl called before createFactory ───────
|
||||
|
||||
test('createBgControl call appears before createFactory in screen.js', () => {
|
||||
// Mutation: move createBgControl call inside createFactory → each renderer
|
||||
// instance gets its own independent control (refcount broken across instances);
|
||||
// or if moved after createFactory but before register, correct for
|
||||
// single-instance but still wrong order risk. This test ensures the call
|
||||
// stays at module scope BEFORE the factory.
|
||||
const s = screenSrc();
|
||||
const bgCallIdx = s.indexOf('createBgControl(');
|
||||
const factoryIdx = s.indexOf('function createFactory()');
|
||||
assert.ok(bgCallIdx >= 0, 'createBgControl call must exist in screen.js');
|
||||
assert.ok(factoryIdx >= 0, 'createFactory must exist in screen.js');
|
||||
assert.ok(bgCallIdx < factoryIdx,
|
||||
'createBgControl must be called before createFactory in screen.js');
|
||||
});
|
||||
|
||||
// ── 12. Construction order: DI values defined before createBgControl call ─────
|
||||
|
||||
test('all DI values are defined before the createBgControl call in screen.js', () => {
|
||||
// Mutation: move createBgControl call before BG_STYLE_IDS / _bgReadGlobal /
|
||||
// _bgSubscribe / _bgUnsubscribe / getVenueSceneOverride binding →
|
||||
// undefined passed as DI params; factory closure captures undefined → TypeError.
|
||||
const s = screenSrc();
|
||||
const bgCallIdx = s.indexOf('createBgControl(');
|
||||
assert.ok(bgCallIdx >= 0, 'createBgControl call must exist in screen.js');
|
||||
const bgStyleIdsIdx = s.indexOf('BG_STYLE_IDS =');
|
||||
const bgReadIdx = s.indexOf('function _bgReadGlobal(');
|
||||
const bgSubIdx = s.indexOf('function _bgSubscribe(');
|
||||
const venueIdx = s.indexOf('let _venueSceneOverride');
|
||||
assert.ok(bgStyleIdsIdx < bgCallIdx, 'BG_STYLE_IDS must be defined before createBgControl call');
|
||||
assert.ok(bgReadIdx < bgCallIdx, '_bgReadGlobal must be defined before createBgControl call');
|
||||
assert.ok(bgSubIdx < bgCallIdx, '_bgSubscribe must be defined before createBgControl call');
|
||||
assert.ok(venueIdx < bgCallIdx, '_venueSceneOverride must be defined before createBgControl call');
|
||||
});
|
||||
|
||||
// ── 13. Generic stranded-caller: _pc* returned symbols in screen.js destructure ─
|
||||
|
||||
test('every _pc* symbol returned by createBgControl is in the screen.js destructure', () => {
|
||||
// Adapted from bc-panel.js test 12 for the factory pattern.
|
||||
// For a factory module, the stranded-caller class is: a symbol that appears
|
||||
// in the `return { ... }` of createBgControl but is NOT in the `const { ... }
|
||||
// = createBgControl(...)` destructure in screen.js — meaning the symbol is
|
||||
// exported at runtime but screen.js never binds it, so any IIFE caller of
|
||||
// that symbol hits ReferenceError (or the undefined stub from a stale
|
||||
// function-scope redeclaration).
|
||||
//
|
||||
// Mutation: add `_pcNewFn` to bg-control.js return but not to screen.js
|
||||
// destructure → leaked is non-empty → test RED.
|
||||
const bgSrc = src();
|
||||
const scrSrc = screenSrc();
|
||||
|
||||
// Symbols in the return { ... } of createBgControl.
|
||||
const returnMatch = bgSrc.match(/return\s*\{([^}]+)\}/);
|
||||
assert.ok(returnMatch, 'bg-control.js must have a return { ... } statement');
|
||||
const returned = new Set(
|
||||
returnMatch[1].split(',').map(s => s.trim().replace(/^(\w+)\s*:.*$/, '$1')).filter(Boolean)
|
||||
);
|
||||
|
||||
// Symbols in the screen.js destructure.
|
||||
const destructureMatch = scrSrc.match(/const\s*\{\s*([^}]+)\}\s*=\s*createBgControl\s*\(/);
|
||||
assert.ok(destructureMatch, 'screen.js must destructure the createBgControl result');
|
||||
const destructured = new Set(
|
||||
destructureMatch[1].split(',').map(s => s.trim().replace(/^(\w+)\s*:.*$/, '$1')).filter(Boolean)
|
||||
);
|
||||
|
||||
// Strip import lines and line comments from IIFE body to avoid false positives.
|
||||
const noImports = scrSrc.replace(/^import\s+.*\n/gm, '');
|
||||
const noComments = noImports.replace(/\/\/[^\n]*/g, '');
|
||||
|
||||
// Returned symbols referenced in the IIFE body but absent from the destructure.
|
||||
const leaked = [...returned].filter(
|
||||
sym => new RegExp('\\b' + sym + '\\b').test(noComments) && !destructured.has(sym)
|
||||
);
|
||||
assert.deepStrictEqual(leaked, [],
|
||||
'screen.js references createBgControl return symbols not in its destructure: ' +
|
||||
leaked.join(', '));
|
||||
});
|
||||
Reference in New Issue
Block a user