Smell-checking AI-suggested React hooks before you merge

AI is weirdly confident about React hooks. Ask for “extract the data fetching into a custom hook” and you’ll get something that looks idiomatic: useSomething, a useEffect, a couple of useMemo s, maybe a useCallback for good measure. Types pass. The demo works once. Then you merge and discover the effect fires twice on every keystroke, or the cleanup never ran, or the hook is just a wrapper around three lines that didn’t need a name.

I treat AI-suggested hooks like AI-suggested refactors: useful drafts, not mergeable by default. Here’s the smell checklist I run before I accept the branch.

useEffect (() => { fetchUser ( userId ). then ( setUser ); }, []); // "run once on mount" Enter fullscreen mode Exit fullscreen mode Except userId changed three routes ago, and the effect still holds the first id. Or the opposite lie:

useEffect (() => { analytics. track ( " view ", { id: item. id }); }, [ item ]); // object identity thrash Enter fullscreen mode Exit fullscreen mode Every parent re-render invents a new item object, so “track once” becomes “track forever.”

What I check: for every useEffect, I ask “what should actually retrigger this?” If the answer is “once,” I want an explicit id/primitive in the deps, or an abortable fetch keyed by that id — not an empty array with a closed-over value. If the answer is “when X changes,” I want X to be a stable primitive or a memoized value I trust, not a fresh object from JSX.

Classic AI move: wrap a handler in useCallback, then put that callback in another hook’s deps… while the callback itself depends on props that change every render.

const onSave = useCallback (() => { save ( formState ); }, [ formState ]); // formState changes constantly useEffect (() => { window. addEventListener ( " keydown ", onSave ); return () => window. removeEventListener ( " keydown ", onSave ); }, [ onSave ]); // rebinds every keystroke of formState Enter fullscreen mode Exit fullscreen mode useCallback didn’t stabilize anything. It just moved the thrash one layer down.

What I check: if a callback is only a dep because another effect needs it, I prefer reading the latest value from a ref, or narrowing the callback’s deps to the primitives it actually needs. “Add useCallback ” is not a fix by itself.

The agent sees fullName used in three places and invents:

const [ fullName, setFullName ] = useState ( "" ); useEffect (() => { setFullName ( ` ${ first } ${ last } ` ); }, [ first, last ]); Enter fullscreen mode Exit fullscreen mode That’s derived data. It doesn’t need state. It needs:

const fullName = ` ${ first } ${ last } `; Enter fullscreen mode Exit fullscreen mode Or a useMemo if the derivation is actually expensive (it almost never is for string concat).

What I check: any useState + useEffect pair that only syncs from other props/state is guilty until proven otherwise. AI does this a lot because “state” feels like the React vocabulary for “a value I can name.”

Fetch-in-effect without abort. Subscriptions without unsubscribe. setTimeout without clear. The demo looks fine because you never navigate away mid-request.

useEffect (() => { let cancelled = false; load ( id ). then (( data ) => { if (! cancelled ) setData ( data ); }); return () => { cancelled = true; }; }, [ id ]); Enter fullscreen mode Exit fullscreen mode That’s the minimum. Better: AbortController so the network work actually stops. AI often ships the happy path only.

What I check: every effect that starts async work, timers, or listeners must have a cleanup that matches. I also watch for “cleanup” that only sets a flag but still lets expensive work continue — fine for setState races, not fine for upload loops.

function useIsOpen ( initial = false ) { const [ open, setOpen ] = useState ( initial ); return { open, open: () => setOpen ( true ), close: () => setOpen ( false ) }; } Enter fullscreen mode Exit fullscreen mode Cute. Also a digression in code review. Custom hooks earn their name when they encapsulate a non-obvious contract: syncing with a store, coordinating multiple effects, sharing behavior across distant components. “Boolean with open/close” is just state.

What I check: would I explain this hook in a PR description without sounding silly? If the hook exists only so the component file is shorter, I inline it. AI loves extracting; I love deleting.

function useSearch ( items: Item []) { const [ query, setQuery ] = useState ( "" ); const filter = () => items. filter (( i ) => i. name. includes ( query )); return { query, setQuery, results: filter () }; } Enter fullscreen mode Exit fullscreen mode Looks fine until filter gets passed to a child as a callback and the child memoizes it, or until someone moves filter into a useEffect with []. Closures over items and query go stale in ways that are hard to see in a generated diff.

What I check: values computed from props/state should either be plain expressions in render, or memoized with the right deps — not casually closed over inside nested functions the agent plans to “reuse.”

When something re-renders too often, the model’s first instinct is memoization:

const sorted = useMemo (() => [... items ]. sort ( byName ), [ items ]); const visible = useMemo (() => sorted. filter ( isVisible ), [ sorted, isVisible ]); const rows = useMemo (() => visible. map ( toRow ), [ visible ]); Enter fullscreen mode Exit fullscreen mode Three memos, still thrashing if items is a new array every parent render. Memo is a tax you pay after you’ve fixed identity and state shape — not a perfume you spray on the smell.

What I check: before accepting a memo, I ask what identity is unstable upstream. Fix that first. Keep useMemo for real work (big lists, expensive pure transforms), not for obj.a + obj.b.

The scariest AI hooks look pure in the name and impure in the body:

function useCartTotal ( cart: Cart ) { useEffect (() => { localStorage. setItem ( " cart-total ", String ( cart. total )); }, [ cart. total ]); return cart. lines. reduce (( s, l ) => s + l. price, 0 ); } Enter fullscreen mode Exit fullscreen mode Callers think they’re reading a number. They’re also writing storage. Side effects buried in “getter” hooks are how you get “why did navigating to settings clear my draft?” bugs.

What I check: naming. useX that returns data should not silently write, fetch, or mutate globals. If it does I/O, the name should say so ( usePersistCartTotal ) and the call site should look intentional.

Before I merge an AI-touched hook, I want yes on all of these:

If two or more fail, I don’t “tweak the hook.” I ask the agent for a smaller change: inline the logic, or rewrite the effect with an explicit contract in the prompt.

When I do want a custom hook from the model, I fence it:

Extract a hook only if the behavior is reused or the effect graph is non-trivial. Prefer derived values over syncing state in effects. Every useEffect must list real deps and a cleanup. Do not add useMemo / useCallback unless you can point to a measured or structural reason. No hidden I/O in hooks named like getters.

That doesn’t make the model perfect. It makes the first draft close enough that the smell checklist is a quick pass instead of a rewrite.

Hooks are easy to generate and expensive to debug after merge. Five minutes of smell-checking beats an afternoon of “why does this fire twice in Strict Mode only on the settings page.”