Conversation
|
Important Review skippedToo many files! This PR contains 189 files, which is 89 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 (1)
📒 Files selected for processing (189)
You can disable this status message by setting the 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 |
This comment has been minimized.
This comment has been minimized.
| // Deep-link scheme matching the app's linking scheme (env.js SCHEME). | ||
| // URL schemes are case-insensitive; argv/open-url matching is done lowercased. | ||
| const deepLinkScheme = 'ResgridDispatch'; | ||
| const deepLinkPrefix = deepLinkScheme.toLowerCase() + '://'; |
There was a problem hiding this comment.
String concatenation operator used to assemble the prefix string. Refactor to use a template literal (e.g., ${deepLinkScheme.toLowerCase()}://) for improved readability and fewer errors.
Kody rule violation: Use Template Literals Instead of String Concatenation
Prompt for LLM
File electron/main.js:
Line 32:
String concatenation operator used to assemble the prefix string. Refactor to use a template literal (e.g., `${deepLinkScheme.toLowerCase()}://`) for improved readability and fewer errors.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try { | ||
| callback(deepLinkUrl); | ||
| } catch (err) { | ||
| console.error('Error in deep-link callback:', err); |
There was a problem hiding this comment.
Unstructured error logging detected in the deep-link catch block. Replace console.error with a structured logger.error call, including operation names and relevant identifiers for searchability and correlation.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File electron/preload.js:
Line 66:
Unstructured error logging detected in the `deep-link` catch block. Replace `console.error` with a structured `logger.error` call, including operation names and relevant identifiers for searchability and correlation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ipcRenderer.on('deep-link', (_event, deepLinkUrl) => { | ||
| try { | ||
| callback(deepLinkUrl); | ||
| } catch (err) { | ||
| console.error('Error in deep-link callback:', err); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Memory leak vulnerability identified in onDeepLink due to a missing unsubscribe function for ipcRenderer.on. Return a deterministic cleanup function (e.g., () => ipcRenderer.removeListener('deep-link', handler)) to prevent duplicate listener stacking.
Kody rule violation: Provide error handlers to subscription/listener APIs
Prompt for LLM
File electron/preload.js:
Line 62 to 68:
Memory leak vulnerability identified in `onDeepLink` due to a missing unsubscribe function for `ipcRenderer.on`. Return a deterministic cleanup function (e.g., `() => ipcRenderer.removeListener('deep-link', handler)`) to prevent duplicate listener stacking.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ipcRenderer.on('deep-link', (_event, deepLinkUrl) => { | ||
| try { | ||
| callback(deepLinkUrl); | ||
| } catch (err) { | ||
| console.error('Error in deep-link callback:', err); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Resource leak identified where ipcRenderer.on event listeners are never removed. Store the listener reference and expose a removal function to allow proper cleanup on unmount.
Kody rule violation: Proper memory management in event listeners
Prompt for LLM
File electron/preload.js:
Line 62 to 68:
Resource leak identified where `ipcRenderer.on` event listeners are never removed. Store the listener reference and expose a removal function to allow proper cleanup on unmount.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
| })(); | ||
|
|
||
| // Swallow post-timeout rejections so they don't surface as unhandled, and |
There was a problem hiding this comment.
Deadlock state in initialization logic permanently blocks retries if initPromise never settles. Reset isInitializing.current = false in the catch block after the 30-second timeout rejection to unblock the initializeApp guard at line 101.
initPromise
.catch((error) => {
if (initTimedOut.current) {
logger.error({
message: 'App initialization failed after initialization timeout',
context: { error, platform: Platform.OS },
});
}
})
.finally(() => {
isInitializing.current = false;
});
await Promise.race([initPromise, initTimeout]);
} catch (error) {
logger.error({
message: 'Failed to initialize app',
context: { error, platform: Platform.OS },
});
// Reset initialization state on error so it can be retried
hasInitialized.current = false;
// If the init promise is still hanging, clear the guard so a retry is possible
isInitializing.current = false;
}Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 198:
Deadlock state in initialization logic permanently blocks retries if `initPromise` never settles. Reset `isInitializing.current = false` in the catch block after the 30-second timeout rejection to unblock the `initializeApp` guard at line 101.
Suggested Code:
initPromise
.catch((error) => {
if (initTimedOut.current) {
logger.error({
message: 'App initialization failed after initialization timeout',
context: { error, platform: Platform.OS },
});
}
})
.finally(() => {
isInitializing.current = false;
});
await Promise.race([initPromise, initTimeout]);
} catch (error) {
logger.error({
message: 'Failed to initialize app',
context: { error, platform: Platform.OS },
});
// Reset initialization state on error so it can be retried
hasInitialized.current = false;
// If the init promise is still hanging, clear the guard so a retry is possible
isInitializing.current = false;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| weatherLongitude={mapCenterLongitude} | ||
| extremeAlerts={extremeAlertCount} | ||
| severeAlerts={severeAlertCount} | ||
| onWeatherAlertsPress={() => router.push('/(app)/weather-alerts' as Href)} |
There was a problem hiding this comment.
Performance degradation caused by inline arrow functions within JSX props. Move function definitions outside the render method to prevent unnecessary memory allocations and re-renders.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/home.tsx:
Line 950:
Performance degradation caused by inline arrow functions within JSX props. Move function definitions outside the render method to prevent unnecessary memory allocations and re-renders.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const setMapCenter = useDispatchConsoleStore((s) => s.setMapCenter); | ||
|
|
||
| // Local state | ||
| const [currentTime, setCurrentTime] = useState(new Date().toLocaleTimeString('en-US', { hour12: false })); |
There was a problem hiding this comment.
Unnecessary state re-render detected in DispatchConsoleWeb due to an unused currentTime setInterval. Remove the currentTime useState and the setInterval useEffect (lines 140 and 152-159) to eliminate redundant renders.
// Removed: currentTime is no longer passed to StatsHeader (it manages its own clock)
// const [currentTime, setCurrentTime] = useState(...);
// useEffect(() => { ... setInterval ... }, []);Prompt for LLM
File src/app/(app)/home.web.tsx:
Line 140:
Unnecessary state re-render detected in `DispatchConsoleWeb` due to an unused `currentTime` `setInterval`. Remove the `currentTime` useState and the `setInterval` useEffect (lines 140 and 152-159) to eliminate redundant renders.
Suggested Code:
// Removed: currentTime is no longer passed to StatsHeader (it manages its own clock)
// const [currentTime, setCurrentTime] = useState(...);
// useEffect(() => { ... setInterval ... }, []);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| logger.info({ | ||
| message: 'Location updated and map is ready', | ||
| context: { | ||
| latitude: location.latitude, |
There was a problem hiding this comment.
PII exposure risk identified by logging raw geolocation latitude. Hash or bucket the value (e.g., Math.round(location.latitude * 10) / 10) to comply with GDPR/LGPD data minimization requirements.
Kody rule violation: Mask PII and secrets in logs
Prompt for LLM
File src/app/(app)/map.tsx:
Line 173:
PII exposure risk identified by logging raw geolocation latitude. Hash or bucket the value (e.g., `Math.round(location.latitude * 10) / 10`) to comply with GDPR/LGPD data minimization requirements.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| logger.info({ | ||
| message: 'Location updated and map is ready', | ||
| context: { | ||
| latitude: location.latitude, |
There was a problem hiding this comment.
GDPR compliance violation detected from emitting verbatim latitude in diagnostic logs. Replace raw values with bucketed/hashed coordinates and append gdpr: { purpose: 'map_tracking', lawful_basis: 'consent' } to the context payload.
Kody rule violation: Redact PII in logs and metrics by default
Prompt for LLM
File src/app/(app)/map.tsx:
Line 173:
GDPR compliance violation detected from emitting verbatim latitude in diagnostic logs. Replace raw values with bucketed/hashed coordinates and append `gdpr: { purpose: 'map_tracking', lawful_basis: 'consent' }` to the context payload.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <html><head> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <script src="https://cdn.jsdelivr.net/npm/hls.js@latest"></script> | ||
| <script src="https://cdn.jsdelivr.net/npm/hls.js@1"></script> |
There was a problem hiding this comment.
Unsafe third-party script injection detected for hls.js. Utilize the Next.js <Script> component with an appropriate loading strategy and onLoad callbacks instead of raw inline <script> tags.
Kody rule violation: Use next/script safely and avoid inline scripts
Prompt for LLM
File src/components/callVideoFeeds/video-player.tsx:
Line 51:
Unsafe third-party script injection detected for `hls.js`. Utilize the Next.js `<Script>` component with an appropriate loading strategy and `onLoad` callbacks instead of raw inline `<script>` tags.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const fileName = file.FileName || file.Name || `file_${file.Id}`; | ||
|
|
||
| if (Platform.OS === 'web') { | ||
| const objectUrl = URL.createObjectURL(fileData); |
There was a problem hiding this comment.
Unhandled exception risk found in the web-platform download path during URL.createObjectURL execution. Wrap the DOM API operations in a try/catch block and utilize finally to clear the downloadingFiles state during failures.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/components/calls/call-files-modal.tsx:
Line 111:
Unhandled exception risk found in the web-platform download path during `URL.createObjectURL` execution. Wrap the DOM API operations in a try/catch block and utilize `finally` to clear the `downloadingFiles` state during failures.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| chunk.map((callId) => | ||
| getCallExtraData(callId) | ||
| .then((res) => ({ callId, dispatches: res?.Data?.Dispatches ?? ([] as DispatchedEventResultData[]) })) | ||
| .catch(() => ({ callId, dispatches: null as DispatchedEventResultData[] | null })) |
There was a problem hiding this comment.
Silent exception swallowing detected in the getCallExtraData catch handler. Implement structured logging with context (e.g., logger.error('getCallExtraData failed', { callId, err })) before returning default values.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/components/dispatch-console/active-calls-panel.tsx:
Line 418:
Silent exception swallowing detected in the `getCallExtraData` catch handler. Implement structured logging with context (e.g., `logger.error('getCallExtraData failed', { callId, err })`) before returning default values.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const CONCURRENCY = 4; | ||
| for (let i = 0; i < toFetch.length; i += CONCURRENCY) { | ||
| const chunk = toFetch.slice(i, i + CONCURRENCY); | ||
| const results = await Promise.all( |
There was a problem hiding this comment.
Batch execution failure risk identified using Promise.all for independent API calls. Replace with Promise.allSettled to isolate individual rejections and inspect each settlement's status explicitly.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/components/dispatch-console/active-calls-panel.tsx:
Line 414:
Batch execution failure risk identified using `Promise.all` for independent API calls. Replace with `Promise.allSettled` to isolate individual rejections and inspect each settlement's status explicitly.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| animation: isLoading ? 'rg-refresh-spin 1s linear infinite' : undefined, | ||
| }} | ||
| > | ||
| <style>{'@keyframes rg-refresh-spin { from { transform: rotate(0deg); } to { transform: rotate(360deg); } }'}</style> |
There was a problem hiding this comment.
CSS scope violation caused by a raw <style> tag injecting global @keyframes on every component render. Move the keyframes into a CSS Module (e.g., animated-refresh-icon.module.css) to prevent style collisions.
Kody rule violation: Use component-scoped styling
Prompt for LLM
File src/components/dispatch-console/animated-refresh-icon.web.tsx:
Line 21:
CSS scope violation caused by a raw `<style>` tag injecting global `@keyframes` on every component render. Move the keyframes into a CSS Module (e.g., `animated-refresh-icon.module.css`) to prevent style collisions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| export const NotesPanel: React.FC<NotesPanelProps> = ({ notes, isLoading, onRefresh, onSelectNote, onNewNote, isCallFilterActive, callNotes, onAddCallNote, isAddingNote, flexWeight }) => { | ||
| const { t } = useTranslation(); | ||
| const [isCollapsed, setIsCollapsed] = useState(false); | ||
| const isCollapsed = useDashboardViewStore(selectCardCollapsed('notes')); |
There was a problem hiding this comment.
Magic string vulnerability detected using the literal 'notes' as a card-identifier key. Extract shared keys into a centralized constant (e.g., NOTES_CARD_KEY) or an enum to prevent typos and streamline refactoring.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/components/dispatch-console/notes-panel.tsx:
Line 85:
Magic string vulnerability detected using the literal `'notes'` as a card-identifier key. Extract shared keys into a centralized constant (e.g., `NOTES_CARD_KEY`) or an enum to prevent typos and streamline refactoring.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </Pressable> | ||
| ) : null} | ||
| {onToggleCollapse ? <Icon as={isCollapsed ? ChevronDown : ChevronUp} size="sm" className="text-gray-500 dark:text-gray-400" /> : null} | ||
| {onToggleCollapse ? <Icon as={isCollapsed ? ChevronUp : ChevronDown} size="sm" className="text-gray-500 dark:text-gray-400" /> : null} |
There was a problem hiding this comment.
Inverted UI logic causes collapsed panels to display ChevronUp instead of ChevronDown, misleading user perception of state. Swap the conditional back to isCollapsed ? ChevronDown : ChevronUp to accurately reflect expansion state.
{onToggleCollapse ? <Icon as={isCollapsed ? ChevronDown : ChevronUp} size="sm" className="text-gray-500 dark:text-gray-400" /> : null}Prompt for LLM
File src/components/dispatch-console/panel-header.tsx:
Line 42:
Inverted UI logic causes collapsed panels to display ChevronUp instead of ChevronDown, misleading user perception of state. Swap the conditional back to `isCollapsed ? ChevronDown : ChevronUp` to accurately reflect expansion state.
Suggested Code:
{onToggleCollapse ? <Icon as={isCollapsed ? ChevronDown : ChevronUp} size="sm" className="text-gray-500 dark:text-gray-400" /> : null}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const buildPopupHtml = (pin: MapMakerInfoData) => | ||
| `<div style="padding: 8px;"> | ||
| <h3 style="margin: 0 0 8px 0; font-weight: 600;">${pin.Title}</h3> |
There was a problem hiding this comment.
Cross-Site Scripting (XSS) vulnerability identified by interpolating pin.Title directly into mapboxgl.Popup.setHTML(). Sanitize the value using escapeHtml() or DOMPurify before rendering untrusted remote data.
Kody rule violation: Always sanitize user inputs
Prompt for LLM
File src/components/maps/unified-map-view.web.tsx:
Line 229:
Cross-Site Scripting (XSS) vulnerability identified by interpolating `pin.Title` directly into `mapboxgl.Popup.setHTML()`. Sanitize the value using `escapeHtml()` or `DOMPurify` before rendering untrusted remote data.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const initialCenter: [number, number] = location.longitude && location.latitude ? [location.longitude, location.latitude] : [-98.5795, 39.8283]; | ||
| const { latitude, longitude } = useLocationStore.getState(); | ||
| const initialCenter: [number, number] = longitude && latitude ? [longitude, latitude] : [-98.5795, 39.8283]; |
There was a problem hiding this comment.
Magic numbers detected for default map center coordinates (-98.5795, 39.8283). Extract these values into a named constant (e.g., DEFAULT_CENTER) at module scope or a configuration file to improve maintainability.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/maps/unified-map-view.web.tsx:
Line 93:
Magic numbers detected for default map center coordinates (`-98.5795, 39.8283`). Extract these values into a named constant (e.g., `DEFAULT_CENTER`) at module scope or a configuration file to improve maintainability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| height: '100%', | ||
| backgroundColor: colorScheme.get() === 'dark' ? '#171717' : '#fff', | ||
| shadowColor: colorScheme.get() === 'dark' ? '#262626' : '#e5e5e5', | ||
| backgroundColor: Appearance.getColorScheme() === 'dark' ? '#171717' : '#fff', |
There was a problem hiding this comment.
Redundant calculation detected where Appearance.getColorScheme() is invoked independently in multiple style rules. Compute const isDark once at the module level and reference it in each ternary operation to optimize execution.
Kody rule violation: Use computed/derived properties for repeated calculations
Prompt for LLM
File src/components/notifications/NotificationDetail.tsx:
Line 210:
Redundant calculation detected where `Appearance.getColorScheme()` is invoked independently in multiple style rules. Compute `const isDark` once at the module level and reference it in each ternary operation to optimize execution.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| })); | ||
|
|
||
| jest.mock('nativewind', () => ({ | ||
| styled: jest.fn((Component: any) => Component), |
There was a problem hiding this comment.
Naming convention violation detected in the styled arrow-function parameter Component. Rename the parameter to camelCase (e.g., component) to comply with standard naming conventions for local variables.
Kody rule violation: Use proper naming conventions
Prompt for LLM
File src/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsx:
Line 20:
Naming convention violation detected in the `styled` arrow-function parameter `Component`. Rename the parameter to camelCase (e.g., `component`) to comply with standard naming conventions for local variables.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| type IAnimatedPressableProps = React.ComponentProps<typeof Pressable> & MotionComponentProps<typeof Pressable, ViewStyle, unknown, unknown, unknown>; | ||
|
|
||
| const AnimatedPressable = createMotionAnimatedComponent(Pressable) as React.ComponentType<IAnimatedPressableProps>; |
There was a problem hiding this comment.
Type safety violation found in createMotionAnimatedComponent casting. Utilize the as operator or pattern matching for safe casts and explicitly guard null results prior to usage.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File src/components/ui/actionsheet/index.tsx:
Line 23:
Type safety violation found in `createMotionAnimatedComponent` casting. Utilize the `as` operator or pattern matching for safe casts and explicitly guard null results prior to usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| : SlideInDown.duration(200); | ||
|
|
||
| const customClass = isHorizontal ? `top-0 ${parentAnchor === 'left' ? 'left-0' : 'right-0'}` : `left-0 ${parentAnchor === 'top' ? 'top-0' : 'bottom-0'}`; | ||
| const exitingAnimation = parentAnchor === 'left' ? SlideOutLeft.duration(200) : parentAnchor === 'right' ? SlideOutRight.duration(200) : parentAnchor === 'top' ? SlideOutUp.duration(200) : SlideOutDown.duration(200); |
There was a problem hiding this comment.
Code duplication identified in the parallel ternary chains of exitingAnimation and enteringAnimation. Extract a single lookup map (e.g., animMap) to synchronize anchor branching logic and simplify future maintenance.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/components/ui/drawer/index.tsx:
Line 171:
Code duplication identified in the parallel ternary chains of `exitingAnimation` and `enteringAnimation`. Extract a single lookup map (e.g., `animMap`) to synchronize anchor branching logic and simplify future maintenance.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| // Only render SystemBars when focused and on supported platforms | ||
| return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme} hidden={{ statusBar: hidden, navigationBar: true }} /> : null; | ||
| return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme === 'dark' ? 'dark' : 'light'} hidden={{ statusBar: hidden, navigationBar: true }} /> : null; |
There was a problem hiding this comment.
Magic strings detected hardcoding 'dark' and 'light' inline for SystemBars styling. Define a centralized enum (e.g., ThemeStyle) to replace these fragile string literals and prevent mistyping.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/ui/focus-aware-status-bar.tsx:
Line 67:
Magic strings detected hardcoding `'dark'` and `'light'` inline for `SystemBars` styling. Define a centralized enum (e.g., `ThemeStyle`) to replace these fragile string literals and prevent mistyping.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }; | ||
|
|
||
| export const clearPasswordVerificationHash = async (): Promise<void> => { | ||
| await removeItem(PASSWORD_VERIFICATION_HASH_KEY); |
There was a problem hiding this comment.
Unguarded asynchronous operation in clearPasswordVerificationHash allows unhandled promise rejection if removeItem fails. Wrap the awaited removeItem call in a try/catch block and log the error context via logger.error(...) to match sibling function patterns.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/lib/auth/api.tsx:
Line 75:
Unguarded asynchronous operation in `clearPasswordVerificationHash` allows unhandled promise rejection if `removeItem` fails. Wrap the awaited `removeItem` call in a try/catch block and log the error context via `logger.error(...)` to match sibling function patterns.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| toasts: state.toasts.filter((toast) => toast.id !== id), | ||
| })); | ||
| }, 3000); | ||
| }, options?.duration ?? 3000); |
There was a problem hiding this comment.
Memory leak risk caused by uncaptured setTimeout return values in the toast store. Capture the timer ID and invoke clearTimeout(timerId) during manual removal to prevent state-update-on-unmounted warnings.
Kody rule violation: Clear timers on teardown/unmount
Prompt for LLM
File src/stores/toast/store.ts:
Line 39:
Memory leak risk caused by uncaptured `setTimeout` return values in the toast store. Capture the timer ID and invoke `clearTimeout(timerId)` during manual removal to prevent state-update-on-unmounted warnings.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| try { | ||
| callback(deepLinkUrl); | ||
| } catch (err) { | ||
| console.error('Error in deep-link callback:', err); |
There was a problem hiding this comment.
Unstructured logging in the catch block bypasses centralized logging and violates Rule [3] by omitting required structured fields. Replace console.error with a structured logger call, such as logger.error('deep-link callback failed', { op: 'onDeepLink', err }), to include operation names and relevant identifiers.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File electron/preload.js:
Line 67:
Unstructured logging in the catch block bypasses centralized logging and violates Rule [3] by omitting required structured fields. Replace `console.error` with a structured logger call, such as `logger.error('deep-link callback failed', { op: 'onDeepLink', err })`, to include operation names and relevant identifiers.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // If the init promise is still hanging, clear the guard so a retry is possible | ||
| isInitializing.current = false; |
There was a problem hiding this comment.
Race condition triggered when the timeout catch block sets isInitializing.current = false and allows a retry while the previous init promise is still pending. The old promise's .finally block (lines 209-211) clears the guard early, causing duplicate SignalR hub connections (connectUpdateHub) and store inits; making the .finally conditional on an epoch counter resolves this.
// capture an instance id so only the CURRENT init's settle clears the guard
const initInstanceId = ++initInstanceRef.current;
initPromise
.catch((error) => { /* ... */ })
.finally(() => {
if (initInstanceRef.current === initInstanceId) {
isInitializing.current = false;
}
});
await Promise.race([initPromise, initTimeout]);
} catch (error) {
// ...
hasInitialized.current = false;
// Only clear the retry guard if we are still the active instance
if (initInstanceRef.current === initInstanceId) {
isInitializing.current = false;
}
}Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 221 to 222:
Race condition triggered when the timeout catch block sets `isInitializing.current = false` and allows a retry while the previous init promise is still pending. The old promise's `.finally` block (lines 209-211) clears the guard early, causing duplicate SignalR hub connections (`connectUpdateHub`) and store inits; making the `.finally` conditional on an epoch counter resolves this.
Suggested Code:
// capture an instance id so only the CURRENT init's settle clears the guard
const initInstanceId = ++initInstanceRef.current;
initPromise
.catch((error) => { /* ... */ })
.finally(() => {
if (initInstanceRef.current === initInstanceId) {
isInitializing.current = false;
}
});
await Promise.race([initPromise, initTimeout]);
} catch (error) {
// ...
hasInitialized.current = false;
// Only clear the retry guard if we are still the active instance
if (initInstanceRef.current === initInstanceId) {
isInitializing.current = false;
}
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| latitudeBucket: Math.round(location.latitude * 10) / 10, | ||
| longitudeBucket: Math.round(location.longitude * 10) / 10, |
There was a problem hiding this comment.
Code duplication identified: the rounding and bucketing logic is repeated across multiple call sites (src/app/(app)/map.tsx:221-222, 176-177). Duplicated logic drifts and complicates GDPR compliance auditing; extract a utility function like bucketCoordinate(v: number, scale = COORDINATE_BUCKET_SCALE) to centralize it.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/app/(app)/map.tsx:
Line 157 to 158:
Code duplication identified: the rounding and bucketing logic is repeated across multiple call sites (`src/app/(app)/map.tsx:221-222`, `176-177`). Duplicated logic drifts and complicates GDPR compliance auditing; extract a utility function like `bucketCoordinate(v: number, scale = COORDINATE_BUCKET_SCALE)` to centralize it.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| } | ||
| </script> | ||
| <script src="https://cdn.jsdelivr.net/npm/hls.js@1.6.16/dist/hls.min.js" |
There was a problem hiding this comment.
Inline script violation: using a raw <script> tag for the third-party library hls.js ignores next/script loading strategies. Replace it with next/script using an appropriate strategy and onLoad callbacks.
Kody rule violation: Use next/script safely and avoid inline scripts
Prompt for LLM
File src/components/callVideoFeeds/video-player.tsx:
Line 74:
Inline script violation: using a raw `<script>` tag for the third-party library `hls.js` ignores `next/script` loading strategies. Replace it with `next/script` using an appropriate strategy and `onLoad` callbacks.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| window.__hlsInitDone = true; | ||
| var url = ${JSON.stringify(feed.Url)}; | ||
| if (url.includes('.m3u8') && window.Hls && Hls.isSupported()) { | ||
| var hls = new Hls(); |
There was a problem hiding this comment.
Improper variable declaration: var hls is function-scoped and prone to accidental redeclaration, whereas const provides block-scoping and prevents reassignment. Replace var with const across all instances (src/components/callVideoFeeds/video-player.tsx:56, 64), as the Hls instance is never reassigned.
Kody rule violation: Always use const and let
Prompt for LLM
File src/components/callVideoFeeds/video-player.tsx:
Line 66:
Improper variable declaration: `var hls` is function-scoped and prone to accidental redeclaration, whereas `const` provides block-scoping and prevents reassignment. Replace `var` with `const` across all instances (`src/components/callVideoFeeds/video-player.tsx:56`, `64`), as the `Hls` instance is never reassigned.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| toasts: state.toasts.filter((toast) => toast.id !== id), | ||
| })); | ||
| }, 3000); | ||
| }, options?.duration ?? 3000); |
There was a problem hiding this comment.
Magic number identified: the literal 3000 represents the default toast auto-dismiss duration but reduces readability and makes future tuning error-prone. Extract a named constant such as DEFAULT_TOAST_DURATION_MS = 3000 and apply it in the ?? fallback across all occurrences in src/app/(app)/map.tsx:157-158, 176-177, and 221-222.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/stores/toast/store.ts:
Line 44:
Magic number identified: the literal `3000` represents the default toast auto-dismiss duration but reduces readability and makes future tuning error-prone. Extract a named constant such as `DEFAULT_TOAST_DURATION_MS = 3000` and apply it in the `??` fallback across all occurrences in `src/app/(app)/map.tsx:157-158`, `176-177`, and `221-222`.
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
Pull Request Description
RD-T43: Expo SDK 56, NativeWind v5, LiveKit 2.10, and Gluestack UI v5 Upgrade
This PR is a major framework upgrade that migrates the Resgrid Dispatch app to Expo SDK 56, NativeWind v5 (Tailwind CSS v4), Gluestack UI v5, and LiveKit 2.10, along with significant dispatch console performance improvements, Electron security hardening, and authentication enhancements.
Framework & Library Upgrades
NativeWind v4 → v5 (Tailwind CSS v3 → v4)
global.cssto Tailwind v4 syntax (@import,@theme,@custom-variant) with full light/dark design token scales defined as CSS custom properties@custom-variantandAppearance.setColorSchemeinstead of NativeWind'scolorScheme.set()cssInterop()calls with NativeWind v5'sstyled()wrapper across all Gluestack UI componentsshadow-sm→shadow-xs,rounded-sm→rounded-xs)Gluestack UI v3 → v5
@gluestack-ui/core/*creator paths@gluestack-ui/utils/nativewind-utilsUIIconfrom core replaces per-componentPrimitiveIcondefinitionswithStyleContextAndStates,withStates) unifiedReact.ComponentRef; context values memoizeduseWindowDimensionsfor reactive heightAnimation Migration (Legend Motion → Reanimated v4)
@legendapp/motiontoreact-native-reanimatedentering/exiting patternsDispatch Console Performance & Features
Performance
ScrollView.map()toFlatListwith virtualization (removeClippedSubviews, windowing)React.memowith stable,useCallback-derived handlersFeatures
persistmiddlewareDashboardViewTogglesmoved into the app header for all responsive layoutsonPresscallback and custom durationElectron Desktop Improvements
ResgridDispatch://URL scheme with single-instance lock,open-url(macOS) andsecond-instance(Windows/Linux) handlers, and renderer bridge via preload IPC..escape)child-src blob:, and plain HTTP/WebSocket for self-hosted Resgrid serversAuthentication & Security
localhost/127.0.0.1Bug Fixes & Robustness
invertColor()handles empty, alpha-channel, and invalid hex without throwingparseDateISOString()returnsnullinstead of throwing on invalid inputgetTimeAgo()/getTimeAgoUtc()handle non-numeric andNaNtimestampsInformationtoWarninglastUpdateMessageset tonullinstead of stringifying potentially large payloadsFlatListkey extractor falls back to index/URL instead of random IDsTesting & Infrastructure
styledadded tonativewindmocks)src/mocks.js)transformIgnorePatternsupdated to includenativewindandreact-native-css