From 6e4a8da734fa0afce973360ec7bd53867c0df747 Mon Sep 17 00:00:00 2001 From: Ionut Adrian Ciolan Date: Mon, 10 Aug 2026 02:31:43 +0300 Subject: [PATCH] fix: handle dirty state before first build When library:changed marks Songs dirty before its DOM shell exists, reload cannot build the screen and the first entry remains blank. Reload only an already-built screen; otherwise continue through the full render path. Keep screen:changed as the primary navigation contract, preserve one rapid re-entry while entry is in flight, and remove the MutationObserver recovery that did not address this path. Signed-off-by: Ionut Adrian Ciolan --- CHANGELOG.md | 2 + static/v3/songs.js | 76 ++++++++- tests/js/v3_songs_entry_lifecycle.test.js | 179 ++++++++++++++++++++++ 3 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 tests/js/v3_songs_entry_lifecycle.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c262781..e02fa67a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -313,6 +313,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). ### Fixed +- Fix blank first entry when library changes before initial build by deferring + reload to the render path if the DOM is not yet built. - **Count-in follows the song's meter and its pickup measure.** The count-in (loop wrap, section practice, and the "Countdown before song" setting) always clicked exactly four beats, so a 3/4 song was counted in 4/4, and a song diff --git a/static/v3/songs.js b/static/v3/songs.js index 09ef004f..76f1b327 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -3828,7 +3828,10 @@ // A library scan / DLC-folder change marked the grid stale — re-fetch // from scratch instead of restoring a cached (possibly empty, pre-DLC) // snapshot. Must win over every fast-path below. - if (_libraryDirty) { _libraryDirty = false; await reload(); return; } + if (_libraryDirty) { + _libraryDirty = false; + if (state.built) { await reload(); return; } + } // Keep the entry-point gates current (a Settings visit may have // toggled artist pages / external links). Fire-and-forget. refreshArtistPageGates(); @@ -3907,6 +3910,58 @@ _clearLibraryScrollSnapshot(); } + // Coalesce duplicate delivery into the current entry, but retain one real + // re-entry request that arrives while it is still running. + function createSongsEntryCoordinator(enter) { + let inFlight = null; + let rerun = false; + return function runSongsEntry() { + if (inFlight) { + rerun = true; + return inFlight; + } + inFlight = Promise.resolve() + .then(async () => { + do { + rerun = false; + await enter(); + } while (rerun); + }) + .finally(() => { + inFlight = null; + rerun = false; + }); + return inFlight; + }; + } + + function createSongsEntryLifecycle(enter, isActive) { + const runEntry = createSongsEntryCoordinator(enter); + let wasActive = false; + return { + onScreenChanged(e) { + const id = e && e.detail && e.detail.id; + if (id === 'v3-songs') { + // showScreen changes classes before emitting, so mark the + // edge consumed before the one-time active check runs. + wasActive = true; + return runEntry(); + } + wasActive = !!isActive(); + return null; + }, + syncActiveClass() { + const active = !!isActive(); + if (active && !wasActive) { + wasActive = true; + return runEntry(); + } + wasActive = active; + return null; + }, + }; + } + // After an upload click-through (which reuses the legacy uploader + // background scan), poll /api/scan-status and reload the v3 grid once the // scan we triggered finishes — the legacy uploader only refreshes the @@ -4380,10 +4435,27 @@ else if (e.key === 'ArrowUp') _gpMove(-cols); }); + const songsScreen = document.getElementById('v3-songs'); + const songsEntryLifecycle = createSongsEntryLifecycle( + onV3SongsScreenEnter, + () => !!(songsScreen && songsScreen.classList.contains('active')), + ); + + // screen:changed is the primary navigation contract. The one-time active + // check covers a screen activated before this later defer script registered. + if (songsScreen) { + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', () => songsEntryLifecycle.syncActiveClass(), { once: true }); + } else { + songsEntryLifecycle.syncActiveClass(); + } + } + if (sm && typeof sm.on === 'function') { sm.on('screen:changed', (e) => { const id = e && e.detail && e.detail.id; - if (id === 'v3-songs') { onV3SongsScreenEnter(); return; } + songsEntryLifecycle.onScreenChanged(e); + if (id === 'v3-songs') return; // Leaving Songs: tear down select mode + the body-mounted batch bar, // so an active multi-selection doesn't leave a floating bar (and // stale selection) visible on unrelated screens. diff --git a/tests/js/v3_songs_entry_lifecycle.test.js b/tests/js/v3_songs_entry_lifecycle.test.js new file mode 100644 index 00000000..4c143e14 --- /dev/null +++ b/tests/js/v3_songs_entry_lifecycle.test.js @@ -0,0 +1,179 @@ +'use strict'; + +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 SONGS_JS = path.join(__dirname, '..', '..', 'static', 'v3', 'songs.js'); + +function loadLifecycle() { + const src = fs.readFileSync(SONGS_JS, 'utf8'); + const coordinator = extractFunction(src, 'function createSongsEntryCoordinator'); + const lifecycle = extractFunction(src, 'function createSongsEntryLifecycle'); + const sandbox = {}; + vm.createContext(sandbox); + vm.runInContext( + `${coordinator}\n${lifecycle}\nglobalThis.__createLifecycle = createSongsEntryLifecycle;`, + sandbox, + ); + return { createLifecycle: sandbox.__createLifecycle, source: src }; +} + +function loadScreenEnter({ dirty, built }) { + const src = fs.readFileSync(SONGS_JS, 'utf8'); + const screenEnter = extractFunction(src, 'async function onV3SongsScreenEnter'); + const calls = { reload: 0, render: 0 }; + const sandbox = { calls }; + vm.createContext(sandbox); + vm.runInContext(` + let _libraryDirty = ${dirty}; + const state = { built: ${built}, artistPage: null, view: 'grid' }; + const document = { getElementById() { return null; } }; + function refreshArtistPageGates() {} + async function applyScoreRefresh() {} + function _readLibraryScrollSnapshot() { return null; } + function _libraryStateHash() { return ''; } + function _chromeIntact() { return false; } + async function reload() { calls.reload++; } + async function render() { calls.render++; } + function _clearLibraryScrollSnapshot() {} + function _applyMainScrollTop() {} + function requestWindowRender() {} + ${screenEnter} + globalThis.__screenEnter = onV3SongsScreenEnter; + `, sandbox); + return { enter: sandbox.__screenEnter, calls }; +} + +function deferred() { + let resolve; + const promise = new Promise((done) => { resolve = done; }); + return { promise, resolve }; +} + +test('class activation renders when the initial screen:changed event was missed', async () => { + const { createLifecycle } = loadLifecycle(); + let active = false; + let entries = 0; + const lifecycle = createLifecycle(async () => { entries++; }, () => active); + + lifecycle.syncActiveClass(); + active = true; + await lifecycle.syncActiveClass(); + + assert.equal(entries, 1); +}); + +test('an already-active screen enters once when songs.js registers late', async () => { + const { createLifecycle } = loadLifecycle(); + let entries = 0; + const lifecycle = createLifecycle(async () => { entries++; }, () => true); + + await lifecycle.syncActiveClass(); + + assert.equal(entries, 1); +}); + +test('screen event and observer edge coalesce while entry is in flight', async () => { + const { createLifecycle } = loadLifecycle(); + let active = true; + let entries = 0; + const gate = deferred(); + const lifecycle = createLifecycle(async () => { + entries++; + await gate.promise; + }, () => active); + + const fromEvent = lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }); + const fromObserver = lifecycle.syncActiveClass(); + // The coordinator starts entry in a microtask so synchronous event + + // initial-state triggers can share the same in-flight Promise. + await Promise.resolve(); + + assert.equal(entries, 1); + assert.equal(fromObserver, null); + gate.resolve(); + await fromEvent; + assert.equal(entries, 1); +}); + +test('leaving and entering again runs the full entry lifecycle again', async () => { + const { createLifecycle } = loadLifecycle(); + let active = true; + let entries = 0; + const lifecycle = createLifecycle(async () => { entries++; }, () => active); + + await lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }); + active = false; + lifecycle.onScreenChanged({ detail: { id: 'v3-home' } }); + active = true; + await lifecycle.syncActiveClass(); + + assert.equal(entries, 2); +}); + +test('rapid leave and re-entry queues one follow-up while entry is in flight', async () => { + const { createLifecycle } = loadLifecycle(); + let active = true; + let entries = 0; + const gate = deferred(); + const lifecycle = createLifecycle(async () => { + entries++; + if (entries === 1) await gate.promise; + }, () => active); + + const first = lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }); + await Promise.resolve(); + active = false; + lifecycle.onScreenChanged({ detail: { id: 'v3-home' } }); + active = true; + const reentry = lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }); + gate.resolve(); + await Promise.all([first, reentry]); + + assert.equal(entries, 2); +}); + +test('a rejected entry clears coordinator state so a later navigation retries', async () => { + const { createLifecycle } = loadLifecycle(); + let entries = 0; + const lifecycle = createLifecycle(async () => { + entries++; + if (entries === 1) throw new Error('entry failed'); + }, () => true); + + await assert.rejects(lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }), /entry failed/); + await lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }); + + assert.equal(entries, 2); +}); + +test('dirty first entry builds the Songs shell instead of reloading absent DOM', async () => { + const { enter, calls } = loadScreenEnter({ dirty: true, built: false }); + + await enter(); + + assert.deepEqual(calls, { reload: 0, render: 1 }); +}); + +test('repeated navigation has one entry per return and no observer wiring', async () => { + const { createLifecycle, source } = loadLifecycle(); + let active = false; + let entries = 0; + const lifecycle = createLifecycle(async () => { entries++; }, () => active); + + for (let i = 0; i < 3; i++) { + active = true; + await lifecycle.onScreenChanged({ detail: { id: 'v3-songs' } }); + active = false; + lifecycle.onScreenChanged({ detail: { id: 'v3-home' } }); + } + + assert.equal(entries, 3); + assert.equal((source.match(/sm\.on\('screen:changed'/g) || []).length, 1); + assert.doesNotMatch(source, /new MutationObserver\(/); + assert.match(source, /DOMContentLoaded[^\n]*syncActiveClass/); +});