6 AI-Generated React Bad Smells to Catch Before You Merge

Let me start by saying this: this isn't telling you to stop using AI to write React.

I use Claude Code + Cursor every day, and my output efficiency has at least doubled. But to be blunt — the risk of directly merging AI-generated React code is higher than you think.

It's not because the AI gets the syntax wrong. It knows the syntax better than you do.

It's because React's "best practices" are a bunch of context-dependent judgments: when to use useEffect, when not to; which layer state should live at; what to use as a key; whether to memoize. These judgments require looking at the entire component tree where this code lives, and most of the time the AI only sees the few lines you pasted in.

So here are 6 bad smells that show up almost daily in AI-generated PRs. For each one I've provided real comparison code — you can spot them at a glance before merging.

---

1. Stuffing render-time calculations into useEffect

This is the most common bad smell in AI-generated React code, bar none.

What AI often writes

function OrderList({ orders }: { orders: Order[] }) {
  const [total, setTotal] = useState(0);
  const [filtered, setFiltered] = useState<Order[]>([]);

  useEffect(() => {
    const filteredOrders = orders.filter(o => o.status === 'paid');
    setFiltered(filteredOrders);
    setTotal(filteredOrders.reduce((sum, o) => sum + o.amount, 0));
  }, [orders]);

  return (
    <div>
      <p>合计: {total}</p>
      <ul>{filtered.map(o => <li key={o.id}>{o.name}</li>)}</ul>
    </div>
  );
}

Why it's bad

Three layers of problems:

1. An extra render cycle. orders changes → component renders (with old total/filtered) → useEffect runs → setState → renders again. A lot of the white flashes / flickers users perceive come from this.
2. State and props can drift out of sync. If one day you add another setFiltered branch and forget to sync total, the data is wrong.
3. One wrong dependency in the array and you get an infinite loop. AI-written code like this often misses dependencies or adds too many.

The right way

Derived data should be calculated directly during render — no state, no effect:

function OrderList({ orders }: { orders: Order[] }) {
  const filtered = orders.filter(o => o.status === 'paid');
  const total = filtered.reduce((sum, o) => sum + o.amount, 0);

  return (
    <div>
      <p>合计: {total}</p>
      <ul>{filtered.map(o => <li key={o.id}>{o.name}</li>)}</ul>
    </div>
  );
}

If the filter is genuinely expensive (e.g., tens of thousands of items with complex computation), then consider useMemo. Don't optimize prematurely.

How to spot it

When you see a useEffect in AI-generated code, first ask yourself: "Can this logic just be written directly above the return?" Most of the time, it can.

---

2. Copying props into useState

What AI often writes

function UserForm({ user }: { user: User }) {
  const [name, setName] = useState(user.name);
  const [email, setEmail] = useState(user.email);

  return (
    <>
      <input value={name} onChange={e => setName(e.target.value)} />
      <input value={email} onChange={e => setEmail(e.target.value)} />
    </>
  );
}

Looks reasonable: a form needs to be editable, so it needs state.

Why it's bad

The user.name inside useState(user.name) is only read once, when the component first mounts. After that, if the parent passes in a new user, name won't update.

This bug is especially sneaky — you can see it in local dev when switching users, but if you only test a single-user flow after the AI writes it, the PR gets merged.

Once it's in production and someone switches users, the form shows the previous user's data. Your boss will ask: "Did you even test this?"

The right way

Decide whether the form is controlled or uncontrolled. Two solutions:

A. Fully controlled — state lives in the parent

function UserForm({ user, onChange }: { 
  user: User; 
  onChange: (patch: Partial<User>) => void 
}) {
  return (
    <>
      <input 
        value={user.name} 
        onChange={e => onChange({ name: e.target.value })} 
      />
      <input 
        value={user.email} 
        onChange={e => onChange({ email: e.target.value })} 
      />
    </>
  );
}

B. Internal state, but force remount with key

<UserForm key={user.id} user={user} />

When user.id changes, React destroys the old component and creates a new one, so state picks up the new initial value.

How to spot it

Seeing useState(props.xxx) is wrong nine times out of ten. Either lift the state up or isolate with a key.

---

3. Fetching in useEffect without handling race conditions

What AI often writes

function UserProfile({ userId }: { userId: string }) {
  const [user, setUser] = useState<User | null>(null);

  useEffect(() => {
    fetch(`/api/users/${userId}`)
      .then(r => r.json())
      .then(setUser);
  }, [userId]);

  return user ? <div>{user.name}</div> : <Skeleton />;
}

Can you spot the problem at a glance?

Why it's bad

When a user quickly clicks through different users A → B → C, the fetches are sent in order, but the responses don't necessarily come back in order.

If A's request is slow over the network, C has already come back and displayed C's name, and then A's response finally arrives — the page suddenly flips back to A's data.

This isn't theoretical. It's a pitfall every frontend developer who's built a user list page has fallen into. AI almost never handles it proactively.

The right way

Use AbortController or a stale flag:

function UserProfile({ userId }: { userId: string }) {
  const [user, setUser] = useState<User | null>(null);

  useEffect(() => {
    const ctrl = new AbortController();
    fetch(`/api/users/${userId}`, { signal: ctrl.signal })
      .then(r => r.json())
      .then(setUser)
      .catch(err => {
        if (err.name !== 'AbortError') throw err;
      });
    return () => ctrl.abort();
  }, [userId]);

  return user ? <div>{user.name}</div> : <Skeleton />;
}

