Conversation
|
Important Review skippedToo many files! This PR contains 131 files, which is 31 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (25)
📒 Files selected for processing (131)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
| <View> | ||
| <Button | ||
| title="Load Feature" | ||
| onPress={() => setShowFeature(true)} |
There was a problem hiding this comment.
Inline arrow functions inside JSX props create new function instances on every render, degrading performance. Move the function definitions outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File .claude/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 150:
Inline arrow functions inside JSX props create new function instances on every render, degrading performance. Move the function definitions outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
|
|
||
| ScriptManager.shared.on('error', (scriptId, error) => { | ||
| console.error(`Failed: ${scriptId}`, error); |
There was a problem hiding this comment.
Unstructured error logging via console.error with a template string reduces searchability and violates Rule [3]. Replace this call with a structured logger such as logger.error('chunk_load_failed', { scriptId, err: error }).
Kody rule violation: Include error context in structured logs
Prompt for LLM
File .claude/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 231:
Unstructured error logging via `console.error` with a template string reduces searchability and violates Rule [3]. Replace this call with a structured logger such as `logger.error('chunk_load_failed', { scriptId, err: error })`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| ```tsx | ||
| // Check if chunk loaded correctly | ||
| ScriptManager.shared.on('loading', (scriptId) => { |
There was a problem hiding this comment.
Memory leak risk identified where an event listener registered on ScriptManager lacks a corresponding removal path, violating Rule [57]. Store the unsubscribe handle and invoke it during cleanup using const off = ScriptManager.shared.on('loading', handler); /* in cleanup */ off();.
Kody rule violation: Proper memory management in event listeners
Prompt for LLM
File .claude/skills/react-native-best-practices/references/bundle-code-splitting.md:
Line 222:
Memory leak risk identified where an event listener registered on `ScriptManager` lacks a corresponding removal path, violating Rule [57]. Store the unsubscribe handle and invoke it during cleanup using `const off = ScriptManager.shared.on('loading', handler); /* in cleanup */ off();`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <FlatList | ||
| data={items} | ||
| renderItem={renderItem} | ||
| keyExtractor={(item, index) => index.toString()} |
There was a problem hiding this comment.
Use of an array index as a key in the 'BetterList' keyExtractor causes reordering issues when items are mutated, violating Rule [56] and the document's own guidance at line 237. Use keyExtractor={(item) => item.id} as correctly demonstrated in lines 26 and 153.
Kody rule violation: Avoid array indexes as keys in React lists
Prompt for LLM
File .claude/skills/react-native-best-practices/references/js-lists-flatlist-flashlist.md:
Line 94:
Use of an array index as a key in the 'BetterList' `keyExtractor` causes reordering issues when items are mutated, violating Rule [56] and the document's own guidance at line 237. Use `keyExtractor={(item) => item.id}` as correctly demonstrated in lines 26 and 153.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| ```jsx | ||
| useEffect(() => { | ||
| const sub = EventEmitter.addListener('event', handler); |
There was a problem hiding this comment.
Unhandled subscription error path identified in EventEmitter.addListener. Provide error handlers to the subscription API and define a deterministic cleanup path.
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File .claude/skills/react-native-best-practices/references/js-memory-leaks.md:
Line 17:
Unhandled subscription error path identified in `EventEmitter.addListener`. Provide error handlers to the subscription API and define a deterministic cleanup path.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // BAD: Memory leak | ||
| const BadTimerComponent = () => { | ||
| useEffect(() => { | ||
| const timer = setInterval(() => { |
There was a problem hiding this comment.
Resource leak identified in setInterval without a deterministic cleanup path. Return a cleanup function that clears the timer, e.g., return () => clearInterval(timer);.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File .claude/skills/react-native-best-practices/references/js-memory-leaks.md:
Line 126:
Resource leak identified in `setInterval` without a deterministic cleanup path. Return a cleanup function that clears the timer, e.g., `return () => clearInterval(timer);`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const handleChange = (text) => { | ||
| setQuery(text); | ||
| fetchResults(text).then(setResults); |
There was a problem hiding this comment.
Missing debounce mechanism in the recommended handleChange example triggers fetchResults on every keystroke, violating Rule [53] for rapid input network calls. Apply debounce to the fetch call using const debouncedSearch = debounce((text) => fetchResults(text).then(setResults).catch(...), 300);.
Kody rule violation: Debounce or throttle user input that triggers work
Prompt for LLM
File .claude/skills/react-native-best-practices/references/js-uncontrolled-components.md:
Line 153:
Missing debounce mechanism in the recommended `handleChange` example triggers `fetchResults` on every keystroke, violating Rule [53] for rapid input network calls. Apply debounce to the fetch call using `const debouncedSearch = debounce((text) => fetchResults(text).then(setResults).catch(...), 300);`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ```tsx | ||
| import performance from 'react-native-performance'; | ||
|
|
||
| export default function HomeScreen() { |
There was a problem hiding this comment.
Default export used for the HomeScreen component introduces naming inconsistencies and complicates refactoring. Change this to a named export using export function HomeScreen() or function HomeScreen() {} with export { HomeScreen };.
Kody rule violation: Avoid default exports
Prompt for LLM
File .claude/skills/react-native-best-practices/references/native-measure-tti.md:
Line 173:
Default export used for the `HomeScreen` component introduces naming inconsistencies and complicates refactoring. Change this to a named export using `export function HomeScreen()` or `function HomeScreen() {}` with `export { HomeScreen };`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| override fun heavyOperation(input: Double, promise: Promise?) { | ||
| moduleScope.launch { | ||
| // Heavy work on coroutine | ||
| val result = expensiveComputation(input) | ||
| promise?.resolve(result) | ||
| } | ||
| } |
There was a problem hiding this comment.
Unhandled promise rejection identified in the Android coroutine implementation where expensiveComputation lacks a try/catch block, causing the JS promise to hang indefinitely if an exception occurs. Wrap the body in try { val result = ...; promise?.resolve(result) } catch (e: Exception) { promise?.reject("ERROR", e.message, e) }, mirroring the example at lines 251-263.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File .claude/skills/react-native-best-practices/references/native-turbo-modules.md:
Line 162 to 168:
Unhandled promise rejection identified in the Android coroutine implementation where `expensiveComputation` lacks a `try/catch` block, causing the JS promise to hang indefinitely if an exception occurs. Wrap the body in `try { val result = ...; promise?.resolve(result) } catch (e: Exception) { promise?.reject("ERROR", e.message, e) }`, mirroring the example at lines 251-263.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }, [extremeAlerts.length]); | ||
| const handleWeatherAlertBannerPress = useCallback(() => { | ||
| dismissBanner(); | ||
| router.push('/(app)/weather-alerts'); |
There was a problem hiding this comment.
Hardcoded route string literal identified in router.push, violating Rule [7] for centralized, reusable string constants. Define a route constant such as const WEATHER_ALERTS_ROUTE = '/(app)/weather-alerts' and reference it in the router call.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/app/(app)/index.tsx:
Line 77:
Hardcoded route string literal identified in `router.push`, violating Rule [7] for centralized, reusable string constants. Define a route constant such as `const WEATHER_ALERTS_ROUTE = '/(app)/weather-alerts'` and reference it in the router call.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| import { Actionsheet } from '../actionsheet'; | ||
|
|
||
| const lastCloseOnOverlayClick = () => mockUIActionsheet.mock.calls[mockUIActionsheet.mock.calls.length - 1][0].closeOnOverlayClick; |
There was a problem hiding this comment.
Potential TypeError identified where array index access on mock.calls lacks bounds verification, throwing if the collection is empty. Use optional chaining (?.) when accessing indices: mock.calls[...]?.[0]?.closeOnOverlayClick.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/components/ui/__tests__/actionsheet.test.tsx:
Line 41:
Potential `TypeError` identified where array index access on `mock.calls` lacks bounds verification, throwing if the collection is empty. Use optional chaining (`?.`) when accessing indices: `mock.calls[...]?.[0]?.closeOnOverlayClick`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| import { Actionsheet } from '../actionsheet'; | ||
|
|
||
| const lastCloseOnOverlayClick = () => mockUIActionsheet.mock.calls[mockUIActionsheet.mock.calls.length - 1][0].closeOnOverlayClick; |
There was a problem hiding this comment.
Unsecured array indexing on mock.calls[...][0] can cause exceptions when the collection is empty. Use safer accessors or verify mock.calls is populated before accessing indices.
Kody rule violation: Check query results before accessing indices
Prompt for LLM
File src/components/ui/__tests__/actionsheet.test.tsx:
Line 41:
Unsecured array indexing on `mock.calls[...][0]` can cause exceptions when the collection is empty. Use safer accessors or verify `mock.calls` is populated before accessing indices.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }, [polygonGeoJSON]); | ||
| cameraRef.current.setCamera({ | ||
| centerCoordinate: [mapCenter.longitude, mapCenter.latitude], | ||
| zoomLevel: 8, |
There was a problem hiding this comment.
Magic number identified where the numeric literal 8 represents a zoom level without a named constant, unlike MAP_PADDING on line 15. Extract this value into a named constant such as const DEFAULT_ZOOM_LEVEL = 8; and reference it in both the setCamera call and the Camera component props, including its duplicate on line 64.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/weather-alerts/weather-alert-detail-map.tsx:
Line 47:
Magic number identified where the numeric literal `8` represents a zoom level without a named constant, unlike `MAP_PADDING` on line 15. Extract this value into a named constant such as `const DEFAULT_ZOOM_LEVEL = 8;` and reference it in both the `setCamera` call and the `Camera` component props, including its duplicate on line 64.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @@ -0,0 +1,93 @@ | |||
| import { CHECK_IN_TARGET_TYPE, type CheckInEligibilityContext, isCheckInTargetEligible } from '@/lib/check-in-eligibility'; | |||
|
|
|||
| export type CheckInTimerStatus = 'critical' | 'ok' | 'overdue' | 'unknown' | 'warning'; | |||
There was a problem hiding this comment.
Type duplication risk identified where CheckInTimerStatus is manually maintained alongside STATUS_COLORS (line 18) and STATUS_SEVERITY (line 26). Declare STATUS_COLORS with as const first, then derive the type using type CheckInTimerStatus = keyof typeof STATUS_COLORS to prevent drift.
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File src/lib/check-in-timer-utils.ts:
Line 3:
Type duplication risk identified where `CheckInTimerStatus` is manually maintained alongside `STATUS_COLORS` (line 18) and `STATUS_SEVERITY` (line 26). Declare `STATUS_COLORS` with `as const` first, then derive the type using `type CheckInTimerStatus = keyof typeof STATUS_COLORS` to prevent drift.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // A status with no destination (Detail 0) must not land on 'select-destination'. | ||
| const getInitialStepForStatus = (status: StatusType): StatusStep => { | ||
| if (toStatusNumber(status.Detail) > 0) { | ||
| return 'select-destination'; |
There was a problem hiding this comment.
Magic string identified where a bare literal 'select-destination' represents a StatusStep value, creating refactoring risks and duplication. Define StatusStep values as named constants using const STATUS_STEPS = { SELECT_DESTINATION: 'select-destination', ADD_NOTE: 'add-note' } as const or a string enum, and return the constant reference.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/stores/status/store.ts:
Line 68:
Magic string identified where a bare literal `'select-destination'` represents a `StatusStep` value, creating refactoring risks and duplication. Define `StatusStep` values as named constants using `const STATUS_STEPS = { SELECT_DESTINATION: 'select-destination', ADD_NOTE: 'add-note' } as const` or a string enum, and return the constant reference.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Description
This PR addresses multiple fixes and improvements related to check-in functionality, status workflows, weather alerts, and UI polish.
Check-in Fixes
Status Bottom Sheet Fixes
Weather Alert Fixes
07/23/2026 4:15:30 PM) from the Core API are now parsed correctly instead of showing "Invalid Date".UI Improvements