Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I think I've seen all of these at some point (:

Unless I'm missing something, I think the "Async effect function" solution is not correct:

  React.useEffect(() => {
    async function runEffect() {
      // Effect logic here
    }
    runEffect();
    return () => {
      // Cleanup logic here
    }
  }, [userId]);
This looks like a race-condition where cleanup may be done before `runEffect` has finishes resolving, and any errors thrown in `runEffect` will be unhandled promise rejections.


This should be number 1 on the list, and it's wrong.

Specifically:

    React.useEffect(() => {
      // Create an async function...
      async function runEffect() {
        const url = `${API}/get-profile?id=${userId}`;
        const res = await fetch(url);
        const json = await res.json(); <-- OK
        
        setUser(json); <--- Wrong.
    }
```

This is, hands down, the #1 mistake that react developers make. I've seen it literally in every react code base I've ever opened (if the code base had any kind of `fetch` in it).

It's covered in the documentation now, explicitly:

https://beta.reactjs.org/learn/synchronizing-with-effects#fe...

This is a prime example of a sharp edge with react hooks; it's a very very common pattern, but because people don't understand what's going on, they do it wrong naively; and even when you do know it's wrong, it's a rare edge-case that causes an invisible bug ("Tried to update a component that wasn't mounted...") and makes code messy and hard to read.

The tldr should be: Do not use async effects. Use suspense.


> It's covered in the documentation now, explicitly:

> Do not use async effects. Use suspense.

The linked documentation does not mention the word "suspense".

Do you have a link that combines these two topics?

Instead the React docs explain a workaround that leads to completely different network request behaviour across development and production:

> In development, you will see two fetches in the Network tab. > In production, there will only be one request.

The proposed solution in the docs now:

> useSomeDataLibrary()

without saying what that "some data library" could be.


The section in the beta docs you linked to shows an async function startFetching() which does setTodos(json). How does that reconcile with you saying that setUser(json) is wrong?


> Do not use async effects. Use suspense.

I have to wonder how much pain would have been saved if the React team had just shipped a `usePromise` hook years ago.


The first thing I noticed looking at React after Svelte was that Svelte had syntax for promises from v1 while dealing with Promises in "the most popular framework on the plane" is still a PITA.

"await blocks" on http://web.archive.org/web/20180103230006/http://svelte.tech...


Yes, that's dangling async behaviour for sure.

Curiously, it's also very similar to the example in the beta docs that wokwokwok just linked to.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: