Are async useEffect Callbacks Actually Harmful, or Just a "Smell"?

user_5921 0
Viewed 58

I can find lots of articles on the web that will tell you to change:

 useEffect(async() => {
   await doSomethingAsynchronous();
 })

into:

 useEffect(() => {
   const pointlessFunction = async () => {
       await doSomethingAsynchronous();
   };
   pointlessFunction();
 })

But what I can't find is any explanation of the actual harm caused by the former.

I understand that React's ESLint rules will complain if you do the former, and I understand that it's complaining because the former might confusingly suggest that useEffect is waiting for the AJAX to complete (when it's not).

In both cases the async function is generating a promise, and in both cases that promise is being ignored (meaning that neither version is actually waiting for doSomethingAsynchronous to complete).

But ... if you understand the above, and can still write working code in spite of it ... is there any actual harm to doing the former?

EDIT:

It seems from the answers so far there is some "harm", but it's only in the form of limiting options. If I go with form #2, I can't:

  • return a cleanup function (because the async will make the function always return a promise)
  • wrap the callback with a try/catch

So maybe that's the only harm: form #1 can't error handle or do cleanup. But I'm still curious: aside from limiting my options, if I don't need to cleanup or handle errors, will form #1 actually break anything (ie. cause harm)?

After all, using a one-line arrow function (() => 5) limits your options too ... but we still use them all the time, and just use the more verbose form when we need it. Why not do the same here?

use-effect async-await reactjs
3 Answers

There are a couple of issues.

  • React expects two possible return values from the callback: nothing (in which case there is no cleanup function to run), or a function (in which case that function is the cleanup function that will be run). If you return something other than those two return values, you're doing something that React doesn't know how to handle - which means that the logic you're implementing may well be buggy - the script-writer is returning a value of an unexpected type, and may be expecting React to do something with it - but it won't.

    It's a bit like doing

    someArray.forEach((item) => {
  // do something
  return true;
});

    Because forEach ignores the value returned from the callback, any time you see code like this, it's probably a bug (or useless). If some library-writer nowadays decided to implement forEach themselves they might decide to be strict and show a warning when it sees a value returned from the callback.

    React is doing something similar. Using an async function (or returning a non-undefined, non-function value) isn't inherently absolutely forbidden and so doesn't produce an error, but it does indicate that there's something wrong, so it produces a warning.

  • Async functions that reject will result in unhandled rejections, which should absolutely be avoided for the same reason that plain runtime errors should be avoided. If you had

    useEffect(async() => {
await doSomethingAsynchronous();
})

    and doSomethingAsynchronous rejected, you would end up with an unhandled rejection - because React does not do anything with the returned Promise, and you should implement the error handling yourself.

user_95152070

While the other answers here are great and informative, none directly answered my original question, ie. "Is there any harm to providing useEffect with an async callback?

The answer is ... no, there is no harm.

However, there is an important limitation to that approach, which is that you cannot provide a cleanup function if you use it (because async functions can never return functions, only promises). Also, some devs may be concerned that future versions of React may break when an async callback is given to useEffect.

Both concerns can easily be addressed with a wrapper around useEffect, called something like useAsyncEffect. This allows you to avoid ES Lint issues (and ensure compatibility with future React versions), while still providing a cleanup function.

export const useAsyncEffect = (effect, dependencies, cleanup) => {
  // If no dependencies are provided but a cleanup function is, fix args
  if (!cleanup && typeof dependencies === 'function') {
    cleanup = dependencies;
    dependencies = undefined;
  }

  // Main wrapper code
  useEffect(() => {
    effect();
    return cleanup;
  }, dependencies);
};
user_59210

I think that it is a bad practice to use async functions in use effects because:

  1. The useEffect function expects to receive a function that returns another function to execute once your component is unmounted, rather than a promise.
  2. As far as I know, non-handled async functions may cause some issues with states when your component is re-rendered, but your async function is not completed yet.
user_80311150
Related