fix artist image render error

separate cache state into its own slice

app/components/ContextMenu.tsx

@@ -118,7 +118,17 @@ const MenuHeader = React.memo<{
   subtitle?: string
 }>(({ coverArt, artistId, title, subtitle }) => (
   <View style={styles.menuHeader}>
-    <CoverArt artistId={artistId} coverArt={coverArt} style={styles.coverArt} resizeMode={FastImage.resizeMode.cover} />
+    {artistId ? (
+      <CoverArt
+        type="artist"
+        artistId={artistId}
+        style={styles.coverArt}
+        resizeMode={FastImage.resizeMode.cover}
+        round={true}
+      />
+    ) : (
+      <CoverArt type="cover" coverArt={coverArt} style={styles.coverArt} resizeMode={FastImage.resizeMode.cover} />
+    )}
     <View style={styles.menuHeaderText}>
       <Text numberOfLines={1} style={styles.menuTitle}>
         {title}

app/components/CoverArt.tsx

@@ -1,7 +1,7 @@
 import { useArtistCoverArtFile, useCoverArtFile } from '@app/hooks/music'
-import { DownloadFile } from '@app/state/music'
+import { DownloadFile } from '@app/state/cache'
 import colors from '@app/styles/colors'
-import React, { useCallback, useEffect, useState } from 'react'
+import React, { useState } from 'react'
 import { ActivityIndicator, StyleSheet, View, ViewStyle } from 'react-native'
 import FastImage, { ImageStyle } from 'react-native-fast-image'
 
@@ -22,16 +22,15 @@ type CoverArtProps = BaseProps & {
   coverArt?: string
 }
 
-const Image: React.FC<{ file?: DownloadFile } & BaseProps> = ({ file, style, imageStyle, resizeMode }) => {
-  const [source, setSource] = useState<number | { uri: string }>(
-    file && file.progress === 1 ? { uri: `file://${file.path}` } : require('@res/fallback.png'),
-  )
+const Image = React.memo<{ file?: DownloadFile } & BaseProps>(({ file, style, imageStyle, resizeMode }) => {
+  const [error, setError] = useState(false)
 
-  useEffect(() => {
-    if (file && file.progress === 1) {
-      setSource({ uri: `file://${file.path}` })
-    }
-  }, [file])
+  let source
+  if (!error && file && file.progress === 1) {
+    source = { uri: `file://${file.path}` }
+  } else {
+    source = require('@res/fallback.png')
+  }
 
   return (
     <>
@@ -39,9 +38,7 @@ const Image: React.FC<{ file?: DownloadFile } & BaseProps> = ({ file, style, ima
         source={source}
         resizeMode={resizeMode || FastImage.resizeMode.contain}
         style={[{ height: style?.height, width: style?.width }, imageStyle]}
-        onError={() => {
-          setSource(require('@res/fallback.png'))
-        }}
+        onError={() => setError(true)}
       />
       <ActivityIndicator
         animating={file && file.progress < 1}
@@ -51,7 +48,7 @@ const Image: React.FC<{ file?: DownloadFile } & BaseProps> = ({ file, style, ima
       />
     </>
   )
-}
+})
 
 const ArtistImage = React.memo<ArtistCoverArtProps>(props => {
   const file = useArtistCoverArtFile(props.artistId)
@@ -65,31 +62,24 @@ const CoverArtImage = React.memo<CoverArtProps>(props => {
   return <Image file={file} {...props} />
 })
 
-const CoverArt: React.FC<CoverArtProps | ArtistCoverArtProps> = props => {
+const CoverArt = React.memo<CoverArtProps | ArtistCoverArtProps>(props => {
   const viewStyles = [props.style]
   if (props.round) {
     viewStyles.push(styles.round)
   }
 
-  const coverArtImage = useCallback(() => <CoverArtImage {...(props as CoverArtProps)} />, [props])
-  const artistImage = useCallback(() => <ArtistImage {...(props as ArtistCoverArtProps)} />, [props])
-
-  let ImageComponent
+  let imageComponent
   switch (props.type) {
     case 'artist':
-      ImageComponent = artistImage
+      imageComponent = <ArtistImage {...(props as ArtistCoverArtProps)} />
       break
     default:
-      ImageComponent = coverArtImage
+      imageComponent = <CoverArtImage {...(props as CoverArtProps)} />
       break
   }
 
-  return (
-    <View style={viewStyles}>
-      <ImageComponent />
-    </View>
-  )
-}
+  return <View style={viewStyles}>{imageComponent}</View>
+})
 
 const styles = StyleSheet.create({
   round: {
@@ -103,4 +93,4 @@ const styles = StyleSheet.create({
   },
 })
 
-export default React.memo(CoverArt)
+export default CoverArt

app/hooks/music.ts

@@ -1,15 +1,17 @@
+import { selectCache } from '@app/state/cache'
 import { selectMusic } from '@app/state/music'
 import { Store, useStore } from '@app/state/store'
-import { useCallback } from 'react'
+import { useCallback, useEffect } from 'react'
 
 export const useArtistInfo = (id: string) => {
   const artistInfo = useStore(useCallback((state: Store) => state.artistInfo[id], [id]))
   const fetchArtistInfo = useStore(selectMusic.fetchArtistInfo)
 
-  if (!artistInfo) {
-    fetchArtistInfo(id)
-    return undefined
-  }
+  useEffect(() => {
+    if (!artistInfo) {
+      fetchArtistInfo(id)
+    }
+  })
 
   return artistInfo
 }
@@ -18,9 +20,11 @@ export const useAlbumWithSongs = (id: string) => {
   const album = useStore(useCallback((state: Store) => state.albumsWithSongs[id], [id]))
   const fetchAlbum = useStore(selectMusic.fetchAlbumWithSongs)
 
-  if (!album) {
-    fetchAlbum(id)
-  }
+  useEffect(() => {
+    if (!album) {
+      fetchAlbum(id)
+    }
+  })
 
   return album
 }
@@ -29,9 +33,11 @@ export const usePlaylistWithSongs = (id: string) => {
   const playlist = useStore(useCallback((state: Store) => state.playlistsWithSongs[id], [id]))
   const fetchPlaylist = useStore(selectMusic.fetchPlaylistWithSongs)
 
-  if (!playlist) {
-    fetchPlaylist(id)
-  }
+  useEffect(() => {
+    if (!playlist) {
+      fetchPlaylist(id)
+    }
+  })
 
   return playlist
 }
@@ -58,12 +64,13 @@ export const useStarred = (id: string, type: string) => {
 
 export const useCoverArtFile = (coverArt: string = '-1') => {
   const file = useStore(useCallback((state: Store) => state.cachedCoverArt[coverArt], [coverArt]))
-  const cacheCoverArt = useStore(selectMusic.cacheCoverArt)
+  const cacheCoverArt = useStore(selectCache.cacheCoverArt)
 
-  if (!file) {
-    cacheCoverArt(coverArt)
-    return undefined
-  }
+  useEffect(() => {
+    if (!file) {
+      cacheCoverArt(coverArt)
+    }
+  })
 
   return file
 }
@@ -71,16 +78,13 @@ export const useCoverArtFile = (coverArt: string = '-1') => {
 export const useArtistCoverArtFile = (artistId: string) => {
   const artistInfo = useArtistInfo(artistId)
   const file = useStore(useCallback((state: Store) => state.cachedArtistArt[artistId], [artistId]))
-  const cacheArtistArt = useStore(selectMusic.cacheArtistArt)
+  const cacheArtistArt = useStore(selectCache.cacheArtistArt)
 
-  if (!artistInfo) {
-    return undefined
-  }
-
-  if (!file) {
-    cacheArtistArt(artistId, artistInfo.largeImageUrl)
-    return undefined
-  }
+  useEffect(() => {
+    if (!file && artistInfo) {
+      cacheArtistArt(artistId, artistInfo.largeImageUrl)
+    }
+  })
 
   return file
 }

app/hooks/trackplayer.ts

@@ -1,5 +1,5 @@
 import { Song } from '@app/models/music'
-import { selectMusic } from '@app/state/music'
+import { selectCache } from '@app/state/cache'
 import { useStore } from '@app/state/store'
 import {
   getCurrentTrack,
@@ -191,7 +191,7 @@ export const useSetQueue = () => {
   const getQueueShuffled = useCallback(() => !!useStore.getState().shuffleOrder, [])
   const setQueueContextType = useStore(selectTrackPlayer.setQueueContextType)
   const setQueueContextId = useStore(selectTrackPlayer.setQueueContextId)
-  const getCoverArtPath = useStore(selectMusic.getCoverArtPath)
+  const getCoverArtPath = useStore(selectCache.getCoverArtPath)
 
   return async (
     songs: Song[],

app/state/cache.ts

@@ -0,0 +1,234 @@
+import { Song } from '@app/models/music'
+import PromiseQueue from '@app/util/PromiseQueue'
+import { SetState, GetState } from 'zustand'
+import { Store } from './store'
+import produce from 'immer'
+import RNFS from 'react-native-fs'
+
+const imageDownloadQueue = new PromiseQueue(10)
+
+export type DownloadFile = {
+  path: string
+  date: number
+  progress: number
+  promise?: Promise<void>
+}
+
+export type CacheSlice = {
+  coverArtDir?: string
+  artistArtDir?: string
+  songsDir?: string
+
+  cachedCoverArt: { [coverArt: string]: DownloadFile }
+  downloadedCoverArt: { [coverArt: string]: DownloadFile }
+
+  cacheCoverArt: (coverArt: string) => Promise<void>
+  getCoverArtPath: (coverArt: string) => Promise<string>
+
+  cachedArtistArt: { [artistId: string]: DownloadFile }
+  downloadedArtistArt: { [artistId: string]: DownloadFile }
+
+  cacheArtistArt: (artistId: string, url?: string) => Promise<void>
+
+  cachedSongs: { [id: string]: DownloadFile }
+  downloadedSongs: { [id: string]: DownloadFile }
+
+  albumCoverArt: { [id: string]: string | undefined }
+  albumCoverArtRequests: { [id: string]: Promise<void> }
+  fetchAlbumCoverArt: (id: string) => Promise<void>
+  getAlbumCoverArt: (id: string | undefined) => Promise<string | undefined>
+  mapSongCoverArtFromAlbum: (songs: Song[]) => Promise<Song[]>
+}
+
+export const selectCache = {
+  cacheCoverArt: (store: CacheSlice) => store.cacheCoverArt,
+  getCoverArtPath: (store: CacheSlice) => store.getCoverArtPath,
+  cacheArtistArt: (store: CacheSlice) => store.cacheArtistArt,
+
+  fetchAlbumCoverArt: (store: CacheSlice) => store.fetchAlbumCoverArt,
+}
+
+export const createCacheSlice = (set: SetState<Store>, get: GetState<Store>): CacheSlice => ({
+  cachedCoverArt: {},
+  downloadedCoverArt: {},
+
+  cacheCoverArt: async coverArt => {
+    const client = get().client
+    if (!client) {
+      return
+    }
+
+    const path = `${get().coverArtDir}/${coverArt}`
+
+    const existing = get().cachedCoverArt[coverArt]
+    if (existing) {
+      if (existing.promise !== undefined) {
+        return await existing.promise
+      } else {
+        return
+      }
+    }
+
+    const promise = imageDownloadQueue
+      .enqueue(
+        () =>
+          RNFS.downloadFile({
+            fromUrl: client.getCoverArtUri({ id: coverArt }),
+            toFile: path,
+          }).promise,
+      )
+      .then(() => {
+        set(
+          produce<CacheSlice>(state => {
+            state.cachedCoverArt[coverArt].progress = 1
+            delete state.cachedCoverArt[coverArt].promise
+          }),
+        )
+      })
+    set(
+      produce<CacheSlice>(state => {
+        state.cachedCoverArt[coverArt] = {
+          path,
+          date: Date.now(),
+          progress: 0,
+          promise,
+        }
+      }),
+    )
+    return await promise
+  },
+
+  getCoverArtPath: async coverArt => {
+    const existing = get().cachedCoverArt[coverArt]
+    if (existing) {
+      if (existing.promise) {
+        await existing.promise
+      }
+      return existing.path
+    }
+
+    await get().cacheCoverArt(coverArt)
+    return get().cachedCoverArt[coverArt].path
+  },
+
+  cachedArtistArt: {},
+  downloadedArtistArt: {},
+
+  cacheArtistArt: async (artistId, url) => {
+    if (!url) {
+      return
+    }
+
+    const client = get().client
+    if (!client) {
+      return
+    }
+
+    const path = `${get().artistArtDir}/${artistId}`
+
+    const existing = get().cachedArtistArt[artistId]
+    if (existing) {
+      if (existing.promise !== undefined) {
+        return await existing.promise
+      } else {
+        return
+      }
+    }
+
+    const promise = imageDownloadQueue
+      .enqueue(
+        () =>
+          RNFS.downloadFile({
+            fromUrl: url,
+            toFile: path,
+          }).promise,
+      )
+      .then(() => {
+        set(
+          produce<CacheSlice>(state => {
+            state.cachedArtistArt[artistId].progress = 1
+            delete state.cachedArtistArt[artistId].promise
+          }),
+        )
+      })
+    set(
+      produce<CacheSlice>(state => {
+        state.cachedArtistArt[artistId] = {
+          path,
+          date: Date.now(),
+          progress: 0,
+          promise,
+        }
+      }),
+    )
+  },
+
+  cachedSongs: {},
+  downloadedSongs: {},
+
+  albumCoverArt: {},
+  albumCoverArtRequests: {},
+
+  fetchAlbumCoverArt: async id => {
+    const client = get().client
+    if (!client) {
+      return
+    }
+
+    const inProgress = get().albumCoverArtRequests[id]
+    if (inProgress !== undefined) {
+      return await inProgress
+    }
+
+    const promise = new Promise<void>(async resolve => {
+      try {
+        const response = await client.getAlbum({ id })
+        set(
+          produce<CacheSlice>(state => {
+            state.albumCoverArt[id] = response.data.album.coverArt
+          }),
+        )
+      } finally {
+        resolve()
+      }
+    }).then(() => {
+      set(
+        produce<CacheSlice>(state => {
+          delete state.albumCoverArtRequests[id]
+        }),
+      )
+    })
+    set(
+      produce<CacheSlice>(state => {
+        state.albumCoverArtRequests[id] = promise
+      }),
+    )
+
+    return await promise
+  },
+
+  getAlbumCoverArt: async id => {
+    if (!id) {
+      return
+    }
+
+    const existing = get().albumCoverArt[id]
+    if (existing) {
+      return existing
+    }
+
+    await get().fetchAlbumCoverArt(id)
+    return get().albumCoverArt[id]
+  },
+
+  mapSongCoverArtFromAlbum: async songs => {
+    const mapped: Song[] = []
+    for (const s of songs) {
+      mapped.push({
+        ...s,
+        coverArt: await get().getAlbumCoverArt(s.albumId),
+      })
+    }
+    return mapped
+  },
+})

app/state/music.ts

@@ -14,24 +14,12 @@ import {
   PlaylistListItem,
   PlaylistWithSongs,
   SearchResults,
-  Song,
 } from '@app/models/music'
 import { Store } from '@app/state/store'
 import { GetAlbumList2Type, StarParams } from '@app/subsonic/params'
-import PromiseQueue from '@app/util/PromiseQueue'
 import produce from 'immer'
-import RNFS from 'react-native-fs'
 import { GetState, SetState } from 'zustand'
 
-const imageDownloadQueue = new PromiseQueue(5)
-
-export type DownloadFile = {
-  path: string
-  date: number
-  progress: number
-  promise?: Promise<void>
-}
-
 export type MusicSlice = {
   //
   // family-style state
@@ -70,29 +58,6 @@ export type MusicSlice = {
   fetchHomeLists: () => Promise<void>
   clearHomeLists: () => void
 
-  //
-  // downloads
-  //
-  coverArtDir?: string
-  artistArtDir?: string
-  songsDir?: string
-
-  cachedCoverArt: { [coverArt: string]: DownloadFile }
-  downloadedCoverArt: { [coverArt: string]: DownloadFile }
-
-  coverArtRequests: { [coverArt: string]: Promise<void> }
-
-  cacheCoverArt: (coverArt: string) => Promise<void>
-  getCoverArtPath: (coverArt: string) => Promise<string>
-
-  cachedArtistArt: { [artistId: string]: DownloadFile }
-  downloadedArtistArt: { [artistId: string]: DownloadFile }
-
-  cacheArtistArt: (artistId: string, url?: string) => Promise<void>
-
-  cachedSongs: { [id: string]: DownloadFile }
-  downloadedSongs: { [id: string]: DownloadFile }
-
   //
   // actions, etc.
   //
@@ -100,12 +65,6 @@ export type MusicSlice = {
   starredAlbums: { [id: string]: boolean }
   starredArtists: { [id: string]: boolean }
   starItem: (id: string, type: string, unstar?: boolean) => Promise<void>
-
-  albumCoverArt: { [id: string]: string | undefined }
-  albumCoverArtRequests: { [id: string]: Promise<void> }
-  fetchAlbumCoverArt: (id: string) => Promise<void>
-  getAlbumCoverArt: (id: string | undefined) => Promise<string | undefined>
-  mapSongCoverArtFromAlbum: (songs: Song[]) => Promise<Song[]>
 }
 
 export const selectMusic = {
@@ -135,12 +94,7 @@ export const selectMusic = {
   fetchHomeLists: (store: MusicSlice) => store.fetchHomeLists,
   clearHomeLists: (store: MusicSlice) => store.clearHomeLists,
 
-  cacheCoverArt: (store: MusicSlice) => store.cacheCoverArt,
-  getCoverArtPath: (store: MusicSlice) => store.getCoverArtPath,
-  cacheArtistArt: (store: MusicSlice) => store.cacheArtistArt,
-
   starItem: (store: MusicSlice) => store.starItem,
-  fetchAlbumCoverArt: (store: MusicSlice) => store.fetchAlbumCoverArt,
 }
 
 function reduceStarred(
@@ -416,110 +370,6 @@ export const createMusicSlice = (set: SetState<Store>, get: GetState<Store>): Mu
     set({ homeLists: {} })
   },
 
-  cachedCoverArt: {},
-  downloadedCoverArt: {},
-
-  coverArtRequests: {},
-
-  cacheCoverArt: async coverArt => {
-    const client = get().client
-    if (!client) {
-      return
-    }
-
-    const path = `${get().coverArtDir}/${coverArt}`
-
-    const existing = get().cachedCoverArt[coverArt]
-    if (existing) {
-      if (existing.promise !== undefined) {
-        return await existing.promise
-      } else {
-        return
-      }
-    }
-
-    const promise = imageDownloadQueue
-      .enqueue<void>(() =>
-        RNFS.downloadFile({
-          fromUrl: client.getCoverArtUri({ id: coverArt }),
-          toFile: path,
-        }).promise.then(() => new Promise(resolve => setTimeout(resolve, 100))),
-      )
-      .then(() => {
-        set(
-          produce<MusicSlice>(state => {
-            state.cachedCoverArt[coverArt].progress = 1
-            delete state.cachedCoverArt[coverArt].promise
-          }),
-        )
-      })
-    set(
-      produce<MusicSlice>(state => {
-        state.cachedCoverArt[coverArt] = {
-          path,
-          date: Date.now(),
-          progress: 0,
-          promise,
-        }
-      }),
-    )
-    return await promise
-  },
-
-  getCoverArtPath: async coverArt => {
-    const existing = get().cachedCoverArt[coverArt]
-    if (existing) {
-      if (existing.promise) {
-        await existing.promise
-      }
-      return existing.path
-    }
-
-    await get().cacheCoverArt(coverArt)
-    return get().cachedCoverArt[coverArt].path
-  },
-
-  cachedArtistArt: {},
-  downloadedArtistArt: {},
-
-  cacheArtistArt: async (artistId, url) => {
-    if (!url) {
-      return
-    }
-
-    const client = get().client
-    if (!client) {
-      return
-    }
-
-    const path = `${get().artistArtDir}/${artistId}`
-
-    set(
-      produce<MusicSlice>(state => {
-        state.cachedArtistArt[artistId] = {
-          path,
-          date: Date.now(),
-          progress: 0,
-        }
-      }),
-    )
-    await imageDownloadQueue.enqueue(
-      () =>
-        RNFS.downloadFile({
-          fromUrl: url,
-          toFile: path,
-        }).promise,
-    )
-    set(
-      produce<MusicSlice>(state => {
-        state.cachedArtistArt[artistId].progress = 1
-      }),
-    )
-  },
-
-  cachedSongs: {},
-  downloadedSongs: {},
-
   starredSongs: {},
   starredAlbums: {},
   starredArtists: {},
@@ -579,70 +429,4 @@ export const createMusicSlice = (set: SetState<Store>, get: GetState<Store>): Mu
       setStarred(unstar)
     }
   },
-
-  albumCoverArt: {},
-  albumCoverArtRequests: {},
-
-  fetchAlbumCoverArt: async id => {
-    const client = get().client
-    if (!client) {
-      return
-    }
-
-    const inProgress = get().albumCoverArtRequests[id]
-    if (inProgress !== undefined) {
-      return await inProgress
-    }
-
-    const promise = new Promise<void>(async resolve => {
-      try {
-        const response = await client.getAlbum({ id })
-        set(
-          produce<MusicSlice>(state => {
-            state.albumCoverArt[id] = response.data.album.coverArt
-          }),
-        )
-      } finally {
-        resolve()
-      }
-    }).then(() => {
-      set(
-        produce<MusicSlice>(state => {
-          delete state.albumCoverArtRequests[id]
-        }),
-      )
-    })
-    set(
-      produce<MusicSlice>(state => {
-        state.albumCoverArtRequests[id] = promise
-      }),
-    )
-
-    return await promise
-  },
-
-  getAlbumCoverArt: async id => {
-    if (!id) {
-      return
-    }
-
-    const existing = get().albumCoverArt[id]
-    if (existing) {
-      return existing
-    }
-
-    await get().fetchAlbumCoverArt(id)
-    return get().albumCoverArt[id]
-  },
-
-  mapSongCoverArtFromAlbum: async songs => {
-    const mapped: Song[] = []
-    for (const s of songs) {
-      mapped.push({
-        ...s,
-        coverArt: await get().getAlbumCoverArt(s.albumId),
-      })
-    }
-    return mapped
-  },
 })

app/state/store.ts

@@ -3,11 +3,13 @@ import { createSettingsSlice, SettingsSlice } from '@app/state/settings'
 import AsyncStorage from '@react-native-async-storage/async-storage'
 import create from 'zustand'
 import { persist, StateStorage } from 'zustand/middleware'
+import { CacheSlice, createCacheSlice } from './cache'
 import { createTrackPlayerSlice, TrackPlayerSlice } from './trackplayer'
 
 export type Store = SettingsSlice &
   MusicSlice &
-  TrackPlayerSlice & {
+  TrackPlayerSlice &
+  CacheSlice & {
     hydrated: boolean
     setHydrated: (hydrated: boolean) => void
   }
@@ -36,6 +38,7 @@ export const useStore = create<Store>(
       ...createSettingsSlice(set, get),
       ...createMusicSlice(set, get),
       ...createTrackPlayerSlice(set, get),
+      ...createCacheSlice(set, get),
 
       hydrated: false,
       setHydrated: hydrated => set({ hydrated }),

app/subsonic/api.ts

@@ -83,7 +83,7 @@ export class SubsonicApiClient {
     }
 
     const url = `${this.address}/rest/${method}?${query}`
-    console.log(`${method}: ${url}`)
+    // console.log(`${method}: ${url}`)
     return url
   }