From cfc3c66f1435eeadd4e7540b3b530c7f9690ae47 Mon Sep 17 00:00:00 2001 From: ranokay Date: Sat, 17 Oct 2026 12:17:59 +0300 Subject: [PATCH] feat(ui): refresh lyrics variants progressively Signed-off-by: ranokay --- ui/src/audioplayer/useEnhancedLyrics.js | 194 ++++++++++++------- ui/src/audioplayer/useEnhancedLyrics.test.js | 135 +++++++++++++ 2 files changed, 260 insertions(+), 69 deletions(-) diff --git a/ui/src/audioplayer/useEnhancedLyrics.js b/ui/src/audioplayer/useEnhancedLyrics.js index 8577e9280..2fcc59248 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.js +++ b/ui/src/audioplayer/useEnhancedLyrics.js @@ -1,6 +1,11 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import subsonic from '../subsonic' -import { getPreferredLyricLanguage, selectLyricLayers } from './lyrics' +import { + collectLyricVariants, + getPreferredLyricLanguage, + hasStructuredLyricContent, + selectLyricLayers, +} from './lyrics' export const emptyLyricLayers = Object.freeze({ main: null, @@ -15,11 +20,8 @@ const COOLDOWN_PATTERN = /(?:cooldown active for|retry after)\s+(\d+)s/gi const cache = new Map() const inFlight = new Map() -const normalizeLyricLayers = (layers) => ({ - main: layers?.main || null, - translation: layers?.translation || null, - pronunciation: layers?.pronunciation || null, -}) +const normalizeStructuredLyrics = (lyrics) => + Array.isArray(lyrics) ? lyrics : [] const readStructuredLyrics = (response) => response?.json?.['subsonic-response']?.lyricsList?.structuredLyrics || [] @@ -36,9 +38,9 @@ const retryDelayFromError = (error) => { const buildCacheKey = ({ trackId, preferredLanguage, updatedAt }) => [trackId || '', updatedAt || '', preferredLanguage || ''].join('\u0000') -const rememberLyrics = (cacheKey, layers, expiresAt = null) => { +const rememberLyrics = (cacheKey, structuredLyrics, expiresAt = null) => { cache.delete(cacheKey) - cache.set(cacheKey, { layers, expiresAt }) + cache.set(cacheKey, { structuredLyrics, expiresAt }) while (cache.size > MAX_LYRIC_CACHE_ENTRIES) { const oldestCacheKey = cache.keys().next().value cache.delete(oldestCacheKey) @@ -54,10 +56,10 @@ const readCachedLyrics = (cacheKey) => { } cache.delete(cacheKey) cache.set(cacheKey, cached) - return cached.layers + return cached.structuredLyrics } -const createLyricsRequest = ({ trackId, preferredLanguage, cacheKey }) => { +const createLyricsRequest = ({ trackId, cacheKey }) => { const controller = new AbortController() const entry = { controller, @@ -68,18 +70,16 @@ const createLyricsRequest = ({ trackId, preferredLanguage, cacheKey }) => { entry.promise = subsonic .getLyricsBySongId(trackId, { signal: controller.signal }) .then((response) => { - const selected = normalizeLyricLayers( - selectLyricLayers(readStructuredLyrics(response), preferredLanguage), - ) - const hasAnyLayer = Boolean( - selected.main || selected.translation || selected.pronunciation, + const structuredLyrics = normalizeStructuredLyrics( + readStructuredLyrics(response), ) + const hasAnyLyric = structuredLyrics.some(hasStructuredLyricContent) rememberLyrics( cacheKey, - selected, - hasAnyLayer ? null : Date.now() + NEGATIVE_CACHE_TTL_MS, + structuredLyrics, + hasAnyLyric ? null : Date.now() + NEGATIVE_CACHE_TTL_MS, ) - return selected + return structuredLyrics }) .finally(() => { entry.settled = true @@ -89,12 +89,12 @@ const createLyricsRequest = ({ trackId, preferredLanguage, cacheKey }) => { return entry } -const acquireLyricsRequest = ({ trackId, preferredLanguage, cacheKey }) => { +const acquireLyricsRequest = ({ trackId, cacheKey }) => { const existing = inFlight.get(cacheKey) const entry = existing && !existing.controller.signal.aborted ? existing - : createLyricsRequest({ trackId, preferredLanguage, cacheKey }) + : createLyricsRequest({ trackId, cacheKey }) entry.consumers += 1 let released = false return { @@ -111,88 +111,127 @@ const acquireLyricsRequest = ({ trackId, preferredLanguage, cacheKey }) => { } } +const emptyLyricsState = (cacheKey) => ({ + cacheKey, + structuredLyrics: null, + loading: false, + refreshing: false, + error: null, + retryAt: null, +}) + const useEnhancedLyrics = ({ trackId, updatedAt, disabled = false, requested = true, + selectedSourceKey = null, }) => { const preferredLanguage = getPreferredLyricLanguage() const cacheKey = useMemo( () => buildCacheKey({ trackId, preferredLanguage, updatedAt }), [preferredLanguage, trackId, updatedAt], ) - const [state, setState] = useState(() => ({ - cacheKey, - layers: emptyLyricLayers, - loading: false, - error: null, - retryAt: null, - })) + const [state, setState] = useState(() => emptyLyricsState(cacheKey)) const [requestGeneration, setRequestGeneration] = useState(0) + const [refreshToken, setRefreshToken] = useState(0) const [, setCountdownTick] = useState(0) + // The refresh path reads the currently displayed state when deciding whether + // to keep it visible, so mirror it through a ref updated before the request + // effect runs. + const stateRef = useRef(state) + useEffect(() => { + stateRef.current = state + }, [state]) + const handledRefreshRef = useRef(refreshToken) + const retry = useCallback(() => { cache.delete(cacheKey) setRequestGeneration((current) => current + 1) }, [cacheKey]) + const refresh = useCallback(() => { + if (!trackId || disabled || !requested) return + setRefreshToken((current) => current + 1) + }, [disabled, requested, trackId]) + useEffect(() => { if (!trackId || disabled || !requested) { - setState({ - cacheKey, - layers: emptyLyricLayers, - loading: false, - error: null, - retryAt: null, - }) + handledRefreshRef.current = refreshToken + setState(emptyLyricsState(cacheKey)) return undefined } - const cached = readCachedLyrics(cacheKey) - if (cached) { - setState({ - cacheKey, - layers: cached, - loading: false, - error: null, - retryAt: null, - }) - return undefined + const isRefresh = refreshToken !== handledRefreshRef.current + handledRefreshRef.current = refreshToken + + if (!isRefresh) { + const cached = readCachedLyrics(cacheKey) + if (cached) { + setState({ + cacheKey, + structuredLyrics: cached, + loading: false, + refreshing: false, + error: null, + retryAt: null, + }) + return undefined + } } + // Progressive refreshes keep the currently shown lyrics; a refresh landing + // while a request is in flight coalesces with it. + const keepingContent = Boolean( + isRefresh && + stateRef.current.cacheKey === cacheKey && + stateRef.current.structuredLyrics?.length, + ) let active = true - const request = acquireLyricsRequest({ - trackId, - preferredLanguage, - cacheKey, - }) - setState({ - cacheKey, - layers: emptyLyricLayers, - loading: true, - error: null, - retryAt: null, - }) + const request = acquireLyricsRequest({ trackId, cacheKey }) + setState((current) => + keepingContent + ? { ...current, refreshing: true } + : { + cacheKey, + structuredLyrics: null, + loading: true, + refreshing: false, + error: null, + retryAt: null, + }, + ) request.promise - .then((layers) => { + .then((structuredLyrics) => { if (!active) return setState({ cacheKey, - layers, + structuredLyrics, loading: false, + refreshing: false, error: null, retryAt: null, }) }) .catch((error) => { if (!active || error?.name === 'AbortError') return + if (keepingContent && stateRef.current.cacheKey === cacheKey) { + // A failed progressive refresh must not blank working lyrics. + setState((current) => + current.cacheKey === cacheKey + ? { ...current, refreshing: false } + : current, + ) + return + } cache.delete(cacheKey) setState({ cacheKey, - layers: emptyLyricLayers, + structuredLyrics: null, loading: false, + refreshing: false, error, retryAt: (() => { const delay = retryDelayFromError(error) @@ -205,14 +244,7 @@ const useEnhancedLyrics = ({ active = false request.release() } - }, [ - cacheKey, - disabled, - preferredLanguage, - requestGeneration, - requested, - trackId, - ]) + }, [cacheKey, disabled, refreshToken, requestGeneration, requested, trackId]) useEffect(() => { if ( @@ -236,24 +268,48 @@ const useEnhancedLyrics = ({ } }, [cacheKey, disabled, requested, retry, state.cacheKey, state.retryAt]) + const structuredLyrics = + state.cacheKey === cacheKey ? state.structuredLyrics : null + const variants = useMemo( + () => collectLyricVariants(structuredLyrics || []), + [structuredLyrics], + ) + const layers = useMemo( + () => + structuredLyrics && structuredLyrics.length > 0 + ? selectLyricLayers( + structuredLyrics, + preferredLanguage, + selectedSourceKey, + ) + : emptyLyricLayers, + [preferredLanguage, selectedSourceKey, structuredLyrics], + ) + if (state.cacheKey !== cacheKey) { return { + variants: [], layers: emptyLyricLayers, loading: Boolean(trackId && !disabled && requested), + refreshing: false, error: null, retryAfterSeconds: null, retry, + refresh, } } return { - layers: state.layers, + variants, + layers, loading: state.loading, + refreshing: state.refreshing, error: state.error, retryAfterSeconds: state.retryAt == null ? null : Math.max(1, Math.ceil((state.retryAt - Date.now()) / 1_000)), retry, + refresh, } } diff --git a/ui/src/audioplayer/useEnhancedLyrics.test.js b/ui/src/audioplayer/useEnhancedLyrics.test.js index 9e871c873..80ca8e8f9 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.test.js +++ b/ui/src/audioplayer/useEnhancedLyrics.test.js @@ -31,6 +31,22 @@ const responseFor = (value, lang = 'en', source) => ({ }, }) +const responseForEntries = (entries) => ({ + json: { + 'subsonic-response': { + lyricsList: { structuredLyrics: entries }, + }, + }, +}) + +const sourceEntry = (provider, value) => ({ + kind: 'main', + lang: 'en', + synced: true, + line: [{ start: 0, value }], + source: { type: 'plugin', provider, format: 'lrc' }, +}) + const createDeferred = () => { let resolve let reject @@ -229,6 +245,125 @@ describe('useEnhancedLyrics', () => { expect(signal.aborted).toBe(true) }) + it('exposes variants in response order with auto selection', async () => { + subsonic.getLyricsBySongId.mockResolvedValueOnce( + responseForEntries([ + sourceEntry('NetEase', 'NetEase line'), + sourceEntry('LRCLib', 'LRCLib line'), + ]), + ) + + const { result } = renderLyrics({ trackId: 'song-variants' }) + + await expectLyric(result, 'NetEase line') + expect(result.current.variants.map((variant) => variant.key)).toEqual([ + 'netease', + 'lrclib', + ]) + }) + + it('re-derives layers for a selected source without refetching', async () => { + subsonic.getLyricsBySongId.mockResolvedValueOnce( + responseForEntries([ + sourceEntry('NetEase', 'NetEase line'), + sourceEntry('LRCLib', 'LRCLib line'), + ]), + ) + + const { result, rerender } = renderLyrics({ trackId: 'song-select' }) + + await expectLyric(result, 'NetEase line') + + rerender({ trackId: 'song-select', selectedSourceKey: 'lrclib' }) + await expectLyric(result, 'LRCLib line') + + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(1) + }) + + it('falls back to auto when the selected source disappears', async () => { + subsonic.getLyricsBySongId + .mockResolvedValueOnce( + responseForEntries([ + sourceEntry('NetEase', 'NetEase line'), + sourceEntry('LRCLib', 'LRCLib line'), + ]), + ) + .mockResolvedValueOnce( + responseForEntries([sourceEntry('NetEase', 'NetEase line')]), + ) + + const { result } = renderLyrics({ + trackId: 'song-fallback', + selectedSourceKey: 'lrclib', + }) + + await expectLyric(result, 'LRCLib line') + + act(() => { + result.current.refresh() + }) + + await expectLyric(result, 'NetEase line') + expect(result.current.variants.map((variant) => variant.key)).toEqual([ + 'netease', + ]) + expect(result.current.error).toBeNull() + }) + + it('refreshes variants without blanking the current lyrics', async () => { + const nextRequest = createDeferred() + subsonic.getLyricsBySongId + .mockResolvedValueOnce( + responseForEntries([sourceEntry('NetEase', 'NetEase line')]), + ) + .mockReturnValueOnce(nextRequest.promise) + + const { result } = renderLyrics({ trackId: 'song-refresh' }) + + await expectLyric(result, 'NetEase line') + + act(() => { + result.current.refresh() + }) + + await waitFor(() => expect(result.current.refreshing).toBe(true)) + expect(result.current.layers.main.line[0].value).toBe('NetEase line') + + act(() => { + nextRequest.resolve( + responseForEntries([ + sourceEntry('NetEase', 'NetEase line'), + sourceEntry('LRCLib', 'LRCLib line'), + ]), + ) + }) + + await waitFor(() => expect(result.current.variants).toHaveLength(2)) + expect(result.current.refreshing).toBe(false) + expect(result.current.layers.main.line[0].value).toBe('NetEase line') + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(2) + }) + + it('keeps current lyrics when a refresh fails', async () => { + subsonic.getLyricsBySongId + .mockResolvedValueOnce( + responseForEntries([sourceEntry('NetEase', 'NetEase line')]), + ) + .mockRejectedValueOnce(new Error('refresh failed')) + + const { result } = renderLyrics({ trackId: 'song-refresh-fail' }) + + await expectLyric(result, 'NetEase line') + + act(() => { + result.current.refresh() + }) + + await waitFor(() => expect(result.current.refreshing).toBe(false)) + expect(result.current.error).toBeNull() + expect(result.current.layers.main.line[0].value).toBe('NetEase line') + }) + it('starts a fresh request after an abandoned request is aborted', () => { const signals = [] subsonic.getLyricsBySongId.mockImplementation((_trackId, options) => {