Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds comprehensive React Native best practices and upgrade documentation across 50+ new skill reference files, combined with targeted application improvements. The documentation spans performance optimization (bundle, JS, native), architectural patterns, and a complete upgrade workflow. Application changes include logging refinements, error handling improvements, store optimizations, and build configuration adjustments. ChangesReact Native Best Practices Documentation
React Native Upgrade Workflow Documentation
Application Code Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/api/common/client.tsx (1)
109-115:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Token refresh failures causing logout must be reported to Sentry.
This path handles non-recoverable token refresh failures (invalid refresh token, 400/401 responses) that force user logout. Downgrading to
logger.warn()removes automatic Sentry reporting, which eliminates visibility into:
- Refresh token validity issues
- Backend authentication service problems
- Forced logout frequency and patterns
These are critical auth failures that directly disrupt user sessions and must be tracked centrally for monitoring and debugging.
As per coding guidelines, "
logger.error()automatically reports to Sentry" for errors requiring tracking.🔄 Revert to error-level logging
if (!isNetworkError) { // Only logout for non-network errors (e.g., invalid refresh token, 400/401 from token endpoint) - logger.warn({ + logger.error({ message: 'Token refresh failed with non-recoverable error, logging out user', context: { error: refreshError }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/common/client.tsx` around lines 109 - 115, The non-recoverable token refresh branch currently calls logger.warn which prevents automatic Sentry reporting; change the call to logger.error and include the refreshError and contextual info so Sentry receives the event (update the block guarded by isNetworkError, replacing logger.warn with logger.error and preserving the message and context), and keep the subsequent useAuthStore.getState().logout() call as-is so the logout is still performed but now recorded as an error for monitoring.src/stores/signalr/signalr-store.ts (4)
182-189:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: SignalR disconnection failures must be reported to Sentry.
Similar to connection failures, SignalR disconnection failures indicate issues with the real-time messaging infrastructure. These should be tracked in Sentry to monitor service health and connection lifecycle issues.
As per coding guidelines, "
logger.error()automatically reports to Sentry" for errors requiring tracking.🔄 Revert to error-level logging
} catch (error) { const err = error instanceof Error ? error : new Error('Unknown error occurred'); - logger.warn({ + logger.error({ message: 'Failed to disconnect from SignalR hubs', context: { error: err }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/signalr/signalr-store.ts` around lines 182 - 189, The catch block handling SignalR disconnection failures in the disconnect handler currently logs with logger.warn and sets state; change this to use logger.error so the event is reported to Sentry (replace the logger.warn call with logger.error while preserving the message and context object), keep the error normalization (error instanceof Error ? error : new Error(...)) and the set({ error: err }) behavior intact; look for the catch within the disconnect logic in signalr-store (the block that constructs err and calls logger.warn) to make this change.
169-176:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: SignalR connection failures must be reported to Sentry.
SignalR connections are critical for real-time call updates and notifications. Downgrading connection failures to
logger.warn()removes automatic Sentry reporting, eliminating visibility into:
- Backend SignalR service availability issues
- Connection reliability and failure patterns
- Network vs. service problems
Users will miss real-time call updates and notifications when these connections fail, directly impacting the app's core functionality.
As per coding guidelines, "
logger.error()automatically reports to Sentry" for errors requiring tracking.🔄 Revert to error-level logging
} catch (error) { const err = error instanceof Error ? error : new Error('Unknown error occurred'); - logger.warn({ + logger.error({ message: 'Failed to connect to SignalR hubs', context: { error: err }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/signalr/signalr-store.ts` around lines 169 - 176, The catch block in the SignalR connection code currently calls logger.warn which prevents Sentry reporting; replace the logger.warn call inside the catch (the one creating the object with message 'Failed to connect to SignalR hubs' and context: { error: err }) with logger.error so the error is reported to Sentry, preserving the same message and context object (and continue to call set({ error: err })). Ensure you use the same error variable (err) and do not strip the Error object when calling logger.error.
252-259:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Geolocation hub disconnection failures must be reported to Sentry.
Geolocation hub disconnection failures should be tracked to monitor real-time location service health and connection lifecycle issues.
As per coding guidelines, "
logger.error()automatically reports to Sentry" for errors requiring tracking.🔄 Revert to error-level logging
} catch (error) { const err = error instanceof Error ? error : new Error('Unknown error occurred'); - logger.warn({ + logger.error({ message: 'Failed to disconnect from SignalR hubs', context: { error: err }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/signalr/signalr-store.ts` around lines 252 - 259, The catch block handling SignalR hub disconnection currently logs with logger.warn, which does not trigger Sentry reporting; change it to logger.error so Sentry receives these failures. Update the catch in the disconnect/cleanup code (the block that creates err from error and calls logger.warn) to call logger.error with the same message and context { error: err } and keep set({ error: err }) unchanged so the error is both reported and stored in state.
239-246:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Geolocation hub connection failures must be reported to Sentry.
Geolocation hub connection failures prevent real-time location updates, a critical feature for emergency response coordination. These failures must be tracked in Sentry.
As per coding guidelines, "
logger.error()automatically reports to Sentry" for errors requiring tracking.🔄 Revert to error-level logging
} catch (error) { const err = error instanceof Error ? error : new Error('Unknown error occurred'); - logger.warn({ + logger.error({ message: 'Failed to connect to SignalR hubs', context: { error: err }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/signalr/signalr-store.ts` around lines 239 - 246, The catch block that currently creates `err` and calls `logger.warn({ message: 'Failed to connect to SignalR hubs', context: { error: err } })` must use error-level logging so Sentry picks it up: replace `logger.warn` with `logger.error` (keeping the same message/context) in the SignalR connection catch (the block that constructs `err` and calls `set({ error: err })`), and optionally include "geolocation" in the message to make it clear this is a geolocation hub connection failure.src/services/location.ts (1)
50-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocation API failures should be reported to Sentry for data loss tracking.
Downgrading to
logger.warn()removes automatic Sentry reporting for location send failures. These failures represent data loss where location updates aren't persisted to the backend. Without central tracking, persistent backend API issues or data integrity problems may go undetected.Location tracking is a core feature, and failures here should be monitored to ensure service health and data completeness.
As per coding guidelines, "
logger.error()automatically reports to Sentry" and should be used for errors requiring tracking.🔄 Revert to error-level logging
} catch (error) { - logger.warn({ + logger.error({ message: 'Failed to send location to API', context: {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/location.ts` around lines 50 - 59, The catch block handling location send failures currently calls logger.warn which avoids automatic Sentry reporting; update the catch in the function that sends location (the block referencing location.coords.latitude/longitude) to call logger.error instead, preserving the same structured payload (message, context with error details, latitude, longitude) so Sentry receives the error and details for centralized tracking.src/services/push-notification.ts (1)
517-523:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReconsider downgrading the catch block error to
logger.warn()— it conflates distinct error types.The catch block (lines 517-523) can fail for two different reasons:
- Permission failures (lines 456, 481): Already logged as
warn— appropriate for user-controlled denials- System/backend failures: Token generation (
messaging().getToken()) or backend registration (registerUnitDevice()) failures — these indicate Firebase misconfiguration, network issues, or server errorsBy using
logger.warn()in the catch block, you're treating unrelated system errors the same as permission denials. These failures disable push notifications and should be tracked. Additionally, the calling code (line 564) expects this as an error and useslogger.error()in its.catch()handler — creating inconsistency.The graceful degradation (returning
null) is separate from logging severity. The app should continue working, but backend registration failures and token generation failures should still report to Sentry for monitoring.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/push-notification.ts` around lines 517 - 523, The catch block at the end of registerForPushNotifications incorrectly downgrades all failures to logger.warn; change it to treat system/backend failures as errors by calling logger.error(...) and send the exception to Sentry (e.g., Sentry.captureException(error)), while still returning null for graceful degradation. Keep permission-denial warnings as-is where they are logged earlier (permission checks at the lines calling requestPermission()/hasPermission()), but in the catch covering messaging().getToken() and registerUnitDevice() ensure the log message includes the error object and context so backend/token failures are tracked consistently with the calling code's expectations.
🧹 Nitpick comments (2)
.opencode/skills/react-native-best-practices/references/native-view-flattening.md (1)
25-29: ⚡ Quick winClarify that
collapsable={false}on custom components requires explicit prop forwarding to take effect.The examples at lines 25–29, 75–79, and 130–139 show
collapsable={false}on custom components (e.g.,<Tab1 />,<TabContent />), but don't clarify that these props only work if the custom component forwards them to an underlying nativeView. The pattern is shown later (lines 145–159) but should be integrated or emphasized in the main examples to prevent developers from relying oncollapsable={false}without proper forwarding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.opencode/skills/react-native-best-practices/references/native-view-flattening.md around lines 25 - 29, The examples using collapsable={false} on custom components (NativeTabBar with Tab1, Tab2 and TabContent) must note that collapsable only takes effect if the custom component forwards the prop to an underlying native View; update the examples and copy or reference the forwarding pattern shown later (the component that spreads/forwards props into the native View) so readers know to forward collapsable (or use {...props}) in Tab1/TabContent implementations; explicitly mention the requirement to forward collapsable to the native View in the main examples and either integrate the forwarding pattern example next to them or call it out prominently.src/app/_layout.tsx (1)
59-74: 💤 Low valueConsider extracting the Mapbox error filter to a separate function.
The
beforeSendfiltering logic is inline and tests a specific known issue. Extracting it to a named function would improve testability and readability, especially if additional filters are added in the future.♻️ Refactor suggestion
// Add near the top of the file after imports function isKnownMapboxGlobeProjectionError(event: Event): boolean { const values = event.exception?.values; if (!values?.length) return false; const top = values[values.length - 1]; if (top.type !== 'TypeError' || !top.value?.includes("Cannot read properties of null (reading '3')")) { return false; } const frames = top.stacktrace?.frames; return frames?.some((f) => f.function?.includes('_cameraForBounds') || f.function?.includes('GeolocateControl') || f.function?.includes('fromInvProjectionMatrix') ) ?? false; } // Then in Sentry.init: Sentry.init({ // ... existing config ... beforeSend(event: Event, hint: EventHint) { if (isKnownMapboxGlobeProjectionError(event)) { return null; } return event; }, // ... rest of config ... });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/_layout.tsx` around lines 59 - 74, Extract the inline Mapbox filter in the beforeSend handler into a named helper function (e.g., isKnownMapboxGlobeProjectionError) and call it from beforeSend; specifically, move the logic that inspects event.exception?.values, picks the top frame, checks top.type === 'TypeError' and top.value.includes("Cannot read properties of null (reading '3')"), and then looks for frames whose function includes '_cameraForBounds', 'GeolocateControl' or 'fromInvProjectionMatrix' into that helper so beforeSend simply returns null when isKnownMapboxGlobeProjectionError(event) is true and otherwise returns the original event; ensure the new function is exported or located near the top of the file so it’s testable and referenced by Sentry.init's beforeSend.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.opencode/skills/react-native-best-practices/references/bundle-analyze-app.md:
- Around line 127-131: The fenced code block that begins with the lines
"Variant: SampleApp-<UUID>.ipa" is missing a language token which triggers
markdown linting errors; update the opening fence by adding a language
identifier (for example "text") so the block reads ```text and leave the block
contents unchanged; target the fenced block containing "Variant:
SampleApp-<UUID>.ipa" in the markdown to apply this change.
In
@.opencode/skills/react-native-best-practices/references/bundle-analyze-js.md:
- Around line 138-148: The example uses an unsupported --json flag on the
react-native bundle command; remove the `--json stats.json` token from the `npx
react-native bundle` invocation and instead use the `--bundle-output` flag to
write the bundle file (keep `--platform android`, `--entry-file index.js`,
`--dev false`, `--minify true`), then produce or save a separate stats JSON for
`npx bundle-stats` to consume; update the snippet so the `react-native bundle`
line references `--bundle-output` and the `npx bundle-stats` command still
points at the saved stats JSON.
In
@.opencode/skills/react-native-best-practices/references/bundle-barrel-exports.md:
- Around line 77-80: Change the unlabeled fenced code block containing the
warning "Warning: Require cycle: components/index.ts -> Button.tsx ->
utils/index.ts -> components/index.ts" to include a language hint (e.g., replace
the opening ``` with ```text) so the block is labeled and passes markdown
linting and renderer consistency checks.
In
@.opencode/skills/react-native-best-practices/references/bundle-library-size.md:
- Around line 52-60: The three unlabeled fenced code blocks (the block starting
with "react-native-paper", the block containing
"https://bundlephobia.com/package/[package-name]", and the block with "moment
(289 kB) vs date-fns (75 kB) vs dayjs (6 kB)") should be updated to include an
explicit language tag (use "text") after the opening backticks to satisfy
markdownlint MD040; locate the fenced blocks around the shown snippets and
change ``` to ```text for each one.
In
@.opencode/skills/react-native-best-practices/references/bundle-native-assets.md:
- Line 33: Add language identifiers to every fenced code block that currently
lacks them to satisfy markdownlint MD040: update each triple-backtick fence so
it starts with an appropriate language tag (e.g., ```bash, ```sh, ```json,
```js, etc.) matching the example content; ensure all previously untagged code
fences in the document are changed to include the correct language identifier.
In
@.opencode/skills/react-native-best-practices/references/bundle-tree-shaking.md:
- Around line 121-123: The example uses Platform.select incorrectly by comparing
its boolean return to the string 'ios'; update the condition to use the boolean
directly (e.g., if (Platform.select({ ios: true, android: false })) ) or,
alternately, use Platform.OS === 'ios' for string-based checking; modify the
example referencing Platform.select so the conditional correctly evaluates
per-platform and remove the incorrect === 'ios' comparison.
In
@.opencode/skills/react-native-best-practices/references/js-lists-flatlist-flashlist.md:
- Around line 94-95: The FlatList examples use index-based keys via the
keyExtractor prop (keyExtractor={(item, index) => index.toString()}); replace
these with stable unique IDs by changing keyExtractor to use item.id (e.g.,
keyExtractor={(item) => item.id}) in both examples so they match the guidance in
the document; ensure the data items expose an id field or add a fallback (e.g.,
item.id ?? item.uuid) if necessary.
In @.opencode/skills/react-native-best-practices/references/js-profile-react.md:
- Around line 56-60: The fenced steps block containing the three profiling steps
(starting with 'Click "Start profiling" (blue circle) or "Reload and start
profiling"') is missing a language label and triggers MD040; update the fence to
use a language tag (e.g., ```text) so the block is treated as plain text and
markdownlint passes.
In
@.opencode/skills/react-native-best-practices/references/js-react-compiler.md:
- Around line 71-79: The code block labeled as JSON contains a line comment ("//
app.json") which makes the snippet invalid JSON; remove the comment or change
the fence to jsonc so the example is valid. Specifically edit the code block
showing app.json (the object with "expo" -> "experiments" -> "reactCompiler":
true) to either drop the leading comment line or mark the block as jsonc so the
snippet can be pasted directly without parse errors.
- Around line 124-153: Replace the deprecated eslint-plugin-react-compiler usage
in the Expo setup with eslint-plugin-react-hooks@latest: update the install
command (replace "npx expo install eslint-plugin-react-compiler -- -D" with
installing eslint-plugin-react-hooks@latest) and change the .eslintrc.js
references so that the imported symbol (currently reactCompiler) refers to
eslint-plugin-react-hooks and you use its preset (recommended-latest) instead of
reactCompiler.configs.recommended while keeping defineConfig and expoConfig
intact; also ensure any mention of "compiler" rules/presets is replaced with the
react-hooks "recommended-latest" preset.
In
@.opencode/skills/react-native-best-practices/references/native-android-16kb-alignment.md:
- Line 15: Update the table row containing "Google Play deadline | November 1,
2025 for apps targeting Android 15+" to remove the historical phrasing and make
it clear the requirement is in effect (e.g., change to "Effective since November
1, 2025 for apps targeting Android 15+" or "Requirement effective November 1,
2025 for apps targeting Android 15+"); locate the exact table cell text in
native-android-16kb-alignment.md and replace the dating phrase accordingly so it
reads as an ongoing requirement rather than a future deadline.
- Around line 55-58: Replace the fragile text-match check (the grep on
alignment.log) with a direct check of zipalign's exit status: run zipalign -c
with tee to capture output into alignment.log but ensure the script respects
zipalign's exit code (use set -o pipefail or inspect PIPESTATUS[0] after the
piped command) and exit non‑zero if zipalign failed; keep the alignment.log for
debugging.
In
@.opencode/skills/react-native-best-practices/references/native-measure-tti.md:
- Around line 73-79: The fenced code block containing the numbered lifecycle
list (which mentions symbols like nativeLaunchStart, nativeLaunchEnd,
appCreationStart, contentAppeared, and screenInteractive) is missing a language
tag; fix it by adding an appropriate language identifier (for example "text" or
"md") immediately after the opening triple backticks so the block satisfies
MD040 and the renderer/linter recognizes the fence language.
In
@.opencode/skills/react-native-best-practices/references/native-platform-setup.md:
- Line 21: Update the documentation entry that currently reads "bundle install"
(occurrences at the short install steps, e.g., the lines containing the literal
"bundle install") to clarify that this command installs gems listed in the
Gemfile, not Bundler itself; change the accompanying text to explicitly state
"Installs gems listed in your Gemfile (does not install Bundler)" and apply the
same clarification to the other occurrence noted (the second "bundle install"
instance).
- Around line 78-79: Update the two Gradle wrapper command examples that
currently use "./gradlew tasks" and "./gradlew assembleRelease" so they use the
repo-root-safe form "android/gradlew tasks" and "android/gradlew
assembleRelease" (matching the style used on the previous line) to ensure the
commands work when run from the repository root and support safe copy/paste.
In @.opencode/skills/react-native-best-practices/references/native-profiling.md:
- Around line 105-111: The fenced code block showing "JS Thread activity after
button press" lacks a language identifier; update that markdown block to include
a language tag (use "text") so the block starts with ```text and ends with ```,
e.g., change the opening fence before "JS Thread activity after button press:"
to ```text to satisfy linting and preserve formatting.
In
@.opencode/skills/react-native-best-practices/references/native-threading-model.md:
- Around line 139-142: The fenced code block containing the lines "JS Thread:
Calculate Yoga tree → Shadow tree" and "Main Thread: Apply layout to native
views" is missing a language identifier (MD040); update that triple-backtick
fence to include an appropriate language tag (e.g., ```text or ```text.js) so
the block is marked with a language, keeping the block content unchanged and
preserving the arrow character.
In `@src/app/_layout.tsx`:
- Around line 59-74: Update the beforeSend hook to use Sentry types instead of
any: import Event and EventHint (and if needed Exception and StackFrame types)
from `@sentry/react-native` and change the signature to beforeSend(event: Event,
hint: EventHint). Replace the local any annotations (the event parameter and the
frames/frame variables used in the top.stacktrace handling) with the proper
types (e.g., Event, EventException or SentryException, and StackFrame[]) and
adjust null checks accordingly so the existing logic that inspects top.type,
top.value, and top.stacktrace.frames (and functions like _cameraForBounds,
GeolocateControl, fromInvProjectionMatrix) works with the correct typed
properties.
In `@src/app/`(app)/_layout.tsx:
- Around line 180-183: Replace the logger.warn call with logger.error for the
initialization failure case: change logger.warn({ message: 'Failed to initialize
app', context: { error } }) to logger.error({ message: 'Failed to initialize
app', context: { error } }) so the same message and context (including the error
object) are reported via logger.error (which preserves Sentry visibility and
respects automatic redaction).
- Around line 229-240: The effect that calls initializeApp() can permanently
stop retrying because it only depends on status and initializeApp while
hasInitialized/isInitializing are refs; update the logic so failures trigger
re-run by introducing a state or counter (e.g., initAttempt or initErrorCount)
that you increment when initializeApp() rejects, include that state in the
effect deps, and ensure initializeApp() catch handlers update that counter (and
optionally set isInitializing/hasInitialized appropriately) so the effect will
re-evaluate and retry initialization within the same foreground session;
references: initializeApp, hasInitialized, isInitializing, status.
---
Outside diff comments:
In `@src/api/common/client.tsx`:
- Around line 109-115: The non-recoverable token refresh branch currently calls
logger.warn which prevents automatic Sentry reporting; change the call to
logger.error and include the refreshError and contextual info so Sentry receives
the event (update the block guarded by isNetworkError, replacing logger.warn
with logger.error and preserving the message and context), and keep the
subsequent useAuthStore.getState().logout() call as-is so the logout is still
performed but now recorded as an error for monitoring.
In `@src/services/location.ts`:
- Around line 50-59: The catch block handling location send failures currently
calls logger.warn which avoids automatic Sentry reporting; update the catch in
the function that sends location (the block referencing
location.coords.latitude/longitude) to call logger.error instead, preserving the
same structured payload (message, context with error details, latitude,
longitude) so Sentry receives the error and details for centralized tracking.
In `@src/services/push-notification.ts`:
- Around line 517-523: The catch block at the end of
registerForPushNotifications incorrectly downgrades all failures to logger.warn;
change it to treat system/backend failures as errors by calling
logger.error(...) and send the exception to Sentry (e.g.,
Sentry.captureException(error)), while still returning null for graceful
degradation. Keep permission-denial warnings as-is where they are logged earlier
(permission checks at the lines calling requestPermission()/hasPermission()),
but in the catch covering messaging().getToken() and registerUnitDevice() ensure
the log message includes the error object and context so backend/token failures
are tracked consistently with the calling code's expectations.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 182-189: The catch block handling SignalR disconnection failures
in the disconnect handler currently logs with logger.warn and sets state; change
this to use logger.error so the event is reported to Sentry (replace the
logger.warn call with logger.error while preserving the message and context
object), keep the error normalization (error instanceof Error ? error : new
Error(...)) and the set({ error: err }) behavior intact; look for the catch
within the disconnect logic in signalr-store (the block that constructs err and
calls logger.warn) to make this change.
- Around line 169-176: The catch block in the SignalR connection code currently
calls logger.warn which prevents Sentry reporting; replace the logger.warn call
inside the catch (the one creating the object with message 'Failed to connect to
SignalR hubs' and context: { error: err }) with logger.error so the error is
reported to Sentry, preserving the same message and context object (and continue
to call set({ error: err })). Ensure you use the same error variable (err) and
do not strip the Error object when calling logger.error.
- Around line 252-259: The catch block handling SignalR hub disconnection
currently logs with logger.warn, which does not trigger Sentry reporting; change
it to logger.error so Sentry receives these failures. Update the catch in the
disconnect/cleanup code (the block that creates err from error and calls
logger.warn) to call logger.error with the same message and context { error: err
} and keep set({ error: err }) unchanged so the error is both reported and
stored in state.
- Around line 239-246: The catch block that currently creates `err` and calls
`logger.warn({ message: 'Failed to connect to SignalR hubs', context: { error:
err } })` must use error-level logging so Sentry picks it up: replace
`logger.warn` with `logger.error` (keeping the same message/context) in the
SignalR connection catch (the block that constructs `err` and calls `set({
error: err })`), and optionally include "geolocation" in the message to make it
clear this is a geolocation hub connection failure.
---
Nitpick comments:
In
@.opencode/skills/react-native-best-practices/references/native-view-flattening.md:
- Around line 25-29: The examples using collapsable={false} on custom components
(NativeTabBar with Tab1, Tab2 and TabContent) must note that collapsable only
takes effect if the custom component forwards the prop to an underlying native
View; update the examples and copy or reference the forwarding pattern shown
later (the component that spreads/forwards props into the native View) so
readers know to forward collapsable (or use {...props}) in Tab1/TabContent
implementations; explicitly mention the requirement to forward collapsable to
the native View in the main examples and either integrate the forwarding pattern
example next to them or call it out prominently.
In `@src/app/_layout.tsx`:
- Around line 59-74: Extract the inline Mapbox filter in the beforeSend handler
into a named helper function (e.g., isKnownMapboxGlobeProjectionError) and call
it from beforeSend; specifically, move the logic that inspects
event.exception?.values, picks the top frame, checks top.type === 'TypeError'
and top.value.includes("Cannot read properties of null (reading '3')"), and then
looks for frames whose function includes '_cameraForBounds', 'GeolocateControl'
or 'fromInvProjectionMatrix' into that helper so beforeSend simply returns null
when isKnownMapboxGlobeProjectionError(event) is true and otherwise returns the
original event; ensure the new function is exported or located near the top of
the file so it’s testable and referenced by Sentry.init's beforeSend.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3a3b2f1-f2c6-4041-9d94-8114fd8098e2
⛔ Files ignored due to path filters (12)
.opencode/skills/react-native-best-practices/references/images/bundle-treemap-source-map-explorer.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/controlled-textinput-pingpong.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/devtools-flamegraph.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/emerge-xray-ios.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/expo-atlas-treemap.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/flashlight-flatlist-vs-flashlist.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/fps-drop-graph.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/memory-heap-snapshot.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/tti-warm-start-diagram.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/view-hierarchy-flattening.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/xcode-instruments-templates.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/xcode-thread-view.pngis excluded by!**/*.png
📒 Files selected for processing (54)
.opencode/skills/react-native-best-practices/POWER.md.opencode/skills/react-native-best-practices/SKILL.md.opencode/skills/react-native-best-practices/agents/openai.yaml.opencode/skills/react-native-best-practices/references/bundle-analyze-app.md.opencode/skills/react-native-best-practices/references/bundle-analyze-js.md.opencode/skills/react-native-best-practices/references/bundle-barrel-exports.md.opencode/skills/react-native-best-practices/references/bundle-code-splitting.md.opencode/skills/react-native-best-practices/references/bundle-hermes-mmap.md.opencode/skills/react-native-best-practices/references/bundle-library-size.md.opencode/skills/react-native-best-practices/references/bundle-native-assets.md.opencode/skills/react-native-best-practices/references/bundle-r8-android.md.opencode/skills/react-native-best-practices/references/bundle-tree-shaking.md.opencode/skills/react-native-best-practices/references/js-animations-reanimated.md.opencode/skills/react-native-best-practices/references/js-atomic-state.md.opencode/skills/react-native-best-practices/references/js-bottomsheet.md.opencode/skills/react-native-best-practices/references/js-concurrent-react.md.opencode/skills/react-native-best-practices/references/js-lists-flatlist-flashlist.md.opencode/skills/react-native-best-practices/references/js-measure-fps.md.opencode/skills/react-native-best-practices/references/js-memory-leaks.md.opencode/skills/react-native-best-practices/references/js-profile-react.md.opencode/skills/react-native-best-practices/references/js-react-compiler.md.opencode/skills/react-native-best-practices/references/js-uncontrolled-components.md.opencode/skills/react-native-best-practices/references/native-android-16kb-alignment.md.opencode/skills/react-native-best-practices/references/native-measure-tti.md.opencode/skills/react-native-best-practices/references/native-memory-leaks.md.opencode/skills/react-native-best-practices/references/native-memory-patterns.md.opencode/skills/react-native-best-practices/references/native-platform-setup.md.opencode/skills/react-native-best-practices/references/native-profiling.md.opencode/skills/react-native-best-practices/references/native-sdks-over-polyfills.md.opencode/skills/react-native-best-practices/references/native-threading-model.md.opencode/skills/react-native-best-practices/references/native-turbo-modules.md.opencode/skills/react-native-best-practices/references/native-view-flattening.md.opencode/skills/upgrading-react-native/SKILL.md.opencode/skills/upgrading-react-native/agents/openai.yaml.opencode/skills/upgrading-react-native/references/expo-sdk-upgrade.md.opencode/skills/upgrading-react-native/references/monorepo-singlerepo-targeting.md.opencode/skills/upgrading-react-native/references/react.md.opencode/skills/upgrading-react-native/references/upgrade-helper-core.md.opencode/skills/upgrading-react-native/references/upgrade-verification.md.opencode/skills/upgrading-react-native/references/upgrading-dependencies.md.opencode/skills/upgrading-react-native/references/upgrading-react-native.mdplugins/withCheckInLiveActivity.jssrc/api/common/client.tsxsrc/app/(app)/_layout.tsxsrc/app/_layout.tsxsrc/components/calls/call-files-modal.tsxsrc/services/__tests__/location.test.tssrc/services/location.tssrc/services/push-notification.tssrc/stores/app/__tests__/core-store.test.tssrc/stores/app/core-store.tssrc/stores/calls/store.tssrc/stores/signalr/__tests__/signalr-store.test.tssrc/stores/signalr/signalr-store.ts
Comment on lines
+127
to
+131
| ``` | ||
| Variant: SampleApp-<UUID>.ipa | ||
| App + On Demand Resources size: 3.5 MB compressed, 10.6 MB uncompressed | ||
| App size: 3.5 MB compressed, 10.6 MB uncompressed | ||
| ``` |
There was a problem hiding this comment.
Specify a language for this fenced block.
Add a language token (e.g., text) to satisfy markdown linting and keep docs rendering consistent.
Proposed fix
-```
+```text
Variant: SampleApp-<UUID>.ipa
App + On Demand Resources size: 3.5 MB compressed, 10.6 MB uncompressed
App size: 3.5 MB compressed, 10.6 MB uncompressed</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 127-127: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.opencode/skills/react-native-best-practices/references/bundle-analyze-app.md
around lines 127 - 131, The fenced code block that begins with the lines
"Variant: SampleApp-<UUID>.ipa" is missing a language token which triggers
markdown linting errors; update the opening fence by adding a language
identifier (for example "text") so the block reads ```text and leave the block
contents unchanged; target the fenced block containing "Variant:
SampleApp-<UUID>.ipa" in the markdown to apply this change.
Comment on lines
+138
to
+148
| # Generate stats | ||
| npx react-native bundle \ | ||
| --platform android \ | ||
| --entry-file index.js \ | ||
| --dev false \ | ||
| --minify true \ | ||
| --json stats.json | ||
|
|
||
| # Analyze | ||
| npx bundle-stats --html --json stats.json | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does npx react-native bundlesupport a--json flag in current React Native CLI versions? If not, what is the recommended way to generate analyzer-compatible stats for Re.Pack/webpack/rspack workflows?
💡 Result:
No, npx react-native bundle does not support a --json flag in current React Native CLI versions. The official bundle command options include --entry-file, --platform, --dev, --minify, --bundle-output, --sourcemap-output, and others, but no --json option is listed in the CLI documentation [1][2].
Citations:
- 1: https://www.mintlify.com/facebook/react-native/cli/bundle
- 2: https://github.com/expo/react-native-community-cli/blob/master/docs/commands.md
Remove the unsupported --json flag from the bundle command example.
The --json stats.json flag at line 144 is not supported by react-native bundle. This command will fail when readers follow it. Use --bundle-output to specify output path instead, and save stats separately if needed for analysis tools.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.opencode/skills/react-native-best-practices/references/bundle-analyze-js.md
around lines 138 - 148, The example uses an unsupported --json flag on the
react-native bundle command; remove the `--json stats.json` token from the `npx
react-native bundle` invocation and instead use the `--bundle-output` flag to
write the bundle file (keep `--platform android`, `--entry-file index.js`,
`--dev false`, `--minify true`), then produce or save a separate stats JSON for
`npx bundle-stats` to consume; update the snippet so the `react-native bundle`
line references `--bundle-output` and the `npx bundle-stats` command still
points at the saved stats JSON.
Comment on lines
+77
to
+80
| ``` | ||
| Warning: Require cycle: | ||
| components/index.ts -> Button.tsx -> utils/index.ts -> components/index.ts | ||
| ``` |
There was a problem hiding this comment.
Add a language hint to the warning code fence.
The fenced block at Line 77 is unlabeled, which fails markdown lint and reduces renderer consistency.
Proposed fix
-```
+```text
Warning: Require cycle:
components/index.ts -> Button.tsx -> utils/index.ts -> components/index.ts</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.opencode/skills/react-native-best-practices/references/bundle-barrel-exports.md
around lines 77 - 80, Change the unlabeled fenced code block containing the
warning "Warning: Require cycle: components/index.ts -> Button.tsx ->
utils/index.ts -> components/index.ts" to include a language hint (e.g., replace
the opening ``` with ```text) so the block is labeled and passes markdown
linting and renderer consistency checks.
Comment on lines
+52
to
+60
| ``` | ||
| react-native-paper | ||
| ├── Minified: 312 kB | ||
| ├── Gzipped: 78 kB | ||
| └── Dependencies: 12 packages | ||
| ├── @callstack/react-theme-provider | ||
| ├── color | ||
| └── ... | ||
| ``` |
There was a problem hiding this comment.
Add explicit languages to fenced code blocks to satisfy markdownlint.
Line 52, Line 102, and Line 107 use unlabeled fenced blocks, which triggers MD040 and may fail docs lint checks.
Suggested patch
-```
+```text
react-native-paper
├── Minified: 312 kB
├── Gzipped: 78 kB
└── Dependencies: 12 packages
├── `@callstack/react-theme-provider`
├── color
└── ...-
https://bundlephobia.com/package/[package-name]
-
moment (289 kB) vs date-fns (75 kB) vs dayjs (6 kB)
</details>
Also applies to: 102-109
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.opencode/skills/react-native-best-practices/references/bundle-library-size.md
around lines 52 - 60, The three unlabeled fenced code blocks (the block starting
with "react-native-paper", the block containing
"https://bundlephobia.com/package/[package-name]", and the block with "moment
(289 kB) vs date-fns (75 kB) vs dayjs (6 kB)") should be updated to include an
explicit language tag (use "text") after the opening backticks to satisfy
markdownlint MD040; locate the fenced blocks around the shown snippets and
change totext for each one.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| React Native convention for multiple resolutions: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
The fences at Line 33, Line 57, Line 80, Line 122, Line 137, Line 147, and Line 190 are missing language tags (MD040), which will keep lint failing.
Also applies to: 57-57, 80-80, 122-122, 137-137, 147-147, 190-190
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.opencode/skills/react-native-best-practices/references/bundle-native-assets.md
at line 33, Add language identifiers to every fenced code block that currently
lacks them to satisfy markdownlint MD040: update each triple-backtick fence so
it starts with an appropriate language tag (e.g., ```bash, ```sh, ```json,
```js, etc.) matching the example content; ensure all previously untagged code
fences in the document are changed to include the correct language identifier.
Comment on lines
+105
to
+111
| ``` | ||
| JS Thread activity after button press: | ||
| - Event handler on main thread | ||
| - Triggers JS work via sync JSI calls | ||
| - Hermes processes React reconciliation | ||
| - ~30% time in "commit" phase (Yoga layout) | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
The block should declare a language (likely text) to satisfy markdown linting and keep formatting consistent.
Proposed fix
-```
+```text
JS Thread activity after button press:
- Event handler on main thread
- Triggers JS work via sync JSI calls
- Hermes processes React reconciliation
- ~30% time in "commit" phase (Yoga layout)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.opencode/skills/react-native-best-practices/references/native-profiling.md
around lines 105 - 111, The fenced code block showing "JS Thread activity after
button press" lacks a language identifier; update that markdown block to include
a language tag (use "text") so the block starts with ```text and ends with ```,
e.g., change the opening fence before "JS Thread activity after button press:"
to ```text to satisfy linting and preserve formatting.
Comment on lines
+139
to
+142
| ``` | ||
| JS Thread: Calculate Yoga tree → Shadow tree | ||
| Main Thread: Apply layout to native views | ||
| ``` |
There was a problem hiding this comment.
Specify a language for this fenced code block.
The block starting at Line 139 has no language identifier (MD040).
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 139-139: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.opencode/skills/react-native-best-practices/references/native-threading-model.md
around lines 139 - 142, The fenced code block containing the lines "JS Thread:
Calculate Yoga tree → Shadow tree" and "Main Thread: Apply layout to native
views" is missing a language identifier (MD040); update that triple-backtick
fence to include an appropriate language tag (e.g., ```text or ```text.js) so
the block is marked with a language, keeping the block content unchanged and
preserving the arrow character.
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes
Improvements