Trace execution path before reporting correctness findings
Read the callers and trace the execution path before reporting a correctness finding. Prefer zero findings over a low-value one.
Expo & React Native · all subjects
correctness-js
44 notes, read out of this brain and free to use. Each one was extracted from a source and is re-checked against its exam.
Do not flag demo or test-app code to library standards
Demo and test-app code in `apps/` is not held to library standards for correctness review.
Empty or absent state reachable with unchecked array access
Flag when a diff makes a collection possibly empty or an index possibly out of range, and the code then uses unchecked array access like `arr[i]!` or `arr[i].x`. This is the single largest cluster of severe findings in code review.
findIndex with Math.max(0, idx) silently returns wrong element
When `findIndex(...)` is clamped with `Math.max(0, idx)`, a -1 result silently selects the first item instead of signalling "not found". This converts a bug into a wrong answer rather than an error.
Early return or filter making empty collection reachable
When a new early `return`, filter, or guard is added that lets a previously non-empty collection reach a consumer that still assumes at least one element, the empty state becomes reachable.
Unreachable added code due to missing imports or exports
Flag an added file, export, branch or guard that nothing in the tree imports. Also flag a name omitted from the enclosing subpath's explicit re-export list, preventing it from reaching consumers.
Unreachable context provider always empty at insertion
A context provider inserted in a diff that is always empty at the position it was inserted is unreachable code.
Error behavior flip from console.warn to render-phase throw
When a `console.warn` plus continue is replaced by a render-phase `throw`, and the old path is a plausible pattern in shipped apps, error behavior flips from a warning into a crash.
Required interface method made optional without signal
When a previously required interface method is made optional, and opting out is coerced to success with no signal, a loud failure silently becomes a wrong result.
Added await in loop serializes concurrent work
An added `await` inside a loop that serializes work which was concurrent is an async logic mistake.
Removed await lets rejection escape as unhandled
A removed `await` allows a rejection to escape as an unhandled rejection.
Subscription or listener with no teardown
A subscription, listener, timer or abort controller created in a diff with no matching teardown on the same path causes a resource leak.
State written after early return or teardown
State written after an early return or after teardown in a component or module, where the component or module may already be gone, is unreachable or unsafe.
Promise left forever pending with no resolution path
A promise returned to a caller that some path leaves forever pending — a branch that neither resolves nor rejects, or an event-driven resolution whose event can never fire.
Test assertion on undefined value
Flag a test where an asserted value is `undefined` or `$undefined`, as the test proves nothing.
Shallow toMatchObject never reads claimed key
A shallow `toMatchObject` test that never reads the key the test name claims proves nothing.
Test assertion satisfied by fallback path
A test whose assertion the fallback path would also satisfy cannot distinguish the behavior it claims to pin from the behavior it replaced. The test proves nothing unless it constructs the case where the two differ.
mockRestore in test body instead of teardown
A `mockRestore()` at the end of a test body rather than in teardown causes one failure to leak the mock into every later test.
Bug fix with no test in package with __tests__ directory
A bug fix with no test in a package that has a `__tests__/` directory violates the red/green workflow of this repository. The failing test is expected to come first.
Dependency added without pnpm-lock.yaml update
Added or bumped dependencies in any `package.json` with no `pnpm-lock.yaml` in the same diff fails CI before any test runs.
Partial bump of package family pins old version
A partial bump of a package family, where the un-bumped siblings transitively pin the old version, causes version desync.
Do not flag toolchain-enforced issues
Do not flag formatting, import order, unused code, or `tsc` strictness rules that the toolchain already enforces. With `noUncheckedIndexedAccess` on, only flag where the author defeated the check with `!` or a cast.
Do not flag cross-cutting or style issues
Do not flag cross-cutting correctness issues (fix applied to one copy, TypeScript disagreeing with native code), style preferences (naming, file organization, helper extraction), missing tests for pure refactors, single-instance patterns, pre-existing debt, hypothetical inputs no caller produces, or performance speculation without measurement.
Cross-cutting correctness review scope and responsibility
The cross-cutting correctness reviewer owns defects visible only when reading code across files in different languages, not logic contained within a single file. The per-language reviewers (correctness-ios, correctness-android, correctness-js) own logic within their own files. Cross-cutting correctness runs on every PR, including those that would not otherwise trigger it.
iOS and Android drift: behavior and default divergence
When a behavior, default, event payload shape, or error code changes on one platform while the other platform's implementation still uses the old behavior, and the TypeScript API exposes both as one function, this is a cross-cutting defect. Before deciding, read the sibling file in the other platform's language.
iOS and Android drift: capability or field added to one platform only
When a new capability, option, or Record field is added to one platform only but the TypeScript type offers it unconditionally, callers experience a silent no-op on the other platform rather than an error. This is a cross-cutting defect.
Error identity divergence across platforms
When the same bad input produces different JavaScript error codes on each platform — typically a raw IllegalArgumentException or NSError on one side (wrapped as ERR_UNEXPECTED by expo-modules-core) against a named CodedException or Exception subclass on the other — JavaScript code that branches on error.code will work on exactly one platform. This is a cross-cutting defect.
Fix landing on one platform with missing regression test on sibling
When a fix lands on one platform with a regression test, but the sibling platform receives the same fix with no test, or the same bug with no fix at all, this is a cross-cutting defect.
Deliberate duplicate code locations in expo-router
The repository carries deliberate duplicates that require cross-platform review: packages/expo-router/src/fork/ versus packages/expo-router/src/react-navigation/ (vendored upstream code that this repo patches), iOS and Android implementations of the same module API, other navigators, codemods or templates that emit the same code, and translated documentation mirrors of edited English pages. Fixes must be applied to all copies.
TypeScript contract versus native implementation mismatch: string-union members
When a TypeScript string-union member has no matching Swift enum raw value or Kotlin constant, or the reverse, this is a cross-cutting defect. Compare actual values, not type names.
TypeScript contract versus native implementation mismatch: Record field keys
When a Record field has a Kotlin @Field(key = …) or Swift @Field name that no longer matches the key the TypeScript side sends, this is a cross-cutting defect.
TypeScript contract versus native implementation mismatch: event names
When an event name in sendEvent(…) or emit(event:) is absent from TypeScript listener types, or when TypeScript has a listener for an event no platform emits, this is a cross-cutting defect.
Documentation contradicting implementation behavior
When TSDoc or documentation states behavior that the implementation contradicts — such as a documented default, a documented fallback that never runs, or a documented platform availability that the native code does not honor — this is a cross-cutting defect. Verify the claim against the code before reporting.
Nullable versus non-nullable field contract mismatch
When a TypeScript field is typed as nullable but one platform can never return it as null, or when a field is non-nullable but one platform can leave it absent, this is a cross-cutting defect.
Changed default altering callers without source changes
When a default value, default queue, default storage mode, or default code path changes in shared infrastructure, existing call sites keep their source unchanged but change behavior. This is a cross-cutting defect. Trace at least two existing callers before reporting and name them.
Function moving between synchronous and asynchronous or queues
When a function moves between synchronous and asynchronous, or between queues, and a caller holds a lock, semaphore, or transaction across the call, this is a cross-cutting defect.
Missing AndroidManifest.xml permission for runtime permission request
When new runtime permission requests or implicit-Intent resolution (resolveActivity, queryIntentActivities, querying another package) are added but the package's android/src/main/AndroidManifest.xml lacks matching <uses-permission> or <queries> declarations, this is a cross-cutting defect.
Missing Info.plist usage description for iOS privacy-sensitive API
When a new iOS capability or privacy-sensitive API is added with no matching Info.plist usage description in the package's config plugin, while the Android side declares its permission, this is a cross-cutting defect.
What NOT to flag: single-file logic
Logic contained inside a single file in a single language belongs to correctness-ios, correctness-android, or correctness-js reviewers and should not be flagged as a cross-cutting defect.
What NOT to flag: intentional and documented platform differences
Platform differences that are intentional and documented with @platform ios, Platform.OS branches, or TSDoc notes stating the API is iOS-only should not be flagged as cross-cutting defects. Verify the annotation exists before assuming divergence is a bug.
What NOT to flag: web implementations diverging from native
Web implementations diverging from native where the TypeScript type already narrows by platform should not be flagged as a cross-cutting defect.
What NOT to flag: pre-existing divergence
Pre-existing divergence that the diff merely moved or reformatted should not be flagged as a cross-cutting defect.
What NOT to flag: demo and test-app code divergence
Divergence in apps/ demo and test-app code should not be flagged as a cross-cutting defect.
Cross-platform finding must name both sides
A cross-platform finding that names only one file is not yet a complete finding. The finding must name the sibling file and quote what it does instead.