This are 5 most common mistakes that people do while using use effect hook.

Use effect is the most complicated hook in React and people make lots of mistakes while using it. Also it's super easy to get stale data if you not use it correctly.

#1 Infinite loop

The most common mistake is to trigger infinite loop with useEffect

import { useState, useEffect } from "react";

const App = () => {
  const [arr, setArr] = useState([]);
  useEffect(() => {
    setArr([1]);
  });
  return <div>App</div>;
};

export default App;

So here we set a state inside useEffect after every render of the component. But setting of the state causes rerender which means we get an infinite loop: useEffect -> setting state -> render -> and it will happen all over again.

So actually we have in useEffect an array of dependencies. So we might write our array as a dependency and then useEffect will be triggered not every time but only if array changed. And it won't fix our problem. Which brings us to the second most common mistake.

  useEffect(() => {
    setArr([1]);
  }, [arr]);

#2 Wrong dependencies

React compares changing of dependencies in the same way how you do comparison in JavaScript. Which means that you can compare properly only primitives like string, number, boolean but not arrays or objects because they are never equal.

There is actually a custom deep comparison hook if you really want to use arrays or objects as dependencies but normally in 99% cases it just means that you don't solve your problem in the correct way and you need to write your code in another way.

#3 Overcomplicate code

I see quite often that people are overusing useEffect just because it looks cool

import { useState, useEffect } from "react";

const App = () => {
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState(null);
  const [todos, setTodos] = useState(null);

  const fetchTodos = () => {
    setLoading(true);
    callApi()
      .then((res) => {
        setTodos(res);
      })
      .catch((err) => setError(err))
      .finally(() => setLoading(false));
  };

  useEffect(() => {
    fetchTodos();
  }, []);

  useEffect(() => {
    if (error) {
      history.push("/");
    }
  }, [error]);

  return <div>App</div>;
};

export default App;

As you can see we have 2 useEffects. One will be called on initialize to fetch todos and one which listens for error property and will trigger redirect to homepage. And it may look logical but actually it doesn't make sense to make it more complex then we need. If we just have a component with a single fetch then we can directly trigger history.push inside a catch and avoid creating additional useEffect.

import { useState, useEffect } from "react";

const App = () => {
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState(null);
  const [todos, setTodos] = useState(null);

  const fetchTodos = () => {
    setLoading(true);
    callApi()
      .then((res) => {
        setTodos(res);
      })
      .catch((err) => {
        setError(err);
        history.push("/");
      })
      .finally(() => setLoading(false));
  };

  useEffect(() => {
    fetchTodos();
  }, []);

  return <div>App</div>;
};

export default App;

So don't overcomplicate your code if it's not needed.

#Bonus Cleanup your async requests

One most problem is to forget cleaning up your async requests. It may happen that you fetch some data in component and set them in state on success. This code will break if you will change the page and your component will be destroyed.

There are several ways to avoid this but actually what we want to do is avoid setting the state after destroy of the component.

const App = () => {
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState(null);
  const [todos, setTodos] = useState(null);

  useEffect(() => {
    let skipGetResponseAfterDestroy = false;
    setLoading(true);
    callApi()
      .then((res) => {
        if (!skipGetResponseAfterDestroy) {
          setTodos(res);
        }
      })
      .catch((err) => {
        if (!skipGetResponseAfterDestroy) {
          setError(err);
          history.push("/");
        }
      })
      .finally(() => {
        if (!skipGetResponseAfterDestroy) {
          setLoading(false);
        }
      });

    return () => {
      skipGetResponseAfterDestroy = true;
    };
  }, []);

  return <div>App</div>;
};

Just to remind you we in useEffect we can return a function which will be called on destroy. Which means we can set a local property in there. And use this property before you do set. In this case we will call all setters only until our component is destroyed. There are other approached which do the same thing like aborting request if component way already destroyed but you achieve the same result we don't set state that was already destroyed.

So this were most common mistakes while using useEffect hook.

Also if you want to improve your programming skill I have lots of full courses regarding different web technologies.