Fix problems regarding cancellation of long-running promises, state updates to unmounted components, maps as states, and existence of files in file system as implicit state

[simonwacker]

See the to-dos in 566b4b0, that is, the to-dos in the files download-switch.tsx and audio-player.tsx.

I thought it may be a good idea to have a Zustand store called download-store or file-store that internally uses Expo's FileSystem but exposes an interface with callbacks and such as we need it. After implementing it partially in #38, I'm not so sure anymore if this is really a good idea. The advantages are:

• Downloads are not bound to components and thus can be kept active in the background. Is this desirable? How do we notify the user of the download progress then? Should we have a section (maybe under settings) where all downloaded files, active downloads, and possible downloads are listed and can be deleted, stopped, started?
• Multiple components related to downloads on the same screen share the same Expo ResumableDownload instance, for example, right now for a playlist, there is a download switch for the currently playing track and one that downloads all tracks and the background music. So, these switches are related and when I download all tracks there is a point when the currently playing track has been downloaded in which case the corresponding switch should change state. And there are similar cases. It can be argued of course that the current design is bad and there should ever only be one download switch. I don't know.

Note that components using the download store need to add and remove callbacks when they are mounted and unmounted.

[timhabermaas]

After implementing it partially in #38, I'm not so sure anymore if this is really a good idea.

I'm not sure either. I've used a (global) state manager (redux) in the past to store all data including data fetched from the server. This required manual syncing between remote data fetches and redux and was in general pretty ugly to work with. Invalidating caches was a pain and since every part of the application could in theory write to the store it wasn't clear when data fetches where happening or which parts triggered them. The only advantage where the redux devtools which showed the deserialized remote data the app currently knows about.

I later switched to using react-query (alternative: SWR) for all remote data fetches while keeping redux for all local UI state not derived from remote data and it was so much nicer to work with. The caching for remote data fetches redux provided was still there, but handled under the hood by the library (including automatic refetches, cache invalidation etc.). Also: The user-facing code doesn't have to use any promises (and therefore avoid the cancelling issue altogether). The hooks return {data: T?, loading: boolean, error: E?} triples and trigger a rerender if any of the values changes (e.g. when the underlying promise resolved).

I've no idea whether one should consider the download status/file system "remote data", though. Personally I would consider everything slower than RAM access (or anything which requires promises) as remote data fetching, but that's a very arbitrary rule. There's probably a more fundamental reason why using local stores for some data feels awkward while it doesn't for some other kinds of data. The motivation has a few words to say about the difference as well.

So, 👍 from me for trying to use something like react-query/SWR for fetching the download status instead of putting it in the zustand store.

[timhabermaas]

As for the "we want one global ResumableDownload instance per track" issue, I'd probably use module level globals in combination with custom hooks based on react-query like so:

let RESUMABLE_DOWNLOADS = {};

useDownloadStatus(track: Track) {
  return useQuery<DownloadStatus>(["download-status", track.id], async function() {
    // NOTE: Probably prone to race conditions. Works fine as long as everything is single threaded (=> no context switch between `await`s).
    if !RESUMABLE_DOWNLOADS[track.id] {
      RESUMABLE_DOWNLOADS[track.id] = ResumableDownload.create(track.filename, track.uri);
    }
    
    const status = await RESUMABLE_DOWNLOADS[track.id].getStatus();
    return status;
  });
}

This should make sure that one track has at most one resumable download instance. We could also use React contexts and hide useContext in the custom hook as well.

[simonwacker]

I like the idea of using react-query but have only a vague sense of the implications. I also like the simplicity of module level globals (although I don't understand when those are initialized and re-initialized if the latter is possible at all). If you are willing to do some implementation, then go for it! Otherwise I'll have a try in 3 weeks time.

[timhabermaas]

Alright, thanks! I'll give it a shot then.

(although I don't understand when those are initialized and re-initialized if the latter is possible at all)

Usually they're set once the module is imported, so that would be at application start or when we display the first DownloadSwitch. I'm not sure how it behaves in dev mode with hot code reloading enabled, though.