From 30c687ae7bcd28a7521a1d2b573739f42692ac58 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Tue, 19 May 2026 09:28:05 +0300 Subject: [PATCH] refactor(webui): cancel similar-artists in route - expose a shell-bridge cancel primitive for similar-artists loading - stop stale similar-artists streams from the artist-detail route lifecycle - keep the legacy loader abort-only and make abort logs page-agnostic - update bridge and route tests for the new cleanup path --- webui/src/app/router.test.tsx | 1 + webui/src/platform/shell/bridge.test.ts | 2 + webui/src/platform/shell/globals.d.ts | 1 + .../src/routes/artist-detail/$source/$id.tsx | 14 ++++++- .../src/routes/artist-detail/-route.test.tsx | 25 +++++++++++ webui/src/routes/issues/-route.test.tsx | 1 + webui/static/core.js | 7 ++++ webui/static/library.js | 3 ++ webui/static/shared-helpers.js | 41 ++++++++++++++----- webui/static/shell-bridge.js | 5 +++ 10 files changed, 87 insertions(+), 13 deletions(-) diff --git a/webui/src/app/router.test.tsx b/webui/src/app/router.test.tsx index 7f1921f9..63295b4b 100644 --- a/webui/src/app/router.test.tsx +++ b/webui/src/app/router.test.tsx @@ -61,6 +61,7 @@ function createShellBridge(overrides: Partial = {}): ShellBridge { setActivePageChrome: vi.fn(), activateLegacyPath: vi.fn(), navigateToArtistDetail: vi.fn(), + cancelSimilarArtistsLoad: vi.fn(), showReactHost: vi.fn(), ...overrides, }; diff --git a/webui/src/platform/shell/bridge.test.ts b/webui/src/platform/shell/bridge.test.ts index a33f44cd..9682bbe7 100644 --- a/webui/src/platform/shell/bridge.test.ts +++ b/webui/src/platform/shell/bridge.test.ts @@ -17,6 +17,7 @@ describe('waitForShellContext', () => { getCurrentProfileContext: vi.fn(() => ({ profileId: 2, isAdmin: true })), resolveLegacyPath: vi.fn(() => 'issues'), setActivePageChrome: vi.fn(), + cancelSimilarArtistsLoad: vi.fn(), showReactHost: vi.fn(), } as NonNullable; @@ -38,6 +39,7 @@ describe('waitForShellContext', () => { getCurrentProfileContext, resolveLegacyPath: vi.fn(() => 'issues'), setActivePageChrome: vi.fn(), + cancelSimilarArtistsLoad: vi.fn(), showReactHost: vi.fn(), } as NonNullable; diff --git a/webui/src/platform/shell/globals.d.ts b/webui/src/platform/shell/globals.d.ts index 82915a63..2db2c644 100644 --- a/webui/src/platform/shell/globals.d.ts +++ b/webui/src/platform/shell/globals.d.ts @@ -51,6 +51,7 @@ declare global { replace?: boolean; }, ) => void; + cancelSimilarArtistsLoad: () => void; showReactHost: (pageId: ShellPageId) => void; }; } diff --git a/webui/src/routes/artist-detail/$source/$id.tsx b/webui/src/routes/artist-detail/$source/$id.tsx index a3348a82..5430d2c3 100644 --- a/webui/src/routes/artist-detail/$source/$id.tsx +++ b/webui/src/routes/artist-detail/$source/$id.tsx @@ -1,5 +1,5 @@ import { createFileRoute } from '@tanstack/react-router'; -import { useLayoutEffect } from 'react'; +import { useEffect, useLayoutEffect } from 'react'; import { useShellBridge } from '@/platform/shell/route-controllers'; @@ -8,7 +8,8 @@ export const Route = createFileRoute('/artist-detail/$source/$id')({ }); // Thin legacy handoff: TanStack owns the URL shape here, but the vanilla JS -// artist-detail page still renders the actual experience for now. +// artist-detail page still renders the actual experience for now. The route +// owns cancellation so similar-artist loading stops when this page changes. function ArtistDetailPage() { const bridge = useShellBridge(); const { source, id } = Route.useParams(); @@ -16,11 +17,20 @@ function ArtistDetailPage() { useLayoutEffect(() => { if (!bridge) return; + bridge.cancelSimilarArtistsLoad(); const normalizedSource = source.toLowerCase() === 'library' ? null : source.toLowerCase(); bridge.navigateToArtistDetail(id, '', normalizedSource, { skipRouteChange: true, }); }, [bridge, id, source]); + useEffect(() => { + if (!bridge) return; + + return () => { + bridge.cancelSimilarArtistsLoad(); + }; + }, [bridge, id, source]); + return null; } diff --git a/webui/src/routes/artist-detail/-route.test.tsx b/webui/src/routes/artist-detail/-route.test.tsx index 92ac09eb..232ff944 100644 --- a/webui/src/routes/artist-detail/-route.test.tsx +++ b/webui/src/routes/artist-detail/-route.test.tsx @@ -16,6 +16,7 @@ function createShellBridge(overrides: Partial = {}): ShellBridge { setActivePageChrome: vi.fn(), activateLegacyPath: vi.fn(), navigateToArtistDetail: vi.fn(), + cancelSimilarArtistsLoad: vi.fn(), showReactHost: vi.fn(), ...overrides, }; @@ -71,4 +72,28 @@ describe('artist-detail route', () => { ); }); }); + + it('cancels the similar artists stream when the route unmounts', async () => { + const { unmount } = renderArtistDetailRoute(['/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg']); + + await waitFor(() => { + expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith( + '2YZyLoL8N0Wb9xBt1NhZWg', + '', + 'spotify', + { + skipRouteChange: true, + }, + ); + }); + + const cancelSimilarArtistsLoad = window.SoulSyncWebShellBridge?.cancelSimilarArtistsLoad as ReturnType; + cancelSimilarArtistsLoad.mockClear(); + + unmount(); + + await waitFor(() => { + expect(cancelSimilarArtistsLoad).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/webui/src/routes/issues/-route.test.tsx b/webui/src/routes/issues/-route.test.tsx index 547745ef..7bbc0353 100644 --- a/webui/src/routes/issues/-route.test.tsx +++ b/webui/src/routes/issues/-route.test.tsx @@ -23,6 +23,7 @@ function createShellBridge(overrides: Partial = {}): ShellBridge { setActivePageChrome: vi.fn(), activateLegacyPath: vi.fn(), navigateToArtistDetail: vi.fn(), + cancelSimilarArtistsLoad: vi.fn(), showReactHost: vi.fn(), ...overrides, }; diff --git a/webui/static/core.js b/webui/static/core.js index 426db439..f1b047f6 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -201,6 +201,13 @@ let artistsSearchController = null; let artistCompletionController = null; // Track ongoing completion check to cancel when navigating away let similarArtistsController = null; // Track ongoing similar artists stream to cancel when navigating away +function cancelSimilarArtistsLoad() { + if (similarArtistsController) { + similarArtistsController.abort(); + similarArtistsController = null; + } +} + // --- Lazy Background Image Observer --- // Watches elements with data-bg-src, applies background-image when visible, unobserves after. const lazyBgObserver = new IntersectionObserver((entries) => { diff --git a/webui/static/library.js b/webui/static/library.js index f96a2bee..0ef9f1f2 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -1192,6 +1192,9 @@ function populateArtistDetailPage(data) { // MusicMap name lookup). Fire-and-forget — the function handles its own // loading state and errors. if (artist && artist.name && typeof loadSimilarArtists === 'function') { + if (typeof cancelSimilarArtistsLoad === 'function') { + cancelSimilarArtistsLoad(); + } loadSimilarArtists(artist.name); } } diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 85957c65..bbc9ca0c 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -695,7 +695,7 @@ async function checkDiscographyCompletion(artistId, discography) { } catch (error) { // Don't show error if it was aborted (user navigated away) if (error.name === 'AbortError') { - console.log('⏹️ Completion check aborted (user navigated to new artist)'); + console.log('⏹️ Completion check aborted (user navigated away)'); return; } @@ -3574,16 +3574,21 @@ async function loadSimilarArtists(artistName) { container.innerHTML = ''; section.style.display = 'block'; + let controller = null; + let signal = null; + try { // Create new abort controller for this similar artists stream - similarArtistsController = new AbortController(); + controller = new AbortController(); + similarArtistsController = controller; + signal = controller.signal; // Use streaming endpoint for real-time bubble creation const url = `/api/artist/similar/${encodeURIComponent(artistName)}/stream`; console.log(`📡 Streaming from: ${url}`); const response = await fetch(url, { - signal: similarArtistsController.signal + signal }); if (!response.ok) { @@ -3612,6 +3617,7 @@ async function loadSimilarArtists(artistName) { buffer = messages.pop() || ''; // Keep incomplete message in buffer for (const message of messages) { + if (signal?.aborted) return; if (!message.trim() || !message.startsWith('data: ')) continue; try { @@ -3622,6 +3628,7 @@ async function loadSimilarArtists(artistName) { } if (jsonData.artist) { + if (signal?.aborted) return; // Hide loading on first artist if (artistCount === 0) { loadingEl.classList.add('hidden'); @@ -3636,6 +3643,7 @@ async function loadSimilarArtists(artistName) { } if (jsonData.complete) { + if (signal?.aborted) return; console.log(`🎉 Streaming complete: ${jsonData.total} artists`); if (artistCount === 0) { @@ -3648,7 +3656,7 @@ async function loadSimilarArtists(artistName) { `; } else { // Lazy load images for similar artists that don't have them - lazyLoadSimilarArtistImages(container); + await lazyLoadSimilarArtistImages(container, signal); } } } catch (parseError) { @@ -3658,13 +3666,14 @@ async function loadSimilarArtists(artistName) { } // Clear the controller when done - similarArtistsController = null; + if (similarArtistsController === controller) { + similarArtistsController = null; + } } catch (error) { // Don't show error if it was aborted (user navigated away) - if (error.name === 'AbortError') { - console.log('⏹️ Similar artists stream aborted (user navigated to new artist)'); - loadingEl.classList.add('hidden'); + if (error.name === 'AbortError' || signal?.aborted) { + console.log('⏹️ Similar artists stream aborted (user navigated away)'); return; } @@ -3683,15 +3692,18 @@ async function loadSimilarArtists(artistName) { `; } finally { // Always clear the controller - similarArtistsController = null; + if (similarArtistsController === controller) { + similarArtistsController = null; + } } } /** * Lazy load images for similar artist bubbles that don't have images */ -async function lazyLoadSimilarArtistImages(container) { +async function lazyLoadSimilarArtistImages(container, signal) { if (!container) return; + if (signal?.aborted) return; const bubblesNeedingImages = container.querySelectorAll('.similar-artist-bubble[data-needs-image="true"]'); @@ -3707,9 +3719,11 @@ async function lazyLoadSimilarArtistImages(container) { const bubbles = Array.from(bubblesNeedingImages); for (let i = 0; i < bubbles.length; i += batchSize) { + if (signal?.aborted) return; const batch = bubbles.slice(i, i + batchSize); await Promise.all(batch.map(async (bubble) => { + if (signal?.aborted) return; const artistId = bubble.getAttribute('data-artist-id'); const artistSource = bubble.getAttribute('data-artist-source') || ''; const artistPlugin = bubble.getAttribute('data-artist-plugin') || ''; @@ -3724,9 +3738,11 @@ async function lazyLoadSimilarArtistImages(container) { ? `/api/artist/${encodeURIComponent(artistId)}/image?${params.toString()}` : `/api/artist/${encodeURIComponent(artistId)}/image`; - const response = await fetch(imageUrl); + const response = await fetch(imageUrl, { signal }); const data = await response.json(); + if (signal?.aborted) return; + if (data.success && data.image_url) { const imageContainer = bubble.querySelector('.similar-artist-bubble-image'); if (imageContainer) { @@ -3737,6 +3753,9 @@ async function lazyLoadSimilarArtistImages(container) { } } } catch (error) { + if (error?.name === 'AbortError' || signal?.aborted) { + return; + } console.warn(`⚠️ Failed to load image for similar artist ${artistId}:`, error); } })); diff --git a/webui/static/shell-bridge.js b/webui/static/shell-bridge.js index 9eb4b04e..16167032 100644 --- a/webui/static/shell-bridge.js +++ b/webui/static/shell-bridge.js @@ -167,6 +167,11 @@ window.SoulSyncWebShellBridge = { }, navigateToArtistDetailPage, navigateToArtistDetail, + cancelSimilarArtistsLoad() { + if (typeof cancelSimilarArtistsLoad === 'function') { + cancelSimilarArtistsLoad(); + } + }, showReactHost(pageId) { showReactHost(pageId); },