In TypeAhead Demo, I don't see how abort is being called when query changes

[pkellner]

I'm worried that you are not aborting you previous request when the query string is changed. It seems that you are just creating another task. I only see the .abort() called when you explicitly cancel by pressing the button.

Am I mis-understanding? seems like you'd need some kind of timer running and then when the query string changes, you'd need to abort the request and create another one (rather than just creating another one).

[pkellner]

I think I figure it out. You call the cleanup when the task manager is replaced and that aborts.

[dai-shi]

Yeah, we abort a previous task here (regardless whether it's running or not).

react-hooks-async/src/use-async-run.js

Lines 7 to 9 in 6f0e2b3

	if (previous.current) {
	previous.current.abort();
	}

Hm, I don't remember why I didn't use cleanup in useEffect...
It should be better because it aborts task on unmount.

[pkellner]

double hm. I'm thinking that in https://github.com/dai-shi/react-hooks-async/blob/master/src/use-async-task.js, you have a cleanup and that gets called automatically when you create an new task on top of an old task. That is, if the create task call is in the render method, and the render method gets called again because of a query change, the new task is created, the old task is destroyed and clean up automatically runs causing the signal to be fired, and the running task to be aborted. Maybe I just made all that up, but I put some logging in and that is what I think is happening.

[dai-shi]

useAsyncTask creates an async task (object) but it doesn't run nor abort by itself.
useAsyncRun is the actual one that has a control of running and aborting.

The reason why they are separated is to allow combining tasks by useAsyncCombine*.

I guess you saw the behavior that useAsyncRun triggers the aborting and then it actually aborts in useAsyncTask in the logging.

[pkellner]

Would you possibly have a moment to look and see if I handled axios request cancel correctly here. I did it in cleanup but it seems not to be working correctly.

https://stackoverflow.com/q/53861916/46942

[dai-shi]

You should not update state after unmounted.

Just coding in my head:

const useAxiosGet = url => {
  const [data, setData] = useState(null);
  const [error, setError] = useState(null);
  const [loading, setLoading] = useState(true);
  useEffect(() => {
    let isMounted = true;
    const source = axios.CancelToken.source();
    axios.get(url, { cancelToken: source.token })
      .then(data => {
        if (isMounted) {
          setData(data);
          setLoading(false);
        }
      })
      .catch(e => {
        if (isMounted) {
          setError(e);
          setLoading(false);
        }
      });
    return () => {
      isMounted = false;
      source.cancel('cancel');
    };
  }, []);
  return { data, error, loading };
};

[pkellner]

Good catch. Otherwise, you believe I'm handling the cancelation token correctly? That's the part that always confused me.

[dai-shi]

Guess so.

Some other notes:

• calling source() out of useEffect doesn't make sense. it won't be used even after newly created.
• try-catch is probablly unnecessary if you use promise.
• setLoading in useEffect is redundant.
• this hook is one-time only, meaning it doesn't follow the change of url.
• window.location= may not unmount the component but just leave the page entirely.

[dai-shi]

Closing this issue. Feel free to open a new one.