Bug: Impossible to use on async hooks

[arietrouw]

The rule that prevents using async functions in the hook makes sense in most cases, but there are some valid hooks that have async functions. See useAsyncEffect as an example. (https://www.npmjs.com/package/use-async-effect)

Since the no async rule is bundled with the 'react-hooks/exhaustive-deps' rule, there is no way to use this with an async hook.

React version: 16, 17

Steps To Reproduce

1. Create a file that uses useAsyncHook

2. Configure plugin ->

   "react-hooks/exhaustive-deps": [
   "warn",
   {
   "additionalHooks": "(useAsyncEffect)"
   }
   ]

3. lint - see error

The current behavior

linter displays:

warning Effect callbacks are synchronous to prevent race conditions. Put the async function inside:

useEffect(() => {
async function fetchData() {
// You can await here
const response = await MyAPI.getData(someId);
// ...
}
fetchData();
}, [someId]); // Or [] if effect doesn't need props or state

Learn more about data fetching with Hooks: https://reactjs.org/link/hooks-data-fetching react-hooks/exhaustive-deps

The expected behavior

Several options:

1. Detect the proper use of async in the hook (may be tough)
2. Make a "react-hooks/no-async" rule to control this behavior (allows exhaustive-dpes check w/o this warning)
3. Add settings for the "additionalHooks" property to specify hook by hook functionality

I would say that all 3 of this would be great, but #2 to me seems to be a very simple fix.

[EduardoAraujoB]

@arietrouw this is a by design behavior, and you really shouldn't do this, useEffect functions are expected do be syncronous by React, if you need to handle async code on a useEffect use an async function inside it, but do not turn your useEffect function async

[joshuakb2]

@arietrouw this is a by design behavior, and you really shouldn't do this, useEffect functions are expected do be syncronous by React, if you need to handle async code on a useEffect use an async function inside it, but do not turn your useEffect function async

This doesn't make sense to me. These two effects do exactly the same thing:

useEffect(() => {
  async function doTheEffect() {
    // do the effect
  }
  doTheEffect();
}, []);

useEffect(async () => {
  // do the effect
}, []);

The only difference is that one returns a promise and the other doesn't. But useEffect ignores any returned value anyway, right? The suggested "correct" way is simply redundant.

The pitfalls in the second approach have to do with async effects in general, not specifically whether the callback function is defined as async or not. The function could just as easily be defined as a normal function with a promise chain and this rule would not complain.

I don't think the exhaustive-deps rule should be mandating a particular style, and I don't see any functional difference here.

[joshuakb2]

But useEffect ignores any returned value anyway, right?

I was slightly mistaken here. useEffect does care about the returned value if it's a function. Any returned function is called when the component unmounts and before the effect reruns, as a cleanup function. So an async effect function can't actually return any cleanup step.

Still, if the effect doesn't have a cleanup step, nothing is lost here. I believe React ignores the returned value if it isn't a function.

[jsamr]

The request from @arietrouw is pretty straightforward: decouple "async" usage from the exhaustive-deps rule. Single responsibility principle: async function usage has nothing to do with the array of dependencies alluded in the "exhaustive-deps" name. And it is a pain for custom hooks added with additionalHooks option. An alternative would be to provide a second option: allowAsyncForHooks: "...".

[arietrouw]

I have worked around this in another way. I made a hook called usePromise which takes a dependency array and does not trigger this rule. It can be found here: @xylabs/react-promise on npmjs

I think this is better since it does not mimic the effect hook behavior

Sorry, no usage in readme yet, but will add.

[arietrouw]

I must say that I still agree with @jsamr that having one rule enforce two concepts, especially when one has nothing to do with its name seems like an anti-pattern.

[davemecha]

AFAIK, there is no nice solution for this but a simple workaround. Just avoid using the term Effect in the custom hook disables the async check. So naming it e.g. useAsyncEffekt would do the trick.

See my answer in this issue: #19034 (comment)

[fregante]

Agreed, but duplicate of #19034 (May 2020 🥲)

[arietrouw]

The work around and all is great, but do you guys not agree with @jsamr in that a single rule that covers two unrelated concepts is an anti-pattern?