CODEWITHSUNDEEP
javascriptreactjsreact-hooksgraphql
Crocsx
Crocsx

Reputation: 7609

how to properly cleanup useEffect that contain async graphql operation

I am using graphql/apollo and react.

I have the following code

  const [state, setState] = useState(undefined);
  useEffect(() => {
    (async () => {
      try {
        const workspace = await getFirstWorkspace();
        // Do Something
        setState(withSomething)
      } catch (error) {
        // Do Something Else
        setState(withErrorSomething)
      }
    })();
  }, [generateLink, getFirstWorkspace, masterDataStoreId]);

now, this worked fine until I updated some packages, I currently get thrown this error.

Uncaught (in promise) DOMException: signal is aborted without reason

From what I understand my useEffect throw this when the component is unmounted an the query didn't finish to run.

Now, this cause my catch to always trigger at least once, cause it looks like when the effect is run again cause one of the dep changed, it fail.

I """ fixed """ it by doing

  const [state, setState] = useState(undefined);
  useEffect(() => {
    (async () => {
      try {
        const workspace = await getFirstWorkspace();
        // Do Something
        setState(withSomething)
      } catch (error) {
        // Do Something Else
        if ((error as any)?.name === 'AbortError') {
          return;
        }
        setState(withErrorSomething)
      }
    })();
  }, [generateLink, getFirstWorkspace, masterDataStoreId]);

And not assign any state in case the error is an abort. But I couldn't find any proper solution or I don't understand why this is problematic before and not now, I did update some package but none mention a change of behavior on this end.

My question is, what should I do to do thing correctly ?

Upvotes: 0

Views: 775

Answers (2)

Proton029
Proton029

Reputation: 1

U could make use of AbortController

  const [state, setState] = useState(undefined);
  useEffect(() => {
    const controller = new AbortController();
    const signal = controller.signal;
    (async () => {
      try {
        const workspace = await getFirstWorkspace(signal);
        // Do Something
        setState(withSomething)
      } catch (error) {
        // Do Something Else
        setState(withErrorSomething)
      }
    })();
    return =()=>Controller.abort();
  }, [generateLink, getFirstWorkspace, masterDataStoreId]);

Upvotes: 0

T.J. Crowder
T.J. Crowder

Reputation: 1074335

I don't think the error you've quoted is coming from React. React used to complain if you did a state update in a component that was no longer mounted, but the error message it used was "Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application." But recent versions of React don't do that because the React team decided it was too fussy.

Still, answering the question as asked:

  1. If getFirstWorkspace offers a way to tell it to cancel what it's doing, you'd do that. For instance, if it supported AbortSignal, you might do this:

    useEffect(() => {
    // *** Create a controller and get its signal
    const controller = new AbortController();
    const { signal } = controller;
    (async () => {
        try {
            // *** Pass the signal to `getFirstWorkspace`
            const workspace = await getFirstWorkspace(signal);
            // *** Only do something if the signal isn't aborted
            if (!signal.aborted) {
                // Do Something
                setState(withSomething);
            }
        } catch (error) {
            // *** Only do something if the signal isn't aborted
            if (!signal.aborted) {
                // Do Something Else
                setState(withErrorSomething);
            }
        }
    })();
    return () => {
        // *** Abort the signal on cleanup
        controller.abort();
    };
}, [generateLink, getFirstWorkspace, masterDataStoreId]);

    ...or similar if it doesn't support AbortSignal specifically but does provide some other way of cancelling its work.

  2. If it doesn't, you could fall back to a flag telling you not to use the result:

    useEffect(() => {
    // *** Start with a flag set to `false`
    let cancelled = false;
    (async () => {
        try {
            const workspace = await getFirstWorkspace();
            // *** Only do something if the flag is still `false`
            if (!cancelled) {
                // Do Something
                setState(withSomething);
            }
        } catch (error) {
            // *** Only do something if the flag is still `false`
            if (!cancelled) {
                // Do Something Else
                setState(withErrorSomething);
            }
        }
    })();
    return () => {
        // *** Set the flag on cleanup
        cancelled = true;
    };
}, [generateLink, getFirstWorkspace, masterDataStoreId]);

It's better to actually cancel the work if you can, but it's fine to have a fallback boolean if you can't. Just don't assume you can't, be sure to check first. :-)

Side note: I love async/await, but when you're doing just a single call and getting a promise, doing an async wrapper and try/catch around await can be a bit overkill. FWIW, just using the promise directly looks like this (using the flag in this case, but it works just as well with the controller/signal):

useEffect(() => {
    let cancelled = false;
    getFirstWorkspace().then(
        (workspace) => {
            if (!cancelled) {
                // Do Something
                setState(withSomething);
            }
        },
        (error) => {
            if (!cancelled) {
                // Do Something Else
                setState(withErrorSomething);
            }
        }
    );
    return () => {
        cancelled = true;
    };
}, [generateLink, getFirstWorkspace, masterDataStoreId]);

Upvotes: 2

Related Questions