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
This commit is contained in:
parent
5e39f1ee09
commit
30c687ae7b
10 changed files with 87 additions and 13 deletions
|
|
@ -61,6 +61,7 @@ function createShellBridge(overrides: Partial<ShellBridge> = {}): ShellBridge {
|
|||
setActivePageChrome: vi.fn(),
|
||||
activateLegacyPath: vi.fn(),
|
||||
navigateToArtistDetail: vi.fn(),
|
||||
cancelSimilarArtistsLoad: vi.fn(),
|
||||
showReactHost: vi.fn(),
|
||||
...overrides,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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<typeof window.SoulSyncWebShellBridge>;
|
||||
|
||||
|
|
@ -38,6 +39,7 @@ describe('waitForShellContext', () => {
|
|||
getCurrentProfileContext,
|
||||
resolveLegacyPath: vi.fn(() => 'issues'),
|
||||
setActivePageChrome: vi.fn(),
|
||||
cancelSimilarArtistsLoad: vi.fn(),
|
||||
showReactHost: vi.fn(),
|
||||
} as NonNullable<typeof window.SoulSyncWebShellBridge>;
|
||||
|
||||
|
|
|
|||
1
webui/src/platform/shell/globals.d.ts
vendored
1
webui/src/platform/shell/globals.d.ts
vendored
|
|
@ -51,6 +51,7 @@ declare global {
|
|||
replace?: boolean;
|
||||
},
|
||||
) => void;
|
||||
cancelSimilarArtistsLoad: () => void;
|
||||
showReactHost: (pageId: ShellPageId) => void;
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ function createShellBridge(overrides: Partial<ShellBridge> = {}): 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<typeof vi.fn>;
|
||||
cancelSimilarArtistsLoad.mockClear();
|
||||
|
||||
unmount();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(cancelSimilarArtistsLoad).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ function createShellBridge(overrides: Partial<ShellBridge> = {}): ShellBridge {
|
|||
setActivePageChrome: vi.fn(),
|
||||
activateLegacyPath: vi.fn(),
|
||||
navigateToArtistDetail: vi.fn(),
|
||||
cancelSimilarArtistsLoad: vi.fn(),
|
||||
showReactHost: vi.fn(),
|
||||
...overrides,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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) => {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}));
|
||||
|
|
|
|||
|
|
@ -167,6 +167,11 @@ window.SoulSyncWebShellBridge = {
|
|||
},
|
||||
navigateToArtistDetailPage,
|
||||
navigateToArtistDetail,
|
||||
cancelSimilarArtistsLoad() {
|
||||
if (typeof cancelSimilarArtistsLoad === 'function') {
|
||||
cancelSimilarArtistsLoad();
|
||||
}
|
||||
},
|
||||
showReactHost(pageId) {
|
||||
showReactHost(pageId);
|
||||
},
|
||||
|
|
|
|||
Loading…
Reference in a new issue