Code review / audit checklist

[coryhouse]

Here's a handy checklist: https://github.com/mgreiler/code-review-checklist

When reviewing a code base, here’s what I search for:

(See my review docs, PRs, package.json’s)

JavaScript

• setTimeout usage

CI

See CI issue
Preview deployments

Source control

What files are the longest?
What files are changed the most? (See Gary Burhardt's https://github.com/garybernhardt/dotfiles)
Which file do you like best?
Which is worst?

Devops

https://dora.dev/research/

Clean up

Clean up first
Signs a team is out of control

Package manager

Consider pnpm or Bun.
Consider Corepack

Code reuse

https://github.com/kucherenko/jscpd - Find copy/pasted code
See #5 for more on reusable components

Dependencies

depcheck or Knip (listed under TypeScript too) - And be sure Knip ignores test folder so code only referenced by tests is found.
Check for updates
Suggest alternative packages

Deploys

Notifying stale SPA users

Formatting

Prettier, running via editor and enforced via CI

Hooks

Using pre-push instead of pre-commit
Hook is fast
Hook should be redundant - CI = real safety
Avoiding useEffect

APIs

Return camelCased JSON
Handle saving stale records and protect from overrwrites

HTTP

Centralized URLs (for reuse in APIs and mocks)

Env

Wrapped, strongly typed, Zod validated env vars

Folders

Mirrors URL structure
Colocates related files
No barrels. my tweet thread

File structure

Mirror URL structure
Consistently cased
Colocating related items
Using Dependency Cruiser to validate and enforce intra-project file dependencies.

TypeScript

Use knip or Ts-prune to find unused code and run via CI to protect (something similar for other types of code like CSS?
Eliminate any - Use type-coverage - to report type coverage
Avoid optional fields
Avoid type assertions via “as” (again, can find via type-coverage)
Use string literals instead of string
Replace string | number with number
Don't use 0 or "" for missing numbers
Force TS Version
Prefer Record over switch
See TypeScript issue for more

URL

Does the URL support deep links for sharing when relevant?
Is data from the URL copied into state?
more here

HTML

disabled
main

General

Cpell

Accessibility

See a11y repo

State

Caching fetch calls

CI

Runs build, test, lint
At least one approval is required
PRs aren’t being rubber stamped
No Lint warnings are allowed
Use Danger to enforce ad-hoc checks list a Changelog entry

ESLint

eslint-disable
Unused imports and vars
No var
Prefer const
Running associated package for each tech

React

index
Audit useEffect
No needless state

Forms

Validation onBlur
Does it reflect latest data if user comes back? (easy mistake to make with react-query as mentioned here)
What if you try saving stale data that someone else already changed?
Dirty notifcation when leaving