Open simonwacker opened 3 years ago
As far as I know the boolean flag to signal cancelation is the only way to "cancel" (native) promises right now. There's also https://github.com/petkaantonov/bluebird#note which is an alternative implementation to promises and supports cancelation. However, all functions which return native promises need to return the alternative implementation, meaning we can't use fetch
etc. directly and have to convert from one promise implementation to the other at some point.
After diving into SWR/react-query's [1] approach to canceling requests it looks like cancelling promises is just one side of the issue. Cancelling promises only avoids calling the success
/error
callbacks if we're no longer interested in the result (e.g. if the component is already unmounted).
The other issue is stopping the request itself. The request would still be active in the background even if we cancel the corresponding promise (I'm really not sure here...) meaning it would waste bandwith. fetch
provides an API to abort requests and react-query's documentation describes how to integrate it with react-query's concept of cancelling requests. SWR doesn't seem to have an API yet for cancelling the underlying request.
It looks like we're currently not using fetch
directly, but DownloadResumable
from expo and we're also using the pauseAsync
function they provide which seems to handle the request cancellation under the hood. Does this mean the "aborting request" issue is already solved? (Sorry for the dumb question, I'm not fully following the connection between DownloadResumable
and the promises yet.)
If request cancellation isn't an issue (only promise cancellation), SWR/react-query could indeed be used to avoid the unmounting issue by hiding manual promise creations into a useQuery
hook which automatically ignores the callbacks if the component its used in is unmounted:
function useDownloadStatus(track: Track) {
return useQuery<DownloadStatus>(["downloadStatus", trackId], function() {
const { exists } = await FileSystem.getInfoAsync(getTrackFileUri(track));
// some mapping
return { state: ..., progress: ... };
}, {staleTime: 0, refetchEvery: 10});
})
// React component
function DownloadStatus(props: { track: Track} {
const { data, loading } = useDownloadStatus(props.track);
<>{data && data.state === IS_DOWNLOADING && <DownloadProgress/>}</>
This would get rid of the syncing between the getInfoAsync
API and the local component state and we could also control when to refetch that data better.
If we need to fetch the array of tracks with their corresponding statuses, we can also use useDownloadStatuses
instead which does the current Promise.all
short circuiting und der the hood.
[1] react-query fills the same niche as SWR and I've used it in the past. Worked great, devtools not yet available on react-native, though. They've been awesome to figure out why stuff was cached/refetched/etc.
Thanks for the thorough suggestions.
Request cancellation is actually not an issue because DownloadResumable
handles that. One caveat with it though is that if you pause a download while awaiting it to finish, the await promise is rejected and the corresponding error does not tell that the cancellation is due to someone invoking pauseAsync
(as least as far as I know).
I like the idea of useDownloadStatus
. However, I currently can't grasp if other issues will pop up using it.
I did it with a
isMounted
flag in https://github.com/simonwacker/yoga-insights/blob/566b4b019c73ab03170839e1469fcedf1c8c35a4/app/components/download-switch/download-switch.tsx#L282-L318Related to #35