DEV Community

Thibault Friedrich
Thibault Friedrich

Posted on

You are using `useEffect` the wrong way 🤯

This article won't describe the API of the React hook useEffect, so refer to the documentation or other articles.

Very often while reviewing PRs, I see developers using the React hook useEffect the wrong way. Behind these errors, multiple reasons coexist. They reduce the performances and even worst, reduce the readability of the code. I know that some of these situations are even visible on the React website but you should avoid using them.

These are few examples:

To add side-effect when some data changed

Developers find smart to move side-effects in dedicated components to simplify the logic.

function Component({ value }) {

  useEffect(() => {
    saveValueOnServer(value)
  }, [value])
}

function Parent() {
  const [value, setValue] = useState()

  return (
    <>
      <Component value={value} />
      <input value={value} onChange={event => setValue(event.target.value)} />
    </>
  )
}
Enter fullscreen mode Exit fullscreen mode

This pattern is inspired by the event-driven interface design from some UI framework like Qt in C++ for example. But it is not the way in react.

  • useEffect requires a variable change so you might be inclined to create artificial variable to trigger this side-effect
  • a event system already exists in javascript so you should this one instead
  • when you are new in the project, it is very hard to track where the side-effects are triggered. Debugging will be harder. It becomes even impossible when you start using data from a data system management like redux.

Solution

When you want to add a side-effect to an operation, add it directly after the operation. It will easier to debug in the future.

function Parent() {
  const [value, setValue] = useState()

  const handleChange = (event) => {
    const newValue = event.target.value
    setValue(newValue)
    saveValueOnServer(newValue) // <-- the code just after will be easier to debug
  }

  return (
    <input value={value} onChange={event => setValue(event.target.value)} />
  )
}
Enter fullscreen mode Exit fullscreen mode

To memoize data

Developers use sometimes useEffect to memoize data:

function Component({ value }) {
  const [computedValue, setComputedValue] = useState()

  useEffect(() => {
    setComputedValue(computeValue(value))   
  }, [value])

  return <>{computedValue}</>
}
Enter fullscreen mode Exit fullscreen mode

This is a wrong way to memoize value for 3 reasons:

  • it requires more code than useMemo
  • useEffect is executed after the render so it means the value will always be displayed with a short time lag (very short but sill existing)
  • the combo useEffect + useState forces 2 renders instead of one

The solution

Use useMemo which is dedicated for this usage:

function Component({ value }) {
  const computedValue = useMemo(() => computeValue(value), [value])

  return <>{computedValue}</>
}
Enter fullscreen mode Exit fullscreen mode

The only reason you would use useEffect for memoization is if the computation of the value is really long and you don't want to slow down the render and prefer to run the computation in the background.

To fetch data

This one is definitely the most frequent Dark pattern and even tutorial suggest this approach:

function Component() {
  const [data, setData] = useState()

  useEffect(() => {
     const fetch = async () => {
        setData(await fetchData())
     }

     fetch().then(() => {}).catch(() => {})
  }, [])

  return <>{data}</>
}
Enter fullscreen mode Exit fullscreen mode

This pattern is really underperforming because:

  • it starts fetching data after the first render instead of before
  • fetching data is asynchronous while useEffect callback is synchronous so developers have to use workarounds and it makes the code messy.

Solution

I suggest to use the hook useQuery from @tanstack/react-query

If you don't want an extra library just for this hook, you can use a custom hook like:

const useOnce = (callback: () => Promise<void>) => {
  const executed = useRef(false)

  if (!executed.current) {
    callback().then(() =>{}).catch(() => {})
    executed.current = true
  }
}

function Component() {
  const [data, setData] = useState()

  useOnce(async () => {
     setData(await fetchData())
  })

  return <>{data}</>
}
Enter fullscreen mode Exit fullscreen mode

The most important reason

useEffect gives a lot of flexibility. But this flexibility is also a doom since as a developer, you won't be understand the goal of this hook in a blink. You will need to dig deeper than with a useMemo or a useQuery.

So please prefer the dedicated hooks to increase the readability of the code.

When you have the right to use useEffect

The only right reason to use useEffect is to attach a listener to a DOM element. Of course you also have some edge cases. However from my experience, 80% of the time, developers use the hook useEffect for situations that are not edge cases. So if you are not sure, don't use it.

function Component() {
  const container = useRef()

  useEffect(() => {
    const onClick = () => {
      // do something
    }

    container.current.addEventListener(onClick)

    return () => container.current.removeEventListner(onClick)
  },[])

  return (
    <div ref={container}>
    </div>
  )
}
Enter fullscreen mode Exit fullscreen mode

So please keep your code more readable and use the hook useEffect less often. All your teammates will thank you.

Top comments (0)