What I actually look for in frontend code review

In my last post I wrote that reading is the new bottleneck.

Here's what I actually look for when I read.

I review frontend code in a project that handles millions of rows of data. Performance is not theoretical here. A bad algorithm does not just look inelegant — it makes the application slow in ways users notice.

Over time I developed a mental checklist. Not a formal document. Not a set of rules I apply blindly. Just signals that make me slow down and ask one more question.

This is that checklist.

Unnecessary iterations

The most common thing I catch is running through the same data twice when once would do.

// ❌ two iterations
const activeValues = data.filter(item => item.active).map(item => item.value);

// ✅ one iteration
const activeValues = data.reduce<number[]>((acc, item) => {
  if (item.active) acc.push(item.value);
  return acc;
}, []);

At a thousand records this probably makes no visible difference.

At a million records, chained .filter().map() is no longer something I ignore automatically. It may still be fine. But I want the author to be aware that we are doing an extra pass and creating an intermediate array.

The fix is not complicated. The awareness is the part that requires practice.

Operations that hide intent

Some code looks clean at a glance and hides unnecessary work underneath.

// ❌ works, but hides intent
const id = Math.random().toString().slice(2);

// ✅ if you need a random ID, be explicit about it
const id = crypto.randomUUID();

The bigger issue here is not only the number of operations. It is intent.

Math.random().toString().slice(2) looks like a workaround. It creates a random number, converts it to a string, then slices part of that string and treats the result as an ID.

crypto.randomUUID() says exactly what kind of value we need.

This is the pattern I see often with generated code: it works, but it does not communicate the decision clearly. The code is accepted because the output looks reasonable, not because the implementation was questioned.

Make aggregation explicit

When I see logic like this in a review, I usually suggest a cleaner approach:

// ❌ imperative accumulator
let totalSize = 0;
cols.forEach(col => {
  totalSize += col.size ?? 0;
});

// ✅ declarative aggregation
const totalSize = (cols: CustomColumnDef<Row>[]): number =>
  cols.reduce((sum, col) => sum + (col.size ?? 0), 0);

This is not about pretending that every let is bad. It is about making the shape of the operation obvious.

The reduce version says: take these values, sum them, return a result.

The imperative version requires the reader to follow a small sequence of steps before they can name what the code is doing.

Readable code is not a luxury. It is what makes the next code review faster.

Logic that belongs outside the component

Sometimes I see component files that do too much. Business logic, derived state, side effects — all mixed together in the render function.

When I spot values that are calculated from props or state, I do not automatically ask for a hook. But I do ask whether the logic has become important enough to name, test, or reuse.

// ❌ derived value calculated inline, every render
function DataTable({ rows }: Props) {
  const visibleCount = rows.filter(r => !r.hidden).length;
  // ...
}

// ✅ extracted, named, testable and reusable
function useVisibleCount(rows: Row[]) {
  return useMemo(() => rows.filter(r => !r.hidden).length, [rows]);
}

This is not always about performance. And it is definitely not a rule that every derived value needs a custom hook.

The point is separation of concerns.

If a calculation is meaningful enough that we discuss it in review, it may be meaningful enough to extract and name. Logic in a hook can be tested in isolation. Logic buried inside a component is harder to reason about and easier to duplicate.

useEffect that should not exist

This is the pattern I see most often in AI-generated code.

The model knows useEffect. It appears constantly in training data. So when a developer asks for help with derived state or data transformation, the model reaches for useEffect by default — even when there is no side effect involved.

// ❌ useEffect for something that is just a derived value
const [fullName, setFullName] = useState('');

useEffect(() => {
  setFullName(`${firstName} ${lastName}`);
}, [firstName, lastName]);

// ✅ it is just a variable
const fullName = `${firstName} ${lastName}`;

Three extra lines, an extra state variable, an extra render cycle — for something that is literally one line of JavaScript.

If nothing external is being synchronized, I first ask whether an effect is needed at all.

The problem is often not the developer. The problem is the prompt.

If you ask AI to "handle" something without specifying constraints, the model will produce the most common pattern it has seen. And useEffect is everywhere.

The fix is asking differently: "solve this without useEffect" or "what is the simplest way to derive this value" usually produces a much cleaner result.

Comments that describe what instead of why

The last thing I look for is harder to quantify but easy to spot.

// ❌ describes what the code does
// iterate over columns and sum their sizes
const totalSize = cols.reduce((sum, col) => sum + (col.size ?? 0), 0);

// ✅ explains why a decision was made
// col.size can be undefined for dynamic columns — default to 0 to avoid NaN
const totalSize = cols.reduce((sum, col) => sum + (col.size ?? 0), 0);

AI-generated comments almost always describe what. Because that is the easiest thing to say about code.

A comment that says "iterate over columns and sum their sizes" adds zero information. The code already says that.

A useful comment explains why — a constraint, a non-obvious decision, a gotcha that will matter in six months.

When I see a block of AI-generated code with comments that mirror the logic line by line, I ask the developer to replace them. Not because comments are bad. Because that kind of comment is noise, and noise makes review harder.

The pattern underneath all of this

These six things look like separate issues. They are not.

They are all the same issue: code that was produced without being fully understood.

The algorithm works but does more work than necessary.
The helper returns a value but hides its intent.
The hook is correct but unnecessary.
The comment is present but says nothing.

AI can generate all of these. And a developer who is moving fast — pressing Enter, getting output, copying the result — will miss all of them.

This is why I keep pushing the same thing in code review: slow down at the moment the tool wants you to speed up.

Read what was generated.
Ask if it is doing more than necessary.
Ask if the decision is visible in the code.
Ask if the next person will understand why.

At data scale, the cost of not asking is measurable.

At any scale, the cost of not reading is real.