What is the best practice to cancel an already invoked asynchronous callback when component is already unmounted?

[simonwacker]

I did it with a isMounted flag in

yoga-insights/app/components/download-switch/download-switch.tsx

Lines 282 to 318 in 566b4b0

	useEffect(() => {
	// TODO Use some better approach to avoid updating state of unmounted
	// component. The current approach is inspired by
	// https://juliangaramendy.dev/blog/use-promise-subscription
	// They adivce the usage of SWR instead as detailed in
	// https://juliangaramendy.dev/blog/managing-remote-data-with-swr
	// According to its documentation SWR supports react native; do a full-text
	// search on https://github.com/vercel/swr
	// SWR may also be used to abort promises as detailed on
	// https://github.com/vercel/swr#conditional-fetching
	let isMounted = true
	// For why this function lives inside `useEffect` read
	// https://reactjs.org/docs/hooks-faq.html#is-it-safe-to-omit-functions-from-the-list-of-dependencies
	const determineDownloadStatuses = async () => {
	await Promise.all(
	tracks.map(async (track) => {
	try {
	const { exists } = await FileSystem.getInfoAsync(getTrackFileUri(track))
	if (isMounted) {
	statuses.set(
	track.trackId,
	exists ? DownloadStatus.Downloaded : DownloadStatus.NotDownloaded,
	)
	}
	} catch (error) {
	console.error(`Failed to determine download status of track ${track.trackId}.`, error)
	}
	}),
	)
	if (isMounted) {
	setSwitchStatus(getSwitchStatus(switchStatus, statuses))
	}
	}
	__DEV__ && console.log("Determining download status.", tracks, switchStatus, statuses)
	determineDownloadStatuses()
	return () => (isMounted = false)
	}, []) // tracks, switchStatus, statuses // TODO List dependencies in a proper way. Get inspired by https://github.com/facebook/react/issues/14476#issuecomment-471199055

Related to #35

[timhabermaas]

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.

[simonwacker]

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.