React: Feature request: useEffect add support for async await functions

Created on 4 Jun 2019  Â·  5Comments  Â·  Source: facebook/react

Feature request for adding to the API for the useEffect hook:

currently if I want to use a class component componentDidMount to fetch things from my server I could use async method like so:

async componentDidMount() {
     try {
        const response = await fetch('...');
        const json = await response.json();
        this.setState({...});
    catch(err) {
        this.setState(...)
    }
}

The useEffect should optionally return a cleanup function so applying this kind of syntax in a useEffect is wrong.
So this is wrong:

useEffect(async () => {
    try {
        const response = await fetch('...');
        const json = await response.json();
        setSomeStateCreatedWithUseStatet(...)
    catch(err) {
        setSomeStateCreatedWithUseStatet(...)
    }
})

Of course I could wrap the above code in another async function like so:

useEffect(() => {
    (async function() {
        try {
            const response = await fetch('...');
            const json = await response.json();
            setSomeStateCreatedWithUseStatet(...)
        catch(err) {
            setSomeStateCreatedWithUseStatet(...)
        }    
    })()
})

But this extra nesting kind of contradict the nice readable syntax that async await is giving us.
Since the cleanup function is running in an async way already, it seems to me that the useEffect
should support a return value of a cleanup function or a promise wrapped cleanup function so we can use the async await syntax in the useEffect hook.
So my suggestion is to not change the useEffect rather add an extra option to use async await functions inside the useEffect.

In this way we can use the regular useEffect but also these usages of useEffect will be valid as well:

useEffect(async () => {
    try {
        const response = await fetch('...');
        const json = await response.json();
        setSomeStateCreatedWithUseStatet(...)
    catch(err) {
        setSomeStateCreatedWithUseStatet(...)
    }
    return function cleanup() {...}
})

or if you prefer the hard way:

useEffect(() => {
    ...
    return new Promise((resolve) => {
        resolve(function cleanup() { ...})   
    })
})

What do you think?

Most helpful comment

The canonical way to do it is to define an async function inside and call it. You don’t need to do an IIFE like you did. Just call it normally.

See this example: https://codesandbox.io/s/jvvkoo8pq3

Using async functions directly as effects is intentionally disallowed. The problem is that props or state may change while your async function is running. So you risk getting very confusing behavior.

Separating async logic and cleanup lets you properly handle this case. Again, this example shows why it’s important: https://codesandbox.io/s/jvvkoo8pq3. Here, we handle the case where responses come out of order.

Your proposed alternative doesn’t work for this because we need to run cleanup before the async function finishes. Indeed our goal is to stop it from finishing because the data is fetched is no longer up to date.

All 5 comments

What happens if the effect needs to clean up before the promise resolves?

What happens if the effect needs to clean up before the promise resolves?

What i meant is that we should maybe add to the useEffect the option to return a cleanup like we are doing now, and also we can add a promise wrapped cleanup (support both ways).
If you need cleanup before the promise resolves maybe you should stick to the old way and return a cleanup function and not a promise.

Another option, which I think is more reasonable, is that if we return a cleanup that is a promise, then the useEffect will wait for the promise to resolve and the cleanup to run before calling the next iteration of the useEffect function.

So on the rare case we have something like this:

useEffect(() => {
    console.log('when will I run?');
    ...
    return new Promise(() => {
        ... this is one long promise and takes 10sec to resolve
        resolve(() => {... some cleanup})
    })
})

we will print 'when will i run?' on componentDidMount
and on componentDidUpdate only after 10s and the cleanup of the last hook finished it will run "when will I run" again.

ในวันที่ พ. 5 มิ.ย. 2019 14:23 น. ywarezk notifications@github.com
เขียนว่า:

What happens if the effect needs to clean up before the promise resolves?

What i meant is that we should maybe add to the useEffect the option to
return a cleanup like we are doing now, and also we can add a promise
wrapped cleanup (support both ways).
If you need cleanup before the promise resolves maybe you should stick to
the old way and return a cleanup function and not a promise.

Another option, which I think is more reasonable, is that if we return a
cleanup that is a promise, then the useEffect will wait for the promise to
resolve and the cleanup to run before calling the next iteration of the
useEffect function.

So on the rare case we have something like this:

useEffect(() => {
console.log('when will I run?');
...
return new Promise(() => {
... this is one long promise and takes 10S to resolve
resolve(() => {... some cleanup})
})
})

we will print 'when will i run?' on componentDidMount
and on componentDidUpdate only after 10s and the cleanup of the last hook
finished it will run "when will I run again".

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/15814?email_source=notifications&email_token=AKB7KA7PCSFS4LJPRXCPDNDPY5SXJA5CNFSM4HS2LL5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW62K7Y#issuecomment-498967935,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKB7KA7VVYGMOA6AN5UODSDPY5SXJANCNFSM4HS2LL5A
.

What about an useAsyncEffect instead?

const useAsyncEffect = (effect, cleanup, deps) => useEffect(() => {
  effect();
  return cleanup;
}, deps);


// usage

const [state, setState] = useState(null);

useAsyncEffect(
  async () => {
    const res = await fetch(url);
    const json = await res.json();
    setState(json);
  },
 () => doCleanup(),
 [url]
);

I have to admit the cleanup argument doesn't make much sense since Promise can't be cancelled...

edit: I just found out a library with the same API I proposed:
https://github.com/rauldeheer/use-async-effect

The canonical way to do it is to define an async function inside and call it. You don’t need to do an IIFE like you did. Just call it normally.

See this example: https://codesandbox.io/s/jvvkoo8pq3

Using async functions directly as effects is intentionally disallowed. The problem is that props or state may change while your async function is running. So you risk getting very confusing behavior.

Separating async logic and cleanup lets you properly handle this case. Again, this example shows why it’s important: https://codesandbox.io/s/jvvkoo8pq3. Here, we handle the case where responses come out of order.

Your proposed alternative doesn’t work for this because we need to run cleanup before the async function finishes. Indeed our goal is to stop it from finishing because the data is fetched is no longer up to date.

Was this page helpful?
0 / 5 - 0 ratings