fix(ui): cancel in-flight image requests on pagination, cache across remounts (#5249)

* feat(ui): cancel in-flight image requests on pagination and cache across remounts

When paginating quickly through list/grid views, image requests for previous
pages were never canceled, queuing on the server and blocking new images.
This adds a useImageUrl hook that loads images via fetch() with AbortController,
so requests are canceled when components unmount. A module-level cache (URL →
blob URL) with reference counting ensures React Admin refreshes display images
instantly without re-fetching.

* feat(ui): update AlbumListPagination to conditionally render based on albumListType

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(ui): abort all in-flight image fetches on pagination change

Pagination component now watches page/perPage via useListContext and
calls abortAllInFlight() when either changes, freeing the browser
connection pool immediately for the next page's data request.

Also adds empty placeholder style to CoverArtAvatar so it renders as a
clean transparent area while loading instead of the default person icon.

* Revert "feat(ui): abort all in-flight image fetches on pagination change"

This reverts commit 3bc09f9d0374aa63572a381e38a30e2f2cec4da8.

* fix(ui): limit concurrent image fetches to prevent connection pool saturation

With <img src>, the browser prioritizes API requests over image loads.
With fetch(), all requests compete equally for the HTTP/1.1 connection
pool (6 per origin), causing API requests to queue behind images and
making pagination feel unresponsive. Caps concurrent image fetches at
4 with a pending queue, leaving connections free for API requests.
Queued fetches for unmounted components are removed without ever
hitting the network.

* fix(ui): fix queued fetch not aborted on unmount

Set queued=false when doFetch executes from the pending queue, so
cleanup correctly calls controller.abort() instead of searching an
already-drained queue.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan Quintão
2026-03-25 21:30:40 -04:00
committed by GitHub
parent 4c91936848
commit 33e20d355e
7 changed files with 423 additions and 35 deletions
+5 -22
View File
@@ -18,6 +18,7 @@ import {
PlayButton, PlayButton,
ArtistLinkField, ArtistLinkField,
OverflowTooltip, OverflowTooltip,
useImageUrl,
} from '../common' } from '../common'
import { COVER_ART_SIZE, DraggableTypes } from '../consts' import { COVER_ART_SIZE, DraggableTypes } from '../consts'
import clsx from 'clsx' import clsx from 'clsx'
@@ -105,7 +106,7 @@ const useCoverStyles = makeStyles({
transition: 'opacity 0.3s ease-in-out', transition: 'opacity 0.3s ease-in-out',
}, },
coverLoading: { coverLoading: {
opacity: 0.5, opacity: 0,
}, },
}) })
@@ -125,8 +126,6 @@ const Cover = withContentRect('bounds')(({
// Force height to be the same as the width determined by the GridList // Force height to be the same as the width determined by the GridList
// noinspection JSSuspiciousNameCombination // noinspection JSSuspiciousNameCombination
const classes = useCoverStyles({ height: contentRect.bounds.width }) const classes = useCoverStyles({ height: contentRect.bounds.width })
const [imageLoading, setImageLoading] = React.useState(true)
const [imageError, setImageError] = React.useState(false)
const [, dragAlbumRef] = useDrag( const [, dragAlbumRef] = useDrag(
() => ({ () => ({
type: DraggableTypes.ALBUM, type: DraggableTypes.ALBUM,
@@ -136,32 +135,16 @@ const Cover = withContentRect('bounds')(({
[record], [record],
) )
// Reset image state when record changes const url = subsonic.getCoverArtUrl(record, COVER_ART_SIZE, true)
React.useEffect(() => { const { imgUrl, loading: imageLoading } = useImageUrl(url)
setImageLoading(true)
setImageError(false)
}, [record.id])
const handleImageLoad = React.useCallback(() => {
setImageLoading(false)
setImageError(false)
}, [])
const handleImageError = React.useCallback(() => {
setImageLoading(false)
setImageError(true)
}, [])
return ( return (
<div ref={measureRef} className={classes.coverContainer}> <div ref={measureRef} className={classes.coverContainer}>
<div ref={dragAlbumRef}> <div ref={dragAlbumRef}>
<img <img
key={record.id} // Force re-render when record changes src={imgUrl || undefined}
src={subsonic.getCoverArtUrl(record, COVER_ART_SIZE, true)}
alt={record.name} alt={record.name}
className={`${classes.cover} ${imageLoading ? classes.coverLoading : ''}`} className={`${classes.cover} ${imageLoading ? classes.coverLoading : ''}`}
onLoad={handleImageLoad}
onError={handleImageError}
/> />
</div> </div>
</div> </div>
+9 -4
View File
@@ -175,12 +175,12 @@ const AlbumListTitle = ({ albumListType }) => {
return <Title subTitle={title} args={{ smart_count: 2 }} /> return <Title subTitle={title} args={{ smart_count: 2 }} />
} }
const AlbumListPagination = (props) => { const AlbumListPagination = ({ albumListType, ...rest }) => {
const { loading } = useListContext() const { loading } = useListContext()
if (loading) { if (loading && albumListType === 'random') {
return null return null
} }
return <Pagination {...props} /> return <Pagination {...rest} />
} }
const randomStartingSeed = Math.random().toString() const randomStartingSeed = Math.random().toString()
@@ -243,7 +243,12 @@ const AlbumList = (props) => {
actions={<AlbumListActions />} actions={<AlbumListActions />}
filters={<AlbumFilter />} filters={<AlbumFilter />}
perPage={perPage} perPage={perPage}
pagination={<AlbumListPagination rowsPerPageOptions={perPageOptions} />} pagination={
<AlbumListPagination
rowsPerPageOptions={perPageOptions}
albumListType={albumListType}
/>
}
title={<AlbumListTitle albumListType={albumListType} />} title={<AlbumListTitle albumListType={albumListType} />}
> >
{albumView.grid ? ( {albumView.grid ? (
+19 -4
View File
@@ -4,12 +4,16 @@ import { makeStyles } from '@material-ui/core/styles'
import clsx from 'clsx' import clsx from 'clsx'
import { COVER_ART_SIZE } from '../consts' import { COVER_ART_SIZE } from '../consts'
import subsonic from '../subsonic' import subsonic from '../subsonic'
import { useImageUrl } from './useImageUrl'
const useStyles = makeStyles({ const useStyles = makeStyles({
avatar: { avatar: {
width: '55px', width: '55px',
height: '55px', height: '55px',
}, },
avatarEmpty: {
backgroundColor: 'transparent',
},
square: { square: {
borderRadius: '4px', borderRadius: '4px',
}, },
@@ -22,15 +26,26 @@ export const CoverArtAvatar = ({
const classes = useStyles() const classes = useStyles()
const recordContext = useRecordContext() const recordContext = useRecordContext()
const record = recordProp || recordContext const record = recordProp || recordContext
if (!record) return null
const square = variant !== 'circular' const square = variant !== 'circular'
const url = record
? subsonic.getCoverArtUrl(record, COVER_ART_SIZE, square)
: null
const { imgUrl } = useImageUrl(url)
if (!record) return null
return ( return (
<Avatar <Avatar
src={subsonic.getCoverArtUrl(record, COVER_ART_SIZE, square)} src={imgUrl || undefined}
variant={variant} variant={variant}
className={clsx(classes.avatar, square && classes.square)} className={clsx(
classes.avatar,
square && classes.square,
!imgUrl && classes.avatarEmpty,
)}
alt={record.name} alt={record.name}
/> >
{/* Empty child prevents default person icon while loading */}
{!imgUrl && <span />}
</Avatar>
) )
} }
+1
View File
@@ -46,3 +46,4 @@ export * from './useSearchRefocus'
export * from './ImageUploadOverlay' export * from './ImageUploadOverlay'
export * from './CoverArtAvatar' export * from './CoverArtAvatar'
export * from './useImageLoadingState' export * from './useImageLoadingState'
export * from './useImageUrl'
+144
View File
@@ -0,0 +1,144 @@
import { useEffect, useState, useRef } from 'react'
// Persists across component mount/unmount cycles so that
// React Admin refreshes (which remount list items) don't re-fetch images.
const cache = new Map()
const MAX_CACHE_SIZE = 300
// Limit concurrent fetches to leave browser connections free for API requests.
// Browsers allow ~6 connections per origin on HTTP/1.1; reserving 2 for API
// calls prevents image fetches from blocking pagination/data requests.
const MAX_CONCURRENT = 4
let activeFetches = 0
const pendingQueue = []
const processQueue = () => {
while (pendingQueue.length > 0 && activeFetches < MAX_CONCURRENT) {
const next = pendingQueue.shift()
next()
}
}
// Evicts oldest unused entries (Map iterates in insertion order).
const evictIfNeeded = () => {
if (cache.size <= MAX_CACHE_SIZE) return
for (const [key, entry] of cache) {
if (cache.size <= MAX_CACHE_SIZE) break
if (entry.refCount === 0) {
if (entry.blobUrl) URL.revokeObjectURL(entry.blobUrl)
cache.delete(key)
}
}
}
/**
* Loads an image via fetch() with AbortController so that in-flight requests
* are canceled on unmount (e.g., during pagination). Uses a module-level cache
* so remounting returns the cached blob URL instantly.
*/
export const useImageUrl = (url) => {
const cached = url ? cache.get(url) : null
const [imgUrl, setImgUrl] = useState(cached?.blobUrl || null)
const [loading, setLoading] = useState(!!url && !cached)
const [error, setError] = useState(cached?.error || false)
const abortedRef = useRef(false)
useEffect(() => {
abortedRef.current = false
if (!url) {
setImgUrl(null)
setLoading(false)
setError(false)
return
}
// Re-check: another component's effect may have populated the cache
// between this component's render and effect execution.
const entry = cache.get(url)
if (entry) {
entry.refCount++
setImgUrl(entry.blobUrl)
setLoading(false)
setError(entry.error || false)
return () => {
entry.refCount--
}
}
const controller = new AbortController()
let queued = true
setImgUrl(null)
setLoading(true)
setError(false)
const doFetch = () => {
queued = false
activeFetches++
fetch(url, { signal: controller.signal })
.then((res) => {
if (!res.ok) {
throw new Error(`HTTP ${res.status}`)
}
return res.blob()
})
.then((blob) => {
activeFetches--
processQueue()
// Guard against late resolution after abort
if (abortedRef.current) {
return
}
const objectUrl = URL.createObjectURL(blob)
// Handle concurrent fetches: if another component already cached
// this URL, use its entry and discard our blob.
const existing = cache.get(url)
if (existing && existing.blobUrl) {
existing.refCount++
URL.revokeObjectURL(objectUrl)
setImgUrl(existing.blobUrl)
} else {
cache.set(url, { blobUrl: objectUrl, refCount: 1 })
evictIfNeeded()
setImgUrl(objectUrl)
}
setLoading(false)
})
.catch((err) => {
activeFetches--
processQueue()
if (err.name === 'AbortError') {
return // Expected on unmount or URL change
}
// Cache the error so repeated mounts don't re-fetch broken URLs
cache.set(url, { blobUrl: null, error: true, refCount: 0 })
setError(true)
setLoading(false)
})
}
if (activeFetches < MAX_CONCURRENT) {
queued = false
doFetch()
} else {
pendingQueue.push(doFetch)
}
return () => {
abortedRef.current = true
if (queued) {
// Remove from queue if not yet started
const idx = pendingQueue.indexOf(doFetch)
if (idx !== -1) pendingQueue.splice(idx, 1)
} else {
controller.abort()
}
const entry = cache.get(url)
if (entry) {
entry.refCount--
}
}
}, [url])
return { imgUrl, loading, error }
}
+234
View File
@@ -0,0 +1,234 @@
import { renderHook, act } from '@testing-library/react-hooks'
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'
// Helper to flush all pending promises
const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0))
// We need a fresh module for each test to reset the module-level cache
let useImageUrl
describe('useImageUrl', () => {
let abortSpy
let OriginalAbortController
let originalCreateObjectURL
let originalRevokeObjectURL
let originalFetch
beforeEach(async () => {
// Reset module to clear the cache
vi.resetModules()
const mod = await import('./useImageUrl')
useImageUrl = mod.useImageUrl
abortSpy = vi.fn()
OriginalAbortController = global.AbortController
originalCreateObjectURL = global.URL.createObjectURL
originalRevokeObjectURL = global.URL.revokeObjectURL
originalFetch = global.fetch
global.AbortController = function () {
this.signal = 'mock-signal'
this.abort = abortSpy
}
global.URL.createObjectURL = vi.fn(() => 'blob:mock-url')
global.URL.revokeObjectURL = vi.fn()
})
afterEach(() => {
global.AbortController = OriginalAbortController
global.URL.createObjectURL = originalCreateObjectURL
global.URL.revokeObjectURL = originalRevokeObjectURL
global.fetch = originalFetch
vi.restoreAllMocks()
})
it('should return null values when url is null', () => {
const { result } = renderHook(() => useImageUrl(null))
expect(result.current.loading).toBe(false)
expect(result.current.imgUrl).toBeNull()
expect(result.current.error).toBe(false)
})
it('should return loading state initially', () => {
global.fetch = vi.fn(() => new Promise(() => {}))
const { result } = renderHook(() =>
useImageUrl('http://example.com/img.jpg'),
)
expect(result.current.loading).toBe(true)
expect(result.current.imgUrl).toBeNull()
expect(result.current.error).toBe(false)
})
it('should fetch image and return blob URL on success', async () => {
const mockBlob = new Blob(['image-data'], { type: 'image/png' })
global.fetch = vi.fn(() =>
Promise.resolve({
ok: true,
blob: () => Promise.resolve(mockBlob),
}),
)
const { result } = renderHook(() =>
useImageUrl('http://example.com/img.jpg'),
)
await act(async () => {
await flushPromises()
})
expect(result.current.loading).toBe(false)
expect(result.current.imgUrl).toBe('blob:mock-url')
expect(result.current.error).toBe(false)
expect(global.fetch).toHaveBeenCalledWith('http://example.com/img.jpg', {
signal: 'mock-signal',
})
})
it('should set error on HTTP failure', async () => {
global.fetch = vi.fn(() => Promise.resolve({ ok: false, status: 404 }))
const { result } = renderHook(() =>
useImageUrl('http://example.com/missing.jpg'),
)
await act(async () => {
await flushPromises()
})
expect(result.current.loading).toBe(false)
expect(result.current.imgUrl).toBeNull()
expect(result.current.error).toBe(true)
})
it('should abort fetch on unmount', async () => {
global.fetch = vi.fn(() => new Promise(() => {}))
const { unmount } = renderHook(() =>
useImageUrl('http://example.com/img.jpg'),
)
await act(async () => {
await flushPromises()
})
unmount()
expect(abortSpy).toHaveBeenCalled()
})
it('should abort previous fetch when URL changes', async () => {
const abortSpies = []
global.AbortController = function () {
const spy = vi.fn()
abortSpies.push(spy)
this.signal = `signal-${abortSpies.length}`
this.abort = spy
}
const mockBlob = new Blob(['data'], { type: 'image/png' })
global.fetch = vi.fn(() =>
Promise.resolve({
ok: true,
blob: () => Promise.resolve(mockBlob),
}),
)
const { rerender } = renderHook(({ url }) => useImageUrl(url), {
initialProps: { url: 'http://example.com/img1.jpg' },
})
await act(async () => {
await flushPromises()
})
// Change URL - should abort the first controller
rerender({ url: 'http://example.com/img2.jpg' })
expect(abortSpies[0]).toHaveBeenCalled()
})
it('should not set error on AbortError', async () => {
const abortError = new DOMException('Aborted', 'AbortError')
global.fetch = vi.fn(() => Promise.reject(abortError))
const { result } = renderHook(() =>
useImageUrl('http://example.com/img.jpg'),
)
await act(async () => {
await flushPromises()
})
expect(result.current.error).toBe(false)
})
it('should use cached blob URL on remount without re-fetching', async () => {
const mockBlob = new Blob(['data'], { type: 'image/png' })
global.fetch = vi.fn(() =>
Promise.resolve({
ok: true,
blob: () => Promise.resolve(mockBlob),
}),
)
// First mount — fetches and caches
const { unmount } = renderHook(() =>
useImageUrl('http://example.com/img.jpg'),
)
await act(async () => {
await flushPromises()
})
expect(global.fetch).toHaveBeenCalledTimes(1)
// Unmount (simulates React Admin refresh)
unmount()
// Remount with same URL — should use cache
const { result: result2 } = renderHook(() =>
useImageUrl('http://example.com/img.jpg'),
)
await act(async () => {
await flushPromises()
})
// Should NOT have fetched again
expect(global.fetch).toHaveBeenCalledTimes(1)
expect(result2.current.imgUrl).toBe('blob:mock-url')
expect(result2.current.loading).toBe(false)
})
it('should cache errors and not re-fetch broken URLs', async () => {
global.fetch = vi.fn(() => Promise.resolve({ ok: false, status: 404 }))
// First mount — fetch fails and error is cached
const { unmount } = renderHook(() =>
useImageUrl('http://example.com/broken.jpg'),
)
await act(async () => {
await flushPromises()
})
expect(global.fetch).toHaveBeenCalledTimes(1)
unmount()
// Remount with same URL — should use cached error, not re-fetch
const { result: result2 } = renderHook(() =>
useImageUrl('http://example.com/broken.jpg'),
)
await act(async () => {
await flushPromises()
})
expect(global.fetch).toHaveBeenCalledTimes(1)
expect(result2.current.error).toBe(true)
expect(result2.current.imgUrl).toBeNull()
expect(result2.current.loading).toBe(false)
})
})
+11 -5
View File
@@ -14,8 +14,12 @@ import {
UrlField, UrlField,
useTranslate, useTranslate,
} from 'react-admin' } from 'react-admin'
import { List } from '../common' import {
import { ToggleFieldsMenu, useSelectedFields } from '../common' List,
useImageUrl,
ToggleFieldsMenu,
useSelectedFields,
} from '../common'
import subsonic from '../subsonic' import subsonic from '../subsonic'
import { StreamField } from './StreamField' import { StreamField } from './StreamField'
import { setTrack } from '../actions' import { setTrack } from '../actions'
@@ -78,10 +82,12 @@ const RadioListActions = ({
const avatarStyle = { width: 40, height: 40 } const avatarStyle = { width: 40, height: 40 }
const CoverArtField = ({ record }) => { const CoverArtField = ({ record }) => {
if (!record) return null const directUrl = record?.uploadedImage
const src = record.uploadedImage
? subsonic.getCoverArtUrl(record, 40, true) ? subsonic.getCoverArtUrl(record, 40, true)
: RADIO_PLACEHOLDER_IMAGE : null
const { imgUrl } = useImageUrl(directUrl)
if (!record) return null
const src = imgUrl || RADIO_PLACEHOLDER_IMAGE
return ( return (
<Avatar src={src} variant="rounded" style={avatarStyle} alt={record.name} /> <Avatar src={src} variant="rounded" style={avatarStyle} alt={record.name} />
) )