The TypeScript Code Review Checklist We Run on Every SvelteKit and React Native Pull Request

What belongs in a TypeScript code review checklist (and what does not)

A useful code review checklist contains only items a machine cannot decide: correctness of the type boundary, whether data is validated where it crosses a system edge, whether a query or security rule exposes data, and whether the change fits the existing architecture. Formatting, import order, naming conventions, and unused variables belong in tooling.

The test we apply to any proposed checklist item is simple. If a reviewer would give the same answer every time regardless of context, it is a lint rule, not a review item. If the answer depends on what the code is doing, who calls it, and what happens when it fails, a human reviews it. That single filter cut our review comments down to the ones that change behaviour.

The checklist below is organised by where things actually break in our stack: the type boundary, the data contract, Firestore, SvelteKit's server/client split, and React Native rendering. Each item exists because a specific class of bug reached staging or production in a codebase we maintain, not because it appeared in a listicle.

• • Human-reviewed: type boundaries, data contracts, security rules, query shape, error handling, architectural fit, and anything touching money, PII, or auth.
• • Automated: formatting, import order, unused code, naming, `any` usage, missing return types, non-null assertions, console statements, dependency array correctness.
• • Not reviewed at all: generated files, lockfiles, translation string additions, and anything the CI pipeline already gates.

The type boundary checklist: `any`, `unknown`, and where types stop being true

TypeScript types are only guarantees inside your own code. At every boundary where data arrives from outside (HTTP responses, Firestore documents, `localStorage`, push notification payloads, third-party SDK callbacks) the declared type is an assertion, not a check. The review question is always: what proves this shape is real at runtime?

The most expensive version of this we have seen is a Firestore document typed as an interface, read directly into application code, and then trusted. A document written by an older version of an admin panel, or by a migration script that skipped a field, produces a runtime `undefined` in a place TypeScript swore was a `string`. The fix is never a better interface. It is validating or defaulting at the read boundary, and typing the raw read as `unknown` or a partial type so the compiler forces you to handle it.

The same rule applies to `catch` blocks. A caught error is `unknown` and narrowing it with a cast to `Error` is a lie the first time a library throws a string or a Firebase error object with a `code` field. On review we ask for real narrowing, and for the error path to actually produce something the user or the log can use.

• • Every external response is parsed or narrowed before it reaches domain code, not cast.
• • No `as` casts across a boundary. Casts inside a narrowing function with a runtime check are fine.
• • Optional fields from the database are handled, not assumed. If a field is required, the write path enforces it and the read path defaults it.
• • Discriminated unions instead of optional-field soup for anything with modes or states.
• • Errors are narrowed from `unknown`, and the failure branch does something other than swallow.

Data contracts, Firestore queries, and security rules

At system edges we require a runtime schema, in practice Zod, defining the contract once and deriving the TypeScript type from it. The review item is not "is there a schema" but "is the schema the single source of truth, and is it applied on the write path as well as the read path?" Two schemas that drift are worse than none.

For Firestore, three query-level checks catch most of what goes wrong. First, does the query have a bound? An unbounded collection read is fine with 200 documents and a cost incident with 200,000. Second, does it need a composite index, and is that index in the committed index file rather than created by hand in the console? Console-created indexes vanish when a project is recreated and produce a failure that only appears in production. Third, is the read on a document path a client can legitimately access, or is the code relying on the UI never asking?

Security rules get reviewed as code, with tests. Any PR that changes a rule needs a matching test in the emulator suite proving both the allowed case and the denied case. The denied case is the one people forget, and it is the only one that matters. We also check that rules validate the shape of writes for anything a client can write directly, because a rule that checks `request.auth.uid` but not the fields being written lets an authenticated user set their own `role` or `credit` field.

• • One schema per contract, type derived from it, validation applied at both ends.
• • Every Firestore query has a limit, a known index, and a matching security rule.
• • Rule changes ship with emulator tests covering allow and deny.
• • Writes clients can perform have field-level validation in rules, not just auth checks.
• • Anything that changes a document shape has a plan for documents already written in the old shape.

SvelteKit: the server boundary and what leaks into the client bundle

In SvelteKit the highest-severity review item is the server/client boundary. Anything returned from a universal `load` function ships to the browser, and anything imported into a component graph gets bundled. The check is mechanical: trace every value returned from `load` and ask whether you would be comfortable seeing it in view-source, because that is exactly where it ends up.

