✨ Allow adding all lists of a user to workspace

[foysalit]

[matthieusieben]

Due to the nature of how react works, this condition will not actually cancel the loop as isConfirmationOpen is a const that will never become false.

For this to work you'll probably need to rely on a ref & AbortController.

const runController = ref<AbortController | null>(null)
const isWorking = abortRef.current != null

const startWorking = async () => {
  // Get a reference to the previous run controller before we replace it with our own
  const prevController = runController.current

  const controller = new AbortController()
  runController.current = controller

  // Abort previous run, if any
  prevController?.abort()

  const { signal } = controller
  try {
    do {
      await someAsyncStuff({
        signal
      })
    } while (!signal.aborted)


    if (signal.aborted) {
      // make sure the catch block is triggered
      throw signal.reason
    }
  } catch (err) {
    if (signal.aborted) {
      toast.info('this action was aborted')
    } else {
      toast.error(`Something went wrong: ${(e as Error).message}`)
    }
  } finally {
    // Prevent re-use (for good measure)
    controller.abort()

    // If this run is still the currently active one
    if (runController.current === controller) {
      // Cleanup reference
      runController.current = null

      // Close the modal
      setIsConfirmationOpen(false)
    }
  } 
}

const abortCurrentRun = () => {
  // No need to call setIsConfirmationOpen here as this will be done through startWorking's finally block
  runController.current?.abort()
}

// Abort the current run when the component is unmounted (e.g. due to navigation)
useEffect(() => {
  return () => runController.current?.abort()
}, [runController])

PS: If this feels too manual, the mutation lib probably already does all of the above

[foysalit]

yeah I knew it wasn't optimal but it does kill the requests if other conditions cause a re-render.
we use this pattern in a few places so I cleaned up all usage of this pattern to use abort controllers. thank you for pointing it out!

[matthieusieben]

For good measure, we should ensure that only one run is possible at any given time.

Suggested change

	const newAbortController = new AbortController()
	// allready running
	if (abortController.current) return
	const newAbortController = new AbortController()

or

Suggested change

	const newAbortController = new AbortController()
	// user cancelled by confirming again
	abortController.current?.abort('user-cancelled')
	const newAbortController = new AbortController()

[foysalit]

hmm... not sure how this case could happen since every interaction aborts the controller after it is instantiated. especially with the abort on unmount. I will give it another look when refactoring these into a hook.

[matthieusieben]

It shouldn't happen, but only because the isAdding UI prevents it to. So if someone else breaks that code (eg. by changing the UI), it might end-up happening in parallel. Adding this is just a safety measure against future changes.

[matthieusieben]

These changes all look very similar. I wonder if there is a way to avoid the code duplication by making some kind of hook ? Looks good otherwise.

[matthieusieben]

We need to cancel the current run if the component is un-mounted (same thing in other places)

Suggested change

	const abortController = useRef<AbortController | null>(null)
	const abortController = useRef<AbortController | null>(null)
	useEffect(() => {
	// User cancelled by closing this view (navigation, other?)
	return () => abortController.current?.abord('user-cancelled')
	}, [])

[foysalit]

I think i was avoiding this earlier on so that folks can run one of these and leave it running in the background while they browse and do other things. but in hindsight, that may be a bit too risky since it will make it very easy for people to leave something like this running for 20K followers or something.

[foysalit]

agreed on the refactoring into a hook recommendation. I'm adding a ticket for that and don't wanna bite that off in this PR. let me get this out and will handle the refactoring in a future, separate PR.