Or even simpler, just use a data-fetching library: SWR, React Query, TanStack Query — they all handle race conditions out of the box.

How to spot it

Whenever there's async code inside a useEffect, the cleanup function must be written. If it's missing, that's a bad smell.

---

4. Using index as list keys

What AI often writes

{items.map((item, i) => (
  <ListItem key={i} data={item} />
))}

The AI won't even hesitate when writing this line.

Why it's bad

When the list changes order, inserts in the middle, or deletes items, React uses keys to match elements between renders. Using index as the key tells React: "The first position is always the first element" — but if you've swapped the content, it won't recognize that.

Consequences:

1. If a list item contains an <input>, and you insert an item above while the user is typing, the focus jumps
2. If a list item is a component with internal state, the state gets mismatched
3. Animations and transitions break

The right way

Use a stable id from the data itself:

{items.map(item => (
  <ListItem key={item.id} data={item} />
))}

If the data genuinely has no id, the frontend can add one when receiving the data using crypto.randomUUID() or an incrementing counter — but you must guarantee the same data always gets the same id — don't generate a new one on every render.

How to spot it

Seeing key={index}, key={i}, or key={idx} — be immediately suspicious. The only exception is if the list will never reorder, never have insertions or deletions, and never contain internal state — only then is it 100% safe.

---

5. Inline objects / functions paired with memo components = memo is useless

What AI often writes

const Row = React.memo(({ style, onClick, data }: Props) => {
  return <div style={style} onClick={onClick}>{data.name}</div>;
});

function Table({ rows }: { rows: Row[] }) {
  return rows.map(row => (
    <Row
      key={row.id}
      style={{ padding: 8, borderBottom: '1px solid #eee' }}
      onClick={() => console.log(row.id)}
      data={row}
    />
  ));
}

Looks reasonable: the child component is memoized, so it should be optimized.

Why it's bad

style={{ padding: 8 }} and onClick={() => ...} are new references on every render.

React.memo uses Object.is for shallow comparison by default. New reference ≠ old reference → memo is completely bypassed → every time the parent renders, every Row re-renders.

You think you've optimized, but you've actually optimized nothing, and you've added the overhead of a shallow comparison.

The right way

Hoist inline objects outside (if they're purely static):

const rowStyle = { padding: 8, borderBottom: '1px solid #eee' };

function Table({ rows }: { rows: Row[] }) {
  const handleClick = useCallback((id: string) => {
    console.log(id);
  }, []);

  return rows.map(row => (
    <Row
      key={row.id}
      style={rowStyle}
      onClick={handleClick}
      rowId={row.id}
      data={row}
    />
  ));
}

const Row = React.memo(({ style, onClick, rowId, data }: Props) => {
  return <div style={style} onClick={() => onClick(rowId)}>{data.name}</div>;
});

Note: having Row internally generate () => onClick(rowId) is what keeps onClick a stable reference.

How to spot it

When you see React.memo, immediately scan the props at the call site — if any are {...}, [...], or () => ..., the memo is doing nothing.

---

6. Mindlessly adding useMemo / useCallback

This one is the opposite of the previous — AI also frequently abuses memo in the other direction.

What AI often writes

function Greeting({ name }: { name: string }) {
  const greeting = useMemo(() => `Hello, ${name}!`, [name]);
  const handleClick = useCallback(() => alert(greeting), [greeting]);

  return <button onClick={handleClick}>{greeting}</button>;
}

Three lines of code, two hooks.

Why it's bad

useMemo and useCallback have their own cost:

• Every render runs a shallow comparison of the dependency array
• React internally maintains an extra memory slot
• Code readability drops sharply

For a string concatenation, computing it directly is ten times faster than useMemo. The onClick passed to a button isn't going to a memo component either, so useCallback is completely pointless.

Code that abuses memo is usually slower than code without memo — because it does the same work plus an extra shallow comparison.

The right way

function Greeting({ name }: { name: string }) {
  const greeting = `Hello, ${name}!`;
  return <button onClick={() => alert(greeting)}>{greeting}</button>;
}

Only add memo in these two cases:

1. The computation is genuinely expensive (e.g., filter/sort/reduce on tens of thousands of items)
2. The value/function is being passed as a prop to a memo component, or used as a dependency of useEffect/useMemo

Otherwise — not adding it is correct.

How to spot it

When you see useMemo/useCallback, ask yourself: "If I remove it, what would actually re-compute or re-render?" If you can't answer, delete it.

---

Finally: a 60-second pre-merge checklist

If you don't have time to read the whole article, paste this into your Code Review template:

□ Can the logic inside useEffect be moved to the render phase?
□ Does useState(props.xxx) appear? If it does, it's wrong.
□ Does the fetch inside useEffect have an AbortController or cleanup?
□ Are list keys stable ids? If not, change them.
□ Do React.memo component props contain inline objects/functions/arrays?
□ useMemo/useCallback — what would happen if you deleted them? If you can't say, delete them.

Six questions, one minute to scan before merging.

---

One-line summary

AI-generated React code has correct syntax, but React isn't purely about syntax.

It's a set of judgments about "when to do something, and when to do nothing." The AI can't see your component tree, doesn't know how users interact, and has no access to production performance data — those judgments still have to be made by you.

Using AI to write code isn't shameful. Merging AI-written code without reviewing it is.