The recurring mistakes are consistent enough to be a checklist. A service account key or admin SDK import reaching a file that is not `+page.server.ts` or `+server.ts`. A `load` returning a whole user document including internal flags, Stripe customer IDs, or email addresses of other users, when the page renders a name and an avatar. Environment variables imported from `$env/static/public` when they should be private, or the reverse, a private variable read in a component and silently failing at build.

The second SvelteKit cluster is data flow. Fetching in `onMount` instead of `load` costs the page its server render and its SEO. Doing a `fetch` in a universal load without using the provided `fetch` breaks credential forwarding and duplicates the request on hydration. Forms that bypass actions and post to an ad hoc endpoint lose progressive enhancement and usually lose validation with it. None of these are style opinions. Each one changes what the user gets.

• • Only server files import admin credentials, secrets, or `$env/static/private`.
• • Everything returned from a universal `load` is intentionally public.
• • The `load` function's `fetch` is used, not the global one.
• • Data the page needs for its first paint comes from `load`, not `onMount`.
• • Mutations go through form actions or `+server.ts` with validation on the server side, never validation only in the component.

React Native: re-renders, bridge traffic, and platform divergence

React Native reviews focus on three things: what causes a re-render, what crosses the native bridge on every frame, and what behaves differently on iOS versus Android. These are the items where a change that looks correct in a simulator on a fast machine becomes a visibly janky list on a mid-range Android device.

The re-render checks are concrete. Inline object and arrow-function props passed into list items defeat memoisation and re-render the whole row on every parent update. Context providers holding a single large object re-render every consumer when any field changes, so state that updates frequently belongs in its own provider or a store with selectors. A `useEffect` whose dependency array includes an object literal runs forever. The linter catches missing dependencies, but only a human catches a dependency that is recreated every render.

For bridge and list performance we check that animations use the native driver or a worklet rather than JS-driven state updates, that long lists have stable `keyExtractor` values and sensible windowing, and that images have explicit dimensions. On the platform side, review asks whether safe area, keyboard avoidance, permissions, and back-button behaviour were considered, since these are the four areas where iOS-only testing reliably ships an Android bug.

• • No new inline objects, arrays, or functions passed into memoised list rows.
• • Frequently changing state is not stored in a context consumed by half the tree.
• • Effects depend on primitives or stable references, and cleanup exists for subscriptions and timers.
• • Animations run on the native side, not via JS state per frame.
• • Any new screen is reasoned about on both platforms: safe area, keyboard, permission prompt, hardware back.

The comments we stopped leaving, and how review runs on a small team

The most valuable change we made to review was deleting comment categories rather than adding checklist items. Anything a tool can decide, a tool decides. That means TypeScript strict mode with `noUncheckedIndexedAccess`, ESLint rules that make `any`, floating promises, and unhandled promise rejections build failures, Prettier on commit, and CI running typecheck, lint, unit tests, and the Firestore rules emulator suite before a human opens the diff.

Concretely, the comments we no longer write: "add a return type here", "this import should be type-only", "unused variable", "you left a console.log", "missing await", "reorder these imports", "this file isn't formatted", "this dependency array is incomplete", "non-null assertion". Every one of those became a rule. A reviewer who spends their attention on import order has none left for the security rule three files down.

On a three to five engineer team, the process around review matters as much as the checklist. Every PR gets one reviewer, chosen because they know the subsystem, not on rotation. Reviews happen within one working day, and if the author is blocked they say so and it happens the same hour. PRs are kept small enough to read in one sitting, which is the single biggest predictor of whether review finds anything. We skip review only for content-only changes, dependency bumps that CI has already validated, and hotfixes, where the review happens immediately after deploy rather than before. When the client owns the repository and has their own engineers, we follow their conventions and add our checks as CI configuration rather than as opinions in comments, so the standard survives after we hand over.

• • Automated before a human looks: typecheck in strict mode, lint, format, unit tests, Firestore rules tests, build.
• • One named reviewer per PR, chosen for subsystem knowledge.
• • One working day SLA, same hour if the author declares themselves blocked.
• • Small PRs. If it cannot be read in one sitting, it gets split.
• • Review skipped for content changes, validated dependency bumps, and hotfixes reviewed post-deploy.
• • On client-owned repos, standards land as CI config, not as reviewer preference.

Most of the patterns above are readable in public: see our open source portfolio, which indexes JMS and five production SvelteKit and Firebase codebases including Agrimatco, Genos, and Bioinspekt.