Frontend Beauty
ESC

Type to search the knowledge base.

Frontend Code Review Interview Round

How senior loops use a PR: correctness, a11y, a race, a secret in the client, and how you talk to a junior author without being cruel.

advanced6 min read
  • interview
  • code-review
  • senior

A frontend code review interview round sometimes skips a fourth algorithm and drops a PR in front of you instead. The miss is nitpicking import order while a Pay handler uses innerHTML, a div click, and a fetch with no abort. Review rounds score whether you protect users and the next reader, and whether you can say that to a junior author without performing dominance.

You are not the compiler. You are the teammate who catches the bug that would have shipped on Friday.

Key takeaways

  • Sort findings: correctness, then users (a11y, XSS, data loss), then maintainers (types, tests, names).
  • One high-severity comment with a repro beats twelve nits.
  • Ask questions when you might be wrong. Be direct when you are not (secrets, XSS).
  • Offer a patch shape, not a rewrite of their PR in your style.

Correctness first, then users, then maintainers

Read the diff once for story, once for bugs. Talk in this order so the interviewer hears judgment, not a stream of comments.

Priority Question Examples
P0 Correctness Does the feature do the wrong thing? Double pay, stale search results, off-by-one, broken loading
P1 Users Can a person be harmed or blocked? XSS, keyboard trap, unlabeled button, secret in the bundle, lost data
P2 Maintainers Will the next edit fail? any, no test, copy-paste hook, effect without cleanup
P3 Nits Style quotes, import sort, optional chaining taste

If time is 30 minutes, you may never reach P3. Say that: “I am stopping at user-facing issues.”

A PR you might be handed

Treat the following as a stand-in, not a company question. Read it like a diff.

function PayButton({ amount, user }) {
  const [html, setHtml] = useState('');

  async function pay() {
    const res = await fetch('/api/pay?amount=' + amount);
    const body = await res.text();
    setHtml(body);
  }

  return (
    <div>
      <div onClick={pay}>Pay {amount}</div>
      <div dangerouslySetInnerHTML={{ __html: html }} />
      <p>{user.apiKey}</p>
    </div>
  );
}

Walk it in the priority order.

Correctness

  • GET for a charge. Caches, prefetch, and retries can replay it. Needs a mutating method and an idempotency key.
  • No disable / in-flight guard. Double click is two requests.
  • No res.ok check. A 402 HTML error page becomes “success UI.”
  • amount concatenated into a query string: encoding, logging, and cache.

Users

  • div + onClick: not a button, no keyboard, no name. See keyboard accessibility.
  • dangerouslySetInnerHTML on a network body: XSS. Even “trusted” payment HTML is a sink.
  • user.apiKey in the document: secrets in frontend bundles. If it rendered, it already leaked.

Maintainers

A comment you could actually paste:

P0: `/api/pay` is fetched with GET and the raw body is injected as HTML.

Repro: click Pay twice, or serve `<img src=x onerror=alert(1)>` as the response.

Please: POST with an idempotency key, `res.ok` + JSON, text content (not HTML),
and a real <button disabled={pending}>. apiKey must never render.

That is specific, ranked, and has a next action. It is not “this is not clean.”

Races and abort

Search boxes and pay sheets share a bug: response N+1 arrives before N. Review for:

  • missing AbortController on the previous request
  • ignoring AbortError vs treating it as a user-facing failure
  • await in useEffect without a cancelled flag or signal
  • setting state after unmount (warning plus a leak of intent)
useEffect(() => {
  const controller = new AbortController();
  fetch(`/api/search?q=${encodeURIComponent(query)}`, { signal: controller.signal })
    .then((r) => r.json())
    .then(setItems)
    .catch((err) => {
      if (err.name === 'AbortError') return;
      setError(err);
    });
  return () => controller.abort();
}, [query]);

If the PR used a module-level let lastId instead, say why a signal is clearer, but do not bike-shed a working counter unless it is wrong.

a11y in reviews

You do not need a full WCAG audit in 20 minutes. You do need:

  • Controls are <button> / <a> / <input>, not clickable divs
  • Visible label, or aria-label that matches purpose
  • Errors as text, tied to the field (accessible forms)
  • Focus on the new dialog, restore on close (accessible modals)
  • Color not the only error signal

Talking points: accessibility interview.

Secrets and XSS

Frontend review is where XSS and leaked tokens still slip. Sinks: innerHTML, dangerouslySetInnerHTML, javascript: URLs, eval, unsanitized href. Tokens: anything in JS bundles, localStorage “because HttpOnly was hard,” debug console.log(user). Map: security interview talking points, XSS prevention checklist.

If you see a secret, that comment is P1 even if the feature “works.”

How you talk to a junior author

Interviewers listen to tone. Cruelty is a no-hire on some loops. Performative softness that hides a P0 is also a miss.

Instead of Say
“This is wrong, rewrite it.” “GET on pay will replay. Can we POST + idempotency key?”
“Why didn’t you use Redux?” “State looks local and that is fine. The bug is the race.”
20 nits on naming One nit max, after P0/P1
“a11y: add aria” “This clickable div is not a keyboard button. <button> gives that free.”
Silence on XSS Direct: “This HTML sink is exploitable. We cannot merge it.”

Procedure:

  1. Restate what the PR is trying to do (they may correct you).
  2. P0 bugs with repro.
  3. P1 users.
  4. One improvement (test, type, abort).
  5. What you would not block on.

Praise one thing that is actually good (“nice isolation of the hook”) so the author can hear the rest. Fake praise is obvious.

Footguns (as the reviewer)

  • Rewriting the PR in the comment box
  • Ignoring tests that already fail
  • Flagging let vs const before XSS
  • Assuming the author cannot see browser APIs you like
  • Saying “ship it” because you are afraid to be direct

Interview out-loud answer

“I would read for behavior first. On this Pay button I would block on GET-plus-HTML injection, double submit, and the rendered apiKey. Then I would ask for a real button, disabled pending state, JSON errors as text, and an abort on unmount. I would leave style nits off the first pass. I would write one P0 comment with a repro so a junior author knows what to change, not that I am clever.”

Further reading