Does This Hook Earn Its Place? Ten Review Questions for React Code

Most React Hook bugs do not look like bugs in a pull request. A missing dependency, an effect that copies props into state, or a useCallback around a handler nobody memoizes all pass review, then surface later as stale screens, extra renders or piled-up listeners. Below, ten frequent Hook mistakes are turned into review questions you can ask of any component, with the mechanism behind each one and the simpler shape the code usually wants.

Questions about state

Can this value be computed during render?

A common pattern keeps fullName in state and uses an effect to call setFullName(`${firstName} ${lastName}`) whenever either name changes. The component renders once with the old value, the effect runs after paint, and the setter schedules a second render. For one frame the UI is inconsistent, and there is an extra moving part to maintain.

Compute it instead: const fullName = `${firstName} ${lastName}`;. React re-runs the component on every render, so the value is always in sync with its inputs. If something can be derived from props or state, it is not state. See also why syncing state with useEffect backfires.

Is the component holding more state than it needs?

The same mistake at a larger scale: a form with three useState calls for firstName, lastName and fullName. Every name change must now be mirrored into fullName, and any path that forgets creates a bug. Keep the two independent inputs and derive the third. For each useState, ask whether the value could be rebuilt from the others; if so, delete it.

Does changing this value need to update the screen?

State exists to drive rendering. A timer id, a DOM node, a previous prop value or a third-party instance (a map, a chart, a socket client) usually does not affect what is drawn, so useState(null) for it means every assignment triggers a pointless render. Use const timerRef = useRef(null); instead: writing timerRef.current persists across renders without scheduling one. The flip side is that you should never read a ref to decide what to display. If the UI must reflect the value, it is state after all.

Questions about effects

Does the dependency array list everything the effect reads?

Passing [] so an effect "runs once", as in useEffect(() => { fetchUser(userId); }, []);, freezes the userId from the first render. When the prop changes, nothing refetches and the UI shows the wrong user. Writing [userId] fixes it.

The array is not a scheduling knob; it declares which values the effect reads. If the effect runs too often, change what it reads (move a function inside it, depend on a primitive rather than an object) instead of hiding dependencies. The react-hooks/exhaustive-deps lint rule catches nearly all of these cases, so treat its warnings as errors. Fetching effects should also ignore or abort stale requests in their cleanup when userId changes quickly.

Can this effect trigger itself?

useEffect(() => { setCount(count + 1); }, [count]); is a loop: the update changes count, which re-runs the effect, forever. The functional updater setCount(prev => prev + 1) reads the latest value without needing count in the closure, so the dependency can go and the effect runs only on mount. Note that it still increments once per mount, and Strict Mode mounts twice in development, so you may see it jump by two locally.

The better question is often whether the update belongs in an effect at all. Responses to user actions go in event handlers; derived values are computed during render. Effects are for synchronizing with systems outside React.

Does the effect undo what it sets up?

An effect that only calls window.addEventListener('resize', handleResize) adds a new listener on every mount and never removes it. Return a cleanup function that calls window.removeEventListener with the same handler. React runs it before the effect re-runs and on unmount. Without it you get memory leaks, handlers firing several times and updates to components that are gone.

A quick check: every add, connect or setInterval in an effect needs a matching remove, disconnect or clearInterval in its return value. The ten-bug useEffect audit covers more effect failure modes.

Questions about memoization

Is this calculation actually expensive?

Wrapping a string concatenation in useMemo with [user] as the dependency costs more than it saves. React still allocates the callback and the array and compares dependencies every render, which is more work than joining two strings. The code is harder to read, and if user is a new object each render the memo recomputes anyway.

Use the plain expression. Reach for useMemo for measurably expensive work, such as sorting or filtering a large list, or when a memoized child or another Hook needs a stable object reference. Measure with the React DevTools Profiler first.

Does anything depend on this function's identity?

useCallback keeps a function reference stable, which only matters when something compares references: a child wrapped in React.memo or another Hook's dependency list. Wrapping a click handler that just logs, attached to a plain button, gains nothing, because the DOM does not care whether the function is new. Write an ordinary function and add useCallback when you can name the consumer that needs it. The React Compiler can apply such memoization automatically; check the current docs for its status in your setup.

Questions about structure

Is every Hook called unconditionally, in the same order?

Declaring useState only inside if (isLoggedIn) breaks the Rules of Hooks. React matches Hooks by call position in a per-component list, not by name. Skip one call and every later Hook reads its neighbor's slot, producing wrong values or a runtime error about a changed number of Hooks.

Call all Hooks at the top level, then branch: declare the user state first and return null afterwards if the user is not logged in. The same applies to loops and early returns. For the internals, see why the Rules of Hooks exist.

Is this component doing too many jobs?

A component mixing hundreds of lines of fetching, state and business logic with markup is hard to read, test and change. Extract the logic into a custom Hook. A useUserData(userId) Hook owns the user state and the effect that calls fetchUser(userId).then(setUser) when userId changes, then returns user. UserProfile shrinks to const user = useUserData(userId); plus rendering user?.name.

This gives you:

• Reuse without copying logic between components.
• Tests for the logic independent of markup.
• Shorter components that are easy to scan.
• One place to change fetching or caching.

Remember that each caller of a custom Hook gets its own state: it shares logic, not data.

Key takeaways

• Derived values are expressions, not state and not effects.
• Keep state minimal; values that never reach the screen go in refs.
• Dependency arrays describe what an effect reads, so lint them honestly.
• Every effect that sets something up must tear it down.
• Memoize only when a measurement or a reference-sensitive consumer justifies it.
• Call Hooks unconditionally, and move large logic blocks into custom Hooks.

Most of these problems come from reaching for a Hook out of habit. Asking "do we really need this Hook here?" in review is cheap and saves long debugging sessions.