I review a lot of React code, and roughly nine out of ten useEffect calls I see in pull requests should not be useEffects at all. They are state derivations dressed up as effects, sequencing logic that re-renders three times to settle, or fetches that belonged in a route loader. Each one works in isolation. Each one becomes a bug when the parent re-renders for the wrong reason, when StrictMode runs the effect twice in development, or when the dependency array drifts away from the function body it was supposed to track.

My stance, sharpened over the last few years: useEffect is the React API for synchronizing with something outside React. A DOM node, a subscription, a timer, an analytics SDK. That is the job. Almost every other use I see is either avoidable or wrong, and the React docs effectively say so since the 2023 "You Might Not Need an Effect" rewrite. I rewrote my own habits in line with that doc, and the five mistakes below are the ones I had to actively unlearn.

The five things I stopped doing

In order of how often I still find them in code review:

Mistake / fix
  1. Deriving state inside an effect      -> compute during render
  2. Chaining effects to sequence updates -> one event handler, one update
  3. Fetching from useEffect on mount     -> route loader, server component, or library cache
  4. Stale closures from missing deps     -> include the dep, or use a ref intentionally
  5. Cleanup that does not actually undo  -> mirror every subscribe with an unsubscribe

None of these are theoretical. Each one I will walk through with the code I keep deleting and the code I keep replacing it with.

Mistake 1: deriving state I could have computed

The shape that should always raise a flag in review:

function Cart({ items }) {
    const [total, setTotal] = useState(0);

    useEffect(() => {
        setTotal(items.reduce((sum, item) => sum + item.priceCents, 0));
    }, [items]);

    return <div>Total: ${(total / 100).toFixed(2)}</div>;
}

This renders twice on every items change. First render: items is new, total is stale. The effect runs after that render, calls setTotal, schedules another render. Second render: total is finally correct. The user does not see the intermediate render in most cases because React batches things and the gap is fast, but the work happened, and any code that reads total during the first render reads stale data.

The rewrite is to compute during render:

function Cart({ items }) {
    const total = items.reduce((sum, item) => sum + item.priceCents, 0);
    return <div>Total: ${(total / 100).toFixed(2)}</div>;
}

No state, no effect, no second render. If the reduce is genuinely expensive (it almost never is for cart-sized arrays), wrap it in useMemo with items as the dependency. Even then I write the useMemo only after I have measured a real performance problem, not preemptively.

The heuristic: if a useEffect's only job is to call a setter based on props or other state, the effect is wrong. Compute the value where it is read, or memoize it. State exists for things that change independently of inputs. Derived data is not state.

Mistake 2: chaining effects to sequence updates

The shape that ships in too many PRs:

function CountrySelector({ user }) {
    const [country, setCountry] = useState(null);
    const [city, setCity] = useState(null);
    const [postalCode, setPostalCode] = useState(null);

    useEffect(() => {
        if (user) setCountry(user.country);
    }, [user]);

    useEffect(() => {
        if (country) setCity(null);  // reset on country change
    }, [country]);

    useEffect(() => {
        if (city) setPostalCode(null);  // reset on city change
    }, [city]);

    // ... render
}

What that does on a single user change: render with old state, run effect 1, render with new country, run effect 2, render with cleared city, run effect 3, render with cleared postal code. Four renders. Three effect runs. And every one of those effects is a place where a future change can introduce a stale read or an infinite loop.

The right shape is one event handler that does the cascade synchronously, in a single state transition:

function CountrySelector({ user }) {
    const [location, setLocation] = useState({
        country: null,
        city: null,
        postalCode: null,
    });

    function handleCountryChange(country) {
        setLocation({ country, city: null, postalCode: null });
    }

    function handleCityChange(city) {
        setLocation((prev) => ({ ...prev, city, postalCode: null }));
    }

    // ... render
}

One setState call per user action. One re-render. The reset semantics are visible in the handler, not scattered across three effects. If user data needs to seed the form, that is a useState(() => deriveInitialLocation(user)) lazy initializer, not an effect.

The heuristic: effects are not the React way to express "when X changes, also change Y". That is what event handlers and reducers are for. Effects are for crossing the boundary between React and something else.

Mistake 3: fetching where the data did not belong

This one is more about ecosystem than React itself, but it lives in useEffect so it counts. The shape:

function UserProfile({ userId }) {
    const [user, setUser] = useState(null);
    const [loading, setLoading] = useState(true);

    useEffect(() => {
        let cancelled = false;
        setLoading(true);
        fetch(`/api/users/${userId}`)
            .then((r) => r.json())
            .then((data) => {
                if (!cancelled) {
                    setUser(data);
                    setLoading(false);
                }
            });
        return () => { cancelled = true; };
    }, [userId]);

    if (loading) return <Spinner />;
    return <div>{user.name}</div>;
}

This is what every tutorial taught for years, and it is wrong for almost every modern React project. It does not handle race conditions on slow connections (the cancelled flag is a partial fix, not a full one), it has no cache, no retry, no revalidation. In StrictMode in development it fires twice. If two siblings mount with the same userId, you fetch twice.

In 2026 the answer is one of three things, not useEffect.

If the page is a route, fetch in the route loader. React Router's loader, Next.js server components, Remix loaders, TanStack Router loaders. The data arrives before the component renders, so there is no loading state in the component itself.

