fix: Add mobile app rules covering hybrid frontend (React Native).#9
fix: Add mobile app rules covering hybrid frontend (React Native).#9sagarkoshti1990 wants to merge 3 commits intotekdi:mainfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds extensive mobile-app guidance (hybrid frontend and React Native) as new markdown rules and examples, and enhances Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant Repo as Repo(Local/Remote)
participant FS as FileSystem
User->>Script: run copy-cursor-rules.sh [args]
Script->>Script: parse arguments (help, --debug)
alt debug mode
Script->>Repo: locate local repository path
else
Script->>Repo: clone repository from remote
end
Script->>Script: validate repo structure (expect 1-tekdi)
Script->>User: prompt for project type
User->>Script: select project type (backend/frontend/mobile-app)
Script->>User: prompt for language
User->>Script: select language
Script->>User: prompt for framework
User->>Script: select framework
Script->>FS: copy common rules
Script->>FS: copy language rules
Script->>FS: copy framework rules
alt framework == react-native
Script->>FS: also copy hybrid-frontend rules
end
Script->>User: print completion (suppress cleanup note if debug)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–90 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (20)
2-common/common-mobile-app.md (1)
1-1: Remove the no-op newline to avoid noisy blame.The commit only adds an empty line, creating needless churn in
git blame. If the file intentionally remains empty, delete the newline and add a short comment explaining why the file exists.6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-4-api-integration-service-layer.mdc (1)
2-2: Fix typo in front-matter description.
intergrating→integrating.-description: USE WHEN intergrating APIs +description: USE WHEN integrating APIs6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-7-accessibility-rules.mdc (1)
2-2: Polish front-matter grammar for clarity.Current text is missing a plural and reads awkwardly.
-description: USE WHEN creating inclusive, accessible web applications to ensure application work for all users, including those with disabilities +description: USE WHEN creating inclusive, accessible applications to ensure they work for all users, including those with disabilities6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-8-testing-rules.mdc (1)
2-2: Tighten description wording.Small grammar tweaks improve readability.
-description: USE WHEN writing test for ensuring testing and quality principles to build reliable, maintainable frontend applications with comprehensive test coverage and robust error handling +description: USE WHEN writing tests to uphold quality principles and build reliable, maintainable frontend applications with comprehensive coverage and robust error handling6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-6-security-rules.mdc (1)
2-2: Deduplicate and shorten the security description.The sentence repeats itself; tightening it improves comprehension.
-description: USE WHEN securing frontend applications that protect both your users and your systems from common web security threats protect your applications and users from common security vulnerabilities +description: USE WHEN securing frontend applications to protect users and systems from common web security threats6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-3-data-state-rules.mdc (1)
13-14: Clarify platform-specific storage APIs
localStorageis unavailable in React Native. Add explicit guidance for native equivalents (AsyncStorage,SecureStore) to avoid confusion for hybrid teams.- - Store session state in client-side or shared backing services (localStorage, SecureStore, etc.) + - Store session state in client-side or shared backing services + - Web: `localStorage`, `sessionStorage` + - React Native: `AsyncStorage`, `SecureStore` (for sensitive data)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-1-architecture-rules.mdc (1)
44-52: Recommend an “index” barrel guidelineTo reinforce the File Organization section, consider adding a rule on optional
index.{ts,js}barrels for feature folders, clarifying when they are helpful vs. when they hide structure.6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-2-ui-patterns-rules.mdc (1)
24-28: Define design-token source for breakpointsThe rule advises avoiding “magic numbers” but doesn’t specify where breakpoint constants live. Point readers to a central design-token file or theme object to keep implementations consistent.
6-mobile-app/javascript/react-native/react-native-3-data-state-rules.mdc (1)
284-303: HardenuseAsyncStorageagainst JSON-parse failures & race conditions
JSON.parsemay throw – wrap it.- Multiple quick
setValuecalls can suffer lost updates because theawaitis outsidesetStoredValue.- const item = await AsyncStorage.getItem(key); - if (item) setStoredValue(JSON.parse(item)); + const item = await AsyncStorage.getItem(key); + if (item) { + try { + setStoredValue(JSON.parse(item)); + } catch { + // silently ignore corrupt value + } + }- const setValue = async (value) => { - const valueToStore = value instanceof Function ? value(storedValue) : value; - setStoredValue(valueToStore); - await AsyncStorage.setItem(key, JSON.stringify(valueToStore)); - }; + const setValue = (value) => { + setStoredValue(prev => { + const valueToStore = value instanceof Function ? value(prev) : value; + AsyncStorage.setItem(key, JSON.stringify(valueToStore)).catch(() => {}); + return valueToStore; + }); + };6-mobile-app/javascript/react-native/react-native-5-performance-rules.mdc (1)
46-53:React.memocustom comparer ignores other user fields – potential stale rendersOnly
id,firstName,lastName,isSelected, andonEditare compared.
Ifuser.avatar,createdAt, or any later-added field changes, the card will not re-render, showing stale data.Consider either:
-}, (prevProps, nextProps) => { - return ( - prevProps.user.id === nextProps.user.id && - prevProps.user.firstName === nextProps.user.firstName && - prevProps.user.lastName === nextProps.user.lastName && - prevProps.isSelected === nextProps.isSelected && - prevProps.onEdit === nextProps.onEdit - ); -}); +}, (p, n) => + p.user === n.user && // pointer compare (user object replaced on updates) + p.isSelected === n.isSelected && + p.onEdit === n.onEdit);or drop the custom comparer and rely on shallow compare if the parent already replaces the
userobject when it mutates.copy-cursor-rules.sh (3)
61-66: Avoid masking subshell exit status; declare variables separately
Assigningcurrent_dirin the same line where the subshell is executed (local current_dir="$(cd ... && pwd)") hides a possiblecdfailure and triggers SC2155. Split the declaration from the assignment.- local current_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + local current_dir + current_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
189-198: Duplicatemobile-apppattern in case block
mobile-app|mobile-app|…contains the same token twice, producing ShellCheck SC2221/SC2222. Remove the duplicate to keep the pattern list intentional.- 3|mobile-app|mobile-app|Mobile-app|Mobile-app|MOBILE-APP|MOBILE-APP) + 3|mobile-app|Mobile-app|MOBILE-APP)
360-387: Consider normalising framework input once instead of huge pattern lists
Long, repetitive case patterns make maintenance hard and trigger multiple SC2221 warnings. Lower-case the input and replace underscores with dashes before thecase, then keep a single canonical token per framework.6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc (1)
118-130: Access tokens stored in plainAsyncStorageare vulnerable
AsyncStorageis unencrypted; malicious apps, backups or rooted devices can read it. Recommend Keychain/SecureStore or an encrypted wrapper.6-mobile-app/javascript/react-native/react-native-1-architecture-rules.mdc (1)
135-147: DuplicateButtonexample may confuse readers
AButtoncomponent is already defined earlier (lines 37-54). Redefining it here with different props could mislead implementers. Consider renaming the atom or referencing the initial definition.6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (5)
2-2: Fix typo in front-matter description.
intergrating→integrating.-description: USE WHEN intergrating APIs +description: USE WHEN integrating APIs
16-30: ClarifyApiResponseshape & optional fields.
statusduplicates HTTP status and inflates every in-memory payload, andmessageis not always present. Consider:-interface ApiResponse<T> { - data: T; - status: number; - message?: string; -} +interface ApiResponse<T> { + data: T; + message?: string; // omit if you never rely on it +}
50-62: Pagination default should be 1-based or explicit.Most APIs start pages at
1; starting at0silently creates an off-by-one gap. At minimum, add a comment or make the default1.
89-102: PATCH helper duplicatescreateUserlogic—extract DRY error handler.Both methods share identical fetch/error/parse boilerplate. A tiny generic helper improves maintainability.
-const apiResponse: ApiResponse<User> = await response.json(); -return apiResponse.data; +return handleJson<User>(response);(where
handleJsonwraps the pattern above)
142-145: Redundant cache set followed by invalidate.You
setQueryDatathen immediatelyinvalidateQueries(['users']), nullifying the optimistic benefit. Drop the explicitinvalidateunless stale persistence is required.- queryClient.setQueryData(['users'], (oldUsers: User[] | undefined) => { - return oldUsers ? [...oldUsers, newUser] : [newUser]; - }); - queryClient.invalidateQueries(['users']); + queryClient.setQueryData(['users'], (old: User[] | undefined) => + old ? [...old, newUser] : [newUser], + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (18)
2-common/common-mobile-app.md(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-1-architecture-rules.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-2-ui-patterns-rules.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-3-data-state-rules.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-4-api-integration-service-layer.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-5-performance-rules.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-6-security-rules.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-7-accessibility-rules.mdc(1 hunks)6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-8-testing-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-1-architecture-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-2-ui-patterns-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-3-data-state-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-5-performance-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-7-accessibility-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc(1 hunks)copy-cursor-rules.sh(12 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
copy-cursor-rules.sh
[warning] 61-61: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 192-192: This pattern always overrides a later one on line 192.
(SC2221)
[warning] 192-192: This pattern never matches because of a previous pattern on line 192.
(SC2222)
[warning] 192-192: This pattern always overrides a later one on line 192.
(SC2221)
[warning] 192-192: This pattern never matches because of a previous pattern on line 192.
(SC2222)
[warning] 192-192: This pattern always overrides a later one on line 192.
(SC2221)
[warning] 192-192: This pattern never matches because of a previous pattern on line 192.
(SC2222)
🔇 Additional comments (4)
6-mobile-app/javascript/hybrid-frontend/hybrid-frontend-5-performance-rules.mdc (1)
1-107: Comprehensive & coherent – no blocking issues spottedContent aligns with established best practices; reads well and complements the RN ruleset.
copy-cursor-rules.sh (1)
657-667: Hybrid-frontend rules copied for Ionic unintentionally
The guard includes"$framework" == "ionic"which results in React-Native-specific hybrid rules being copied into Ionic projects. Verify Ionic actually re-uses those rules; otherwise restrict the check toreact-nativeonly.6-mobile-app/javascript/react-native/react-native-1-architecture-rules.mdc (1)
268-291:AbortControllerrequires RN ≥ 0.64 or a polyfill
Older RN versions running on JSC lackAbortController. Add a compatibility note or import a polyfill to keep the example production-ready.6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (1)
265-274:getNextPageParamlogic mis-aligns with 0-based pages.You return
allPages.lengthwhich equals the current page count, notpageParam + 1, causing duplicate fetch of page 0 on firstfetchNextPage(). Quick fix:-return lastPage.length === pageSize ? allPages.length : undefined; +return lastPage.length === pageSize ? allPages.length : undefined; // assumes 1-basedor compute
pageParam + 1when your API is 0-based.
6-mobile-app/javascript/react-native/react-native-2-ui-patterns-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-2-ui-patterns-rules.mdc
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-3-data-state-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-7-accessibility-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (8)
6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc (2)
18-25: Good: RN Testing Library imports are correct (no web-only userEvent).
120-121: Good: Using RN-specific renderHook import (hooks package deprecated).6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (3)
87-96: Good fix on non‑JSON error bodies.The safe JSON parse with fallback resolves the previous double‑throw/swallow issue.
40-44:Config.API_HOSTused but not imported. Add import and brief note.Add
react-native-configimport at the top of the TSX block and note its source.```tsx +import Config from 'react-native-config'; interface ApiResponse<T> {Add a sentence near this snippet: “Config.API_HOST is provided by react-native-config.” --- `205-207`: **Import `NetInfo` and remove unused handlers.** Without the import, the example won’t compile; the local `handleOnline/handleOffline` vars are unused. ```diff import { useEffect, useState, useCallback } from "react"; import { useQuery } from "@tanstack/react-query"; +import NetInfo from "@react-native-community/netinfo"; useEffect(() => { - const handleOnline = () => setIsOnline(true); - const handleOffline = () => setIsOnline(false); - const unsubscribe = NetInfo.addEventListener((state) => setIsOnline(state.isConnected ?? true) ); return unsubscribe; }, []);Also applies to: 234-242
6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc (3)
13-16: Good: RN does not render HTML; no DOM-based sanitizers imported.Resolves prior concern about DOMPurify import breaking Metro.
92-105: Good: Hermes/JSC note with fallback forURLusage.The explanatory comments and fallback parser address runtime crashes on older engines.
290-302: Missing imports foruseMemoanduseAuthinuseSecureApisnippet.Without imports, copy/paste fails at runtime.
+import { useMemo } from "react"; +import { useAuth } from "./useAuth"; // adjust path as applicable export const useSecureApi = () => { const { accessToken } = useAuth();
🧹 Nitpick comments (8)
6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc (4)
2-2: Polish the front-matter description (grammar/clarity).-description: USE WHEN writing test for ensuring testing and quality principles to build reliable, maintainable frontend applications with comprehensive test coverage and robust error handling +description: USE WHEN writing tests to ensure testing and quality principles for reliable, maintainable frontend apps with comprehensive coverage and robust error handling
74-76: Assert loading state clears after data loads.Add a quick regression check so examples don’t leave stale loaders:
await screen.findByText("John Doe"); + expect(screen.queryByText(/loading/i)).toBeNull();
100-107: Clarify accessibility requirement for alert role.
getByRole('alert')will only work if your component setsrole="alert"oraccessibilityRole="alert". Add a note to prevent copy-paste confusion. The role and query are supported in RNTL. (callstack.github.io)await waitFor(() => { - expect(screen.getByRole("alert")).toHaveTextContent( + // Ensure the error element has role/ accessibilityRole "alert" in the component under test. + expect(screen.getByRole("alert")).toHaveTextContent( /failed to load user/i ); });
223-229: Tighten console.error spy lifecycle (avoid type casts).-describe("ErrorBoundary", () => { - beforeEach(() => { - jest.spyOn(console, "error").mockImplementation(() => {}); - }); - - afterEach(() => { - (console.error as jest.Mock).mockRestore(); - }); +describe("ErrorBoundary", () => { + let consoleSpy: jest.SpyInstance; + beforeEach(() => { + consoleSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + afterEach(() => { + consoleSpy.mockRestore(); + });6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (3)
43-50: Harden error messages: include status code; avoid empty statusText.Improves diagnostics across all service methods.
-throw new Error(`Failed to fetch users: ${response.statusText}`); +throw new Error(`Failed to fetch users (${response.status}${response.statusText ? ' ' + response.statusText : ''}).`); -`Failed to fetch paginated users: ${response.statusText}` +`Failed to fetch paginated users (${response.status}${response.statusText ? ' ' + response.statusText : ''})` -`Failed to fetch user: ${response.statusText}` +`Failed to fetch user (${response.status}${response.statusText ? ' ' + response.statusText : ''})` -`Failed to update user: ${response.statusText}` +`Failed to update user (${response.status}${response.statusText ? ' ' + response.statusText : ''})`Also applies to: 61-69, 71-78, 102-115
164-166: Use TanStack Query v4 object signatures for invalidate/cancel.Aligns with current API and improves type hints.
- queryClient.invalidateQueries(["users"]); + queryClient.invalidateQueries({ queryKey: ["users"] }); - await queryClient.cancelQueries(["users", id]); + await queryClient.cancelQueries({ queryKey: ["users", id] }); - queryClient.invalidateQueries(["users", id]); - queryClient.invalidateQueries(["users"]); + queryClient.invalidateQueries({ queryKey: ["users", id] }); + queryClient.invalidateQueries({ queryKey: ["users"] });Also applies to: 176-177, 188-191
287-287: Export or remove unusedrequestDeduplicator.It’s declared but not used in examples.
-const requestDeduplicator = new RequestDeduplicator(); +export const requestDeduplicator = new RequestDeduplicator();If intentionally unused here, consider removing to avoid dead code in docs.
6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc (1)
106-113: Tighten protocol parse in JSC fallback.Use the capture group (without colon) to avoid edge-cases and normalize once.
- const protocolMatch = url.match(/^([^:]+):/); + const protocolMatch = url.match(/^([^:]+):/); if (!protocolMatch) return false; - const protocol = protocolMatch[0].toLowerCase(); - return ["http:", "https:", "mailto:"].includes(protocol); + const proto = `${protocolMatch[1].toLowerCase()}:`; + return ["http:", "https:", "mailto:"].includes(proto);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (6)
6-mobile-app/javascript/react-native/react-native-2-ui-patterns-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-3-data-state-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-7-accessibility-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- 6-mobile-app/javascript/react-native/react-native-2-ui-patterns-rules.mdc
- 6-mobile-app/javascript/react-native/react-native-7-accessibility-rules.mdc
- 6-mobile-app/javascript/react-native/react-native-3-data-state-rules.mdc
🔇 Additional comments (1)
6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc (1)
224-243: Rate-limit hook looks good.Clear windowing logic and memoization.
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc
Outdated
Show resolved
Hide resolved
6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (4)
18-25: Add the missing Config import to make the snippet copy‑pasteable.Without importing Config, the service example won’t run as-is.
```tsx +import Config from 'react-native-config'; + interface ApiResponse<T> {Also add a one-liner in the doc that API_HOST is provided via react-native-config (or your env system).
40-45: Add a fetchWithTimeout helper and use it to actually cancel slow requests.Promise.race doesn’t abort RN fetches; AbortController does. Provide a default timeout and reuse everywhere.
interface CreateUserRequest { email: string; firstName: string; lastName: string; password: string; } +const fetchWithTimeout = ( + input: RequestInfo | URL, + init: RequestInit = {}, + timeoutMs = 15000 +) => { + const controller = new AbortController(); + const id = setTimeout(() => controller.abort(), timeoutMs); + const signal = init.signal ?? controller.signal; + return fetch(input, { ...init, signal }).finally(() => clearTimeout(id)); +}; + class UserService { private baseURL = `${Config.API_HOST}/users`;And replace bare fetch calls with fetchWithTimeout (e.g., in getUsers):
- const response = await fetch(this.baseURL); + const response = await fetchWithTimeout(this.baseURL);(Apply similarly in other methods.)
205-208: Import NetInfo for the network-aware hook.Missing import breaks the snippet.
import { useEffect, useState, useCallback } from "react"; import { useQuery } from "@tanstack/react-query"; +import NetInfo from "@react-native-community/netinfo";
276-281: Fix accidental diff markers in code block.Leading “+” chars make the snippet invalid when copied.
- const timeout = new Promise<never>((_, reject) => -+ setTimeout(() => reject(new Error(`Request timed out after ${timeoutMs}ms`)), timeoutMs) -+ ); -+ const request = Promise.race([requestFn(), timeout]) -+ .finally(() => this.pendingRequests.delete(key)) as Promise<T>; + const timeout = new Promise<never>((_, reject) => + setTimeout(() => reject(new Error(`Request timed out after ${timeoutMs}ms`)), timeoutMs) + ); + const request = Promise.race([requestFn(), timeout]) + .finally(() => this.pendingRequests.delete(key)) as Promise<T>;Also note: Promise.race doesn’t cancel the underlying fetch on RN—pair with AbortController/timeouts.
🧹 Nitpick comments (3)
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (3)
154-167: Avoid redundant refetch right after local cache insert.You already merge the new user into ["users"]; invalidating immediately refetches and can cause flicker/duplication. Either keep setQueryData OR invalidate; not both.
queryClient.setQueryData(["users"], (oldUsers: User[] | undefined) => { return oldUsers ? [...oldUsers, newUser] : [newUser]; }); - queryClient.invalidateQueries(["users"]);(If server is source of truth, drop setQueryData and keep invalidate instead.)
234-236: Remove unused handlers.handleOnline/handleOffline are declared but never used.
- const handleOnline = () => setIsOnline(true); - const handleOffline = () => setIsOnline(false);
268-285: Consider type safety and eviction in RequestDeduplicator.Map<string, Promise> can mix types under the same key across call sites; add a convention that keys include the resource type or store a typed wrapper. Also consider a max size/LRU to prevent unbounded growth under churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (3)
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc(1 hunks)6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- 6-mobile-app/javascript/react-native/react-native-6-security-rules.mdc
- 6-mobile-app/javascript/react-native/react-native-8-testing-rules.mdc
🔇 Additional comments (1)
6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc (1)
118-119: Nice: centralizing service instance for hooks.Exported userService keeps the examples tidy and consistent.
| const response = await fetch(`${this.baseURL}?${params}`); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Failed to fetch paginated users: ${response.statusText}` | ||
| ); | ||
| } | ||
| const apiResponse: ApiResponse<User[]> = await response.json(); | ||
| return apiResponse.data; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm URLSearchParams availability on your RN targets or add a safe fallback.
Older RN apps/environments may lack URLSearchParams without polyfills. Verify your minimum RN version or add a tiny helper to build the query string.
🌐 Web query:
Does React Native (0.72 and later) include a global URLSearchParams implementation by default on Hermes/JSC?
💡 Result:
Short answer: No. React Native (including 0.72+) does not reliably provide a full global URLSearchParams implementation on Hermes or JSC — the built-in URL support is lightweight/incomplete and many apps use react-native-url-polyfill (or similar) to get URLSearchParams. [1][2][3]
Sources:
- react-native-url-polyfill README (notes React Native's URL is incomplete; recommends the polyfill). [1]
- Hermes issue discussing lack of full URL/Web API support and recommending polyfills. [2]
- npm / package docs for react-native-url-polyfill (installation and usage). [3]
Add URLSearchParams polyfill or manual query builder
React Native’s built-in URL support is minimal and does not reliably include URLSearchParams on Hermes/JSC (RN 0.72+) (github.com). Import a polyfill (e.g., react-native-url-polyfill or react-native-fast-url) in your app’s entry file or replace new URLSearchParams() with a small helper to build the ?key=value… string before lines 61–69.
🤖 Prompt for AI Agents
In 6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc
around lines 61–69, the code relies on URLSearchParams which is not guaranteed
in React Native (Hermes/JSC); either import a URLSearchParams polyfill at the
app entry (e.g., add `import 'react-native-url-polyfill/auto'` or similar to
index.js/App.tsx) or replace the URLSearchParams usage with a small helper that
URL-encodes keys and values and concatenates them into a query string
(encodeURIComponent for keys/values, join with & and prefix with ?), then use
that string in the fetch call so the request works consistently on RN runtimes.
| if (!response.ok) { | ||
| throw new Error(`Failed to update user: ${response.statusText}`); | ||
| } | ||
|
|
||
| const apiResponse: ApiResponse<User> = await response.json(); | ||
| return apiResponse.data; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify error parsing with createUser; preserve server message for updateUser.
Mirror the defensive JSON parsing used in createUser so non‑JSON bodies don’t mask the failure reason.
- if (!response.ok) {
- throw new Error(`Failed to update user: ${response.statusText}`);
- }
+ if (!response.ok) {
+ let detail = 'Failed to update user';
+ try {
+ const body = await response.json();
+ detail = body?.message ?? detail;
+ } catch {}
+ throw new Error(detail);
+ }📝 Committable suggestion
‼️ 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.
| if (!response.ok) { | |
| throw new Error(`Failed to update user: ${response.statusText}`); | |
| } | |
| const apiResponse: ApiResponse<User> = await response.json(); | |
| return apiResponse.data; | |
| } | |
| if (!response.ok) { | |
| let detail = 'Failed to update user'; | |
| try { | |
| const body = await response.json(); | |
| detail = body?.message ?? detail; | |
| } catch {} | |
| throw new Error(detail); | |
| } | |
| const apiResponse: ApiResponse<User> = await response.json(); | |
| return apiResponse.data; | |
| } |
🤖 Prompt for AI Agents
In 6-mobile-app/javascript/react-native/react-native-4-api-integration-rules.mdc
around lines 109 to 115, the updateUser branch throws a generic Error using
response.statusText and then parses JSON normally, which can lose
server-provided error details when the body is non-JSON; change to mirror
createUser’s defensive parsing: when response.ok is false, read the raw response
body (e.g., response.text()), attempt to JSON.parse it to extract a server
message field, and if parsing fails fall back to the raw text or
response.statusText, then throw a new Error that includes the HTTP status plus
the preserved server message so callers get the original failure reason.
1 participant
No description provided.