Things I Keep Fixing in React Codebases

None of these are clever. They're the things I find myself pointing out in code review over and over, in projects of very different sizes and quality. Each one is small on its own. Together they're most of the difference between a React codebase that's pleasant to work in and one that isn't.

Every map in JSX gets a key

When you render a list with map, the outermost element inside the callback needs a key. React uses it to work out which items were added, removed, or moved between renders. Without it, React falls back to matching by position, and the moment the list is reordered or filtered, component state starts sticking to the wrong rows.

const ids = [1, 2, 3, 4, 5]

function List() {
  return (
    <ul>
      {ids.map((id) => (
        <li key={id}>{id}</li>
      ))}
    </ul>
  )
}

The key should be something stable that identifies the item, usually an id from your data. Using the array index works until the list changes shape, which is exactly when you need it to work. React also warns about missing keys in the console, which brings me to a later point about not ignoring warnings.

Let ESLint enforce the Next.js rules

Next.js ships an ESLint config that encodes most of the framework's own advice: use next/image instead of <img>, use next/link for internal navigation, don't load scripts in ways that block rendering, and so on. Turning it on is one line:

{
  "extends": "next/core-web-vitals"
}

Rules aren't laws. If one of them is wrong for your case, override it in the same file instead of adding an inline comment to every call site:

{
  "extends": "next/core-web-vitals",
  "rules": {
    "@next/next/no-img-element": "off"
  }
}

The point is that the decision is made once, in one place, with the reasoning next to it. Everyone on the team inherits it.

Don't send requests with undefined in the URL

Open the network tab on almost any React app and you'll eventually see something like this:

GET /api/posts-by-user-id/undefined
GET /api/posts-by-user-id/undefined/categories

It happens because the id comes from the router, or from a parent component, and on the first render it isn't there yet. The request goes out anyway. At best it's a wasted round trip and a rejected request in your logs. At worst the backend doesn't validate the parameter well, tries to parse "undefined" as a number or a UUID, and responds with a 500 that now shows up in your error tracking as if something real broke.

The fix is to not fire the request until you actually have what it needs. With react-query that's the enabled option:

const router = useRouter()
const blogId = router.query.id

const { data } = useQuery({
  queryKey: ["blog", blogId],
  queryFn: () => axios.get(`/api/blogs/${blogId}`),
  enabled: typeof blogId === "string",
})

While blogId is undefined, the query simply doesn't run. Once the router has the value, it does. The same idea applies to plain useEffect fetches: check for the value first, or return early.

Build URLs in one place

I've lost count of how many times I've seen this:

<Image src={config.API_BASE_URL + "public/uploads" + user.image} />

And then the same expression in forty other files, five times in each. The public/uploads prefix is an implementation detail of the backend. When it changes, or when the files move to a CDN, you get to find and edit every copy, and you will miss one.

Put it behind a function:

const constructStaticFileUrl = (...paths: string[]) =>
  config.API_BASE_URL + "public/uploads/" + paths.join("/")

<Image src={constructStaticFileUrl(user.image)} />
<Image src={constructStaticFileUrl(user.id, "images", "cover.png")} />

Now there's exactly one place that knows how static file URLs are shaped. It's also the natural place to add encoding or sanitising later, which is much harder to retrofit across forty files.

Know when to stop being DRY

The previous point is a case for not repeating yourself. This one is the counterweight, because I've seen DRY taken too far more often than not far enough.

The story usually goes like this. A component is used on two pages, so it gets extracted into a shared one. Then one page needs a slightly different header. A prop gets added. Then the other page needs a different footer, and a different empty state, and a different click handler. Each change adds a prop and a conditional. A year later the component takes fifteen props, half of them booleans, and nobody can change it without breaking a page they didn't know used it.

Some rules of thumb I actually follow:

• Two copies is fine. Two straightforward components are easier to maintain than one component that's contorting itself to serve both.
• Pick a threshold, say three uses, and only extract something shared when you cross it. Until then, duplication is cheaper than the wrong abstraction.
• When a shared component starts accumulating conditionals, break it into smaller components and push the conditional logic down to the piece that actually varies, instead of threading flags through the top.
• If what varies is behaviour rather than markup, move the behaviour into a hook. Different pages can use different hooks with the same component underneath.

The goal isn't to minimise lines of code. It's to make sure that when you change something, you can predict what else changes.

Clean up your console.logs

Logging while you debug is fine. Leaving it in is not.

The obvious cost is noise. Someone else is working on a different feature, opens the console, and gets a wall of output from your component that they have to mentally filter out. The less obvious cost is that the noise hides real problems. When a component throws and the error gets pushed off the screen by fifty lines of console.log(data), it's easy to never see it.

Before you push, search the diff for console. and remove what you added. If a log is genuinely useful to keep, it probably wants to be behind a proper logger with levels, not a bare console.log.

Treat warnings and errors as a to-do list

A codebase where the terminal and the browser console are full of warnings is a codebase where nobody reads warnings any more. That's the real damage. The next warning that matters gets the same treatment as the hundred that didn't.

So don't let them pile up. Each warning gets one of two outcomes:

1. Fix it.
2. Decide, deliberately, that the tool is wrong in this specific case, and silence it right there with an inline eslint-disable-next-line and a short comment saying why.

Sometimes you do know better than the linter. That's fine. What's not fine is leaving the warning in place so that the next person has to work out whether it's known and accepted or a bug waiting to happen.

That's the list

Nothing here requires a library or a refactor. It's mostly a matter of noticing, and of deciding as a team that these things matter enough to fix in review. In my experience they pay for themselves within weeks.