If the page is interactive and needs a cache across navigations, use TanStack Query (formerly react-query) or SWR. Both give you caching, deduplication, retry, focus revalidation, and they are roughly thirty lines to wire up.

function UserProfile({ userId }) {
    const { data: user, isLoading } = useQuery({
        queryKey: ['user', userId],
        queryFn: () => fetch(`/api/users/${userId}`).then((r) => r.json()),
    });

    if (isLoading) return <Spinner />;
    return <div>{user.name}</div>;
}

If the data lives in a global store (Zustand, Redux, Jotai), fetch in a store action and read from the store. The component subscribes; the store handles the lifecycle.

The only time I still write a fetch inside useEffect is for a one-shot, non-cacheable, component-local request that is not worth a query library. Telemetry pings, maybe. Real product data, never.

Mistake 4: missing dependencies, stale closures

This is the bug class the exhaustive-deps lint rule was invented to prevent, and the bug class teams disable that lint rule to silence. Both moves miss the point. The rule is right; the code is wrong.

function SearchBox({ onSubmit }) {
    const [query, setQuery] = useState('');
    const [results, setResults] = useState([]);

    useEffect(() => {
        const id = setInterval(() => {
            // Stale closure: this captures the FIRST `query`, not the current one.
            console.log('polling with', query);
            fetch(`/api/search?q=${query}`)
                .then((r) => r.json())
                .then(setResults);
        }, 5000);
        return () => clearInterval(id);
    }, []); // eslint-disable-line react-hooks/exhaustive-deps

    return <input value={query} onChange={(e) => setQuery(e.target.value)} />;
}

The interval was created on the first render. Its closure captured query from that render: the empty string. Every five seconds it polls with the empty string forever, even though the user has typed five characters since.

Two correct fixes, depending on intent.

Fix one, the simple one: include query in the dependency array, so the interval is recreated when the query changes:

useEffect(() => {
    const id = setInterval(() => {
        fetch(`/api/search?q=${query}`).then((r) => r.json()).then(setResults);
    }, 5000);
    return () => clearInterval(id);
}, [query]);

This tears down and recreates the interval every keystroke. For a five-second interval that is fine. For a one-millisecond animation frame loop it would be a disaster.

Fix two, when you genuinely want one stable interval that reads the latest value: store the latest query in a ref and read through the ref inside the interval callback.

const queryRef = useRef(query);
useEffect(() => { queryRef.current = query; }, [query]);

useEffect(() => {
    const id = setInterval(() => {
        fetch(`/api/search?q=${queryRef.current}`).then(/* ... */);
    }, 5000);
    return () => clearInterval(id);
}, []);

The ref is the explicit "I want a mutable value that is not part of render output" channel. The lint rule is happy because the effect does not depend on query directly; the ref is stable across renders.

Which fix is right depends on what you want. The wrong move, every time, is to silence the lint rule and keep the bug.

Mistake 5: cleanup that did not actually clean up

The last one is the hardest to spot in review because the bug only shows up under specific conditions: StrictMode double-mount in development, or fast remounts in production (route changes, parent re-keying, conditional rendering).

useEffect(() => {
    socket.connect();
    socket.on('message', handleMessage);
    // No cleanup. Or partial cleanup.
}, []);

When the component unmounts (or, in StrictMode, mounts and unmounts and mounts again), the socket listener is still registered. The next mount registers a second one. After three remounts there are four listeners; every message fires the handler four times.

The rule I follow now: every subscribe-shaped call inside an effect must be paired with an unsubscribe-shaped call in the cleanup, and the cleanup must be exactly the inverse of the setup.

useEffect(() => {
    socket.connect();
    socket.on('message', handleMessage);
    return () => {
        socket.off('message', handleMessage);
        socket.disconnect();
    };
}, []);

React 18's StrictMode will mount, unmount, and mount your component again in development specifically to surface this class of bug. It is not a hostile feature; it is a free unit test for cleanup correctness. If your effect breaks in StrictMode, it would have broken in production the moment a user navigated to your page twice in a row.

The checklist I run mentally for every effect:

• Setup runs N times. Cleanup must run N times too. Setup count equals cleanup count.
• After cleanup, the world should look exactly the way it did before setup. No leftover listeners, no leftover timers, no leftover requests in flight.
• If the cleanup needs more state than the closure can give it, capture that state in the setup and reference it in the cleanup. Do not read from someRef.current in cleanup if the ref might have been reassigned by a later effect.

The rule I default to now

useEffect has exactly one good reason to exist in my code: I am crossing a boundary that React does not know about, and I need to keep the inside in sync with the outside. A WebSocket I subscribe to. A <canvas> I draw on. A MutationObserver I attach to a DOM node React is not managing. An analytics call that has to fire when a particular page becomes visible. Those are real synchronizations. Effects are the right tool for them.

Everything else is doing the wrong job with the wrong hook. Derived values belong in render or useMemo. State transitions belong in event handlers or reducers. Data fetching belongs in route loaders or query libraries. Stale closures belong in lint-rule violations that I fix instead of silence. Missing cleanups belong in code review comments that say no.

When I write a useEffect now, I write it last, after I have tried to solve the problem without one. Most of the time I find I do not need it. The codebases that adopted that habit got faster, less buggy, and easier to refactor, all from the same single discipline: stop reaching for useEffect first.