Code Review React Native

Code Review (React Native)

Strict, opinionated code review for React Native plugin codebases with Kotlin + Swift bridges and an optional Expo config plugin. Optimised for catching the kinds of issues that survive linters and tests but degrade a codebase over time.

Review priorities, in order

1. Correctness & contract — bugs, breaking changes, public-API violations, SemVer; identifiers and API surface aligned across TypeScript, Kotlin, and Swift.
2. Native-layer cleanliness — no hardcoded fallbacks for values owned by TypeScript or the platform SDK; no behaviour duplicated across language boundaries; platform-specific features explicitly documented.
3. Single responsibility — each function, class, and file does one thing.
4. Deduplication (DRY) — repeated parsing, serialisation, and validation patterns get extracted.
5. Readability — naming, argument styles, generated-vs-handwritten boundaries, no surprises.
6. Maintainability — generated files untouched, defaults documented in one place, tests for new public surface.
7. React Native / TypeScript best practices — const, @deprecated discipline, null vs undefined, strict tsconfig honoured.
8. Polish — JSDoc style, example brevity, changelog accuracy, PR description present.

Workflow

1. Gather context

If the user references a GitHub PR by number:

gh pr view <N> --json title,body,author,state,baseRefName,headRefName,additions,deletions,changedFiles,files,url
gh pr diff <N>
git log <base>..<head> --oneline

If the changes are local:

git diff <base>..HEAD --stat
git diff <base>..HEAD

Always read the full file (not only the diff) for any non-trivial change so you see the surrounding context the diff hides — especially around native bridges, event emitter wiring, and EventIdentifiers.

2. Classify every changed file

Each file falls into exactly one of these buckets, and the bucket determines what's acceptable:

3. Run the checklist below and produce the report

Hard rules — block the merge

1. No hand-edits to generated / built files

lib/ (built by react-native-builder-bob) and plugin/build/ (compiled from plugin/src/) must be touched only by their build tool.

Red flags:

• Behaviour-changing edits inside lib/ that differ from what yarn build would produce.
• plugin/build/ edited directly instead of editing plugin/src/ and re-running the build.
• A // eslint-disable or // @ts-ignore directive added to a file inside lib/.

Action: ask the author to run yarn build and commit the clean output.

2. SemVer must match the change

A breaking public-API change requires a major version bump. "Public" includes anything exported from src/index.tsx and anything that changes the wire format with the native side.

Common breaks to flag:

• Renamed or retyped fields on exported TypeScript types or interfaces.
• Renamed string values consumers may pattern-match against.
• Removed or repurposed enum variants that map to native callback codes.
• Changed required/optional status on config parameters.

If CHANGELOG.md claims SemVer adherence and the bump doesn't match the change, call it out with the affected symbols and the recommended version.

3. No hardcoded fallbacks in the native layer

The native layer (Kotlin / Swift) is a transport adapter between TypeScript and the platform SDK. It must not:

• Invent default values for fields the TypeScript layer marks required. Such defaults are unreachable but advertise an optional contract that doesn't exist.
• Substitute defaults for nullable/optional fields. Either the SDK has its own default (skip the call) or the default belongs in TypeScript.
• Encode the same default in multiple places. A default as a literal in optString("foo", "BAR"), as an enum in getOrDefault(Foo.BAR), and again as a fallback object is three sources of truth.
• Hardcode enum names as raw strings (e.g. "SIDELOADED_ONLY"). Use the SDK enum's .name property or avoid manual parsing entirely.

Recommended remediation:

• For required TypeScript fields: drop the native default; let parsing throw and surface through the existing error path.
• For optional TypeScript fields: skip the builder/setter call when the field is absent; let the SDK apply its own default.
• Document the default once — in the TypeScript JSDoc — so the public contract is unambiguous.

4. NativeModules name must be consistent across all layers

The string used to register and look up the native module must agree across three places:

Any drift in this string causes the module to be undefined at runtime with no compile-time error.

5. NativeEventEmitter channel data aligned across TypeScript, Kotlin, and Swift

Threats and execution state are delivered through obfuscated event channels. The channel metadata is fetched at runtime from native via two methods:

Verify for every change touching these methods:

• Array length is the same on Kotlin and Swift sides.
• The semantic order of items in the array is the same on both native sides and matches what src/channels/threat.ts and src/channels/raspExecutionState.ts destructure by index.
• The TypeScript type declarations in src/types/types.ts (TalsecPlugin interface) reflect any change to the array shape.

6. EventIdentifiers must stay obfuscated — never replace with hardcoded strings

ios/utils/EventIdentifiers.swift derives all channel identifiers at runtime from RandomGenerator.generateRandomIdentifiers(length: N). This obfuscation is intentional and security-relevant.

Hard blocks:

• Replacing a generatedNumbers[i] reference with a hardcoded string or integer constant.
• Adding a new identifier slot without expanding length and updating all index references consistently.
• Using the same index in two different identifier roles (index collision).

The Kotlin side generates its own independent random identifiers via an equivalent mechanism; they are not shared with iOS. This is by design — each process re-generates at launch. Do not attempt to synchronise iOS and Android identifier values.

7. NativeEventEmitter listener registration and cleanup symmetry

Every addListener(channelName, callback) call on NativeEventEmitter(FreeraspReactNative) must have a matching cleanup path.

Check:

• removeListenerForEvent(channelName) is called in Kotlin's removeListenerForEvent react method, and the matching Swift handler does the same.
• The returned EmitterSubscription (or the subscription stored in useFreeRasp) is .remove()d on cleanup.
• Kotlin's addListener increments the listener count so the native module doesn't drop events on multi-listener setups.