Native integration of code viewing and submission, plus improved server connection handling #2
Conversation
📝 WalkthroughWalkthroughThis pull request introduces live checkout functionality with comprehensive session and code submission management, adds connection monitoring and error handling UI components, implements dynamic API URL configuration, updates Expo/React Native dependencies to v54/v19, and adds numerous supporting hooks, utilities, and type definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as CheckoutUI
participant Hook as useLiveClasses
participant Submit as submitCode
participant WS as WebSocket
participant HTTP as HTTP Fallback
participant AutoSplash as AutoSplash Service
participant API as API Server
User->>UI: Enter code & submit
UI->>Hook: submitCode(submissionData)
Hook->>Submit: submitCode(data)
Submit->>Submit: prepareSubmission()
Submit->>API: POST /api/submit/options
API-->>Submit: SubmitOptionsResponse
alt WebSocket Available
Submit->>WS: Connect to /api/submit/code
Submit->>WS: Send auth token
Submit->>WS: Send submission data
Submit->>WS: Send user agent
Submit->>WS: Send JWT
WS-->>Submit: SubmissionResult + jwt
Submit->>AutoSplash: connectAutoSplash(jwt)
AutoSplash->>API: Connect to /api/submit/autosplash
API-->>AutoSplash: AutoSplash poll updates
AutoSplash-->>Submit: Result (success/error)
else WebSocket Fails
Submit->>HTTP: POST /api/submit/code
HTTP-->>Submit: SubmissionResult
end
Submit-->>Hook: { result, showAutoSplash, jwt }
Hook->>UI: Update submission state & UI
UI-->>User: Show success/error + optional AutoSplash modal
sequenceDiagram
participant App as App/Tab Layout
participant Monitor as useConnectionMonitor
participant API as /api/app/state
participant Context as ConnectionContext
participant UI as Error UI Components
App->>Monitor: Start polling (5s interval)
loop Every 5 seconds
Monitor->>API: GET /api/app/state
alt Success (2xx/304)
API-->>Monitor: { appState: { webState, message, link } }
alt webState === 0 (Unavailable)
Monitor->>Context: setUnavailable(message, link)
else Normal
Monitor->>Context: clearConnectionError()
end
else Network/Server Error
API-->>Monitor: Error
Monitor->>Context: setConnectionError()
end
end
Context-->>UI: Notify listeners (banner, overlay, etc.)
UI-->>App: Render error state with retry UI
User->>UI: Tap retry or wait for recovery
Monitor->>Context: Clear error on next success
UI-->>App: Dismiss error UI & show "Back online"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (14)
app/(tabs)/_layout.tsx (1)
32-32: Type safety: Avoidanycast for route params.The cast
(route.params as any)bypasses type safety. Consider defining a proper type for route params or using optional chaining with a default value:- const currentParams = (route.params as any) || {}; + const currentParams = route.params || {};Or define a proper type:
type TabRouteParams = { refresh?: number; [key: string]: any; }; const currentParams = (route.params as TabRouteParams) || {};hooks/useApiUrl.ts (1)
26-29: Enhance URL validation for better error handling.The current validation only checks for protocol presence. Consider validating the URL structure more thoroughly to catch malformed URLs early:
// Validate URL format (basic check) - if (newUrl && !newUrl.startsWith('http://') && !newUrl.startsWith('https://')) { - throw new Error('URL must start with http:// or https://'); + if (!newUrl || newUrl.trim() === '') { + throw new Error('URL cannot be empty'); + } + + try { + const url = new URL(newUrl); + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + throw new Error('URL must use HTTP or HTTPS protocol'); + } + } catch (error) { + if (error instanceof TypeError) { + throw new Error('Invalid URL format'); + } + throw error; }This uses the native
URLconstructor to validate the entire URL structure, catching issues like missing domains, invalid characters, etc.hooks/useAuth.ts (1)
73-80: Dynamic API URL usage in auth flow looks goodUsing
getApiUrl()to build the auth URL keeps sign-in aligned with the configured backend, and the web/native branches both use the same base URL. Just ensure any custom API URL stored by the user does not already include the/apploginpath to avoid surprises.Also applies to: 96-99
components/ConnectionErrorBanner.tsx (1)
19-23: Keep displayed API domain in sync with runtime configurationThe domain is derived once on mount via
getApiUrl()in an effect with an empty dependency array, so if the user changes the configured API URL while the app is running, the banner will continue to show the old domain.If you expect runtime changes to the API endpoint, consider subscribing to whatever source of truth already tracks it (e.g., a
useApiUrlhook or connection context field) instead of a one-timegetApiUrl()call, soapiDomainstays accurate.Also applies to: 27-40
utils/api.ts (2)
46-48: Clarify expectations around JSON responses and empty bodiesThis implementation assumes all successful responses have a JSON body. If an endpoint legitimately returns a 2xx with an empty body (e.g., HTTP 204),
JSON.parse(responseText)will throw and the call will be reported assuccess: falsewith an “Invalid response format” error.If such endpoints exist (or may in future), consider handling them explicitly before parsing, for example:
if (!responseText) { return { success: true, data: undefined as any, status: response.status }; }or by checking the status code/content-type before parsing.
Also applies to: 60-64, 99-122
69-89: Deduplicate connection error propagation logicThe “invalid JSON” branches for both non-OK and OK responses, plus the outer
catch, all dynamically importConnectionContextand callsetConnectionErrorin very similar ways.To reduce bundle size and keep behavior consistent, consider extracting a small helper within this module:
async function notifyConnectionError(message: string) { try { const { getConnectionContext } = await import('@/contexts/ConnectionContext'); const context = getConnectionContext(); context?.setConnectionError(message); } catch { // Context not available; ignore } }and reuse it in the three places where you currently inline that logic.
Also applies to: 99-122, 129-142
components/ConnectionErrorOverlay.tsx (1)
23-26: MakeshowHelpLinksexplicitly boolean
const showHelpLinks = disconnectedAt && Date.now() - disconnectedAt > 5000;can yield either anumber(ifdisconnectedAtis falsy like0) or a boolean. It works in JSX, but the type is a bit loose.You can make intent and types clearer with:
const showHelpLinks = !!disconnectedAt && Date.now() - disconnectedAt > 5000;This keeps
showHelpLinksstrictly boolean.constants/api.ts (1)
19-47: Custom URL persistence and backwards‑compat constant are well handled
setCustomApiUrl/resetApiUrlcorrectly keep storage in sync and avoid storing a redundant value when the URL equals the default, and keepingCHECKOUT_API_URLas a synchronous alias toDEFAULT_API_URLis a reasonable backward‑compat shim while new code migrates togetApiUrl.You may later consider centralizing the default URL (e.g. importing
DEFAULT_CHECKOUT_API_URLwherever needed) to avoid hard‑coded copies across files.app/(tabs)/index.tsx (2)
48-148: Auto‑refresh + connection‑aware error handling are generally well structuredThe separation between
fetchData(connection‑aware error setting) andhandleFetchData(loading/refresh state) is clean, and the various triggers (mount, focus, tab param, interval) correctly reuse the same path. The 2‑second interval plus focus‑based refresh is quite aggressive, though; if this hits a non‑trivial endpoint in production you may want to lengthen the interval or pause polling when the screen is backgrounded or when the user is interacting with content.
154-212: Minor copy tweak: list formatting can produce “A, and B”
formatNextSessionyields “A, B, and C” for 3+ sessions, which is nice, but for exactly 2 sessions it generates “A, and B” (extra comma). You can special‑case the 2‑item case to produce “A and B” while keeping the current behavior for 3+ items.- const activityReferences = nextSessions - .map((s, index) => { - let refText = `${s.activityReference}`; - return index === nextSessions.length - 1 && nextSessions.length > 1 - ? `and ${refText}` - : refText; - }) - .join(', '); + let activityReferences: string; + if (nextSessions.length === 1) { + activityReferences = nextSessions[0].activityReference; + } else if (nextSessions.length === 2) { + activityReferences = `${nextSessions[0].activityReference} and ${nextSessions[1].activityReference}`; + } else { + activityReferences = nextSessions + .map((s, index) => + index === nextSessions.length - 1 ? `and ${s.activityReference}` : s.activityReference + ) + .join(', '); + }hooks/useConnectionMonitor.ts (1)
60-66: Avoid state updates from thesetTimeoutafter unmount
setTimeout(() => setWasDisconnected(false), 100);can fire after the hook’s owning component unmounts, causing a “setState on unmounted component” warning. Since you already track mount state, you can guard or store/clear the timeout id on cleanup.- const [wasDisconnected, setWasDisconnected] = useState(false); + const [wasDisconnected, setWasDisconnected] = useState(false); + const wasDisconnectedTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); @@ - if (wasDisconnectedBefore) { - setWasDisconnected(true); - // Reset after a short delay - setTimeout(() => setWasDisconnected(false), 100); - } + if (wasDisconnectedBefore) { + setWasDisconnected(true); + // Reset after a short delay + if (wasDisconnectedTimeoutRef.current) { + clearTimeout(wasDisconnectedTimeoutRef.current); + } + wasDisconnectedTimeoutRef.current = setTimeout(() => { + if (isMountedRef.current) { + setWasDisconnected(false); + } + }, 100); + } @@ - return () => { - isMountedRef.current = false; - if (checkIntervalRef.current) { - clearInterval(checkIntervalRef.current); - } - }; + return () => { + isMountedRef.current = false; + if (checkIntervalRef.current) { + clearInterval(checkIntervalRef.current); + } + if (wasDisconnectedTimeoutRef.current) { + clearTimeout(wasDisconnectedTimeoutRef.current); + } + };app/(tabs)/settings.tsx (1)
40-51: Resolved API URL loading and link wiring look consistentLoading the resolved API URL on focus via
getApiUrl()and usingcurrentResolvedApiUrlas the base for all outbound links (account, data, privacy/terms, help) keeps the UI in sync with storage and the new URL configuration hook.You could simplify by having
useApiUrlalso expose the resolved URL (or by using itsdefaultUrlhelper) so this component doesn’t need to know aboutgetApiUrldirectly.package.json (2)
19-51: Reconsider caret versioning on critical packages.Multiple dependencies use caret (
^) versioning, which permits breaking changes within major version ranges. expo@^54.0.10 and related packages should align closely with Expo ecosystem versions for best compatibility. Consider pinning critical packages more strictly (e.g.,~X.Y.Zor exact versions) to reduce the risk of unexpected runtime breakage, especially for:
@expo/vector-icons: ^15.0.2@react-native-cookies/cookies: ^6.2.1react-native-web: ^0.21.0react-native-worklets-core: ^1.6.2This is particularly important during major platform upgrades.
Apply this diff to tighten versioning on critical packages:
- "@expo/vector-icons": "^15.0.2", + "@expo/vector-icons": "~15.0.2", - "@react-native-cookies/cookies": "^6.2.1", + "@react-native-cookies/cookies": "~6.2.1", - "react-native-web": "^0.21.0", + "react-native-web": "~0.21.0", - "react-native-worklets-core": "^1.6.2" + "react-native-worklets-core": "~1.6.2"
19-51: Tighten versioning strategy on critical dependencies.You're mixing caret (
^) and tilde (~) versioning across critical packages during a major platform upgrade. Caret versioning permits breaking changes within major versions. Expo 54 has documented compatible versions for all ecosystem packages (e.g., expo-router ~6.0.8, expo-splash-screen ~31.0.10, expo-web-browser ~15.0.7).Consider pinning these dependencies more strictly to reduce unexpected breakage:
@expo/vector-icons: ^15.0.2→ prefer~15.0.2@react-native-cookies/cookies: ^6.2.1→ prefer~6.2.1react-native-web: ^0.21.0→ prefer~0.21.0react-native-worklets-core: ^1.6.2→ prefer~1.6.2Apply this diff:
- "@expo/vector-icons": "^15.0.2", + "@expo/vector-icons": "~15.0.2", - "@react-native-cookies/cookies": "^6.2.1", + "@react-native-cookies/cookies": "~6.2.1", - "react-native-web": "^0.21.0", + "react-native-web": "~0.21.0", - "react-native-worklets-core": "^1.6.2" + "react-native-worklets-core": "~1.6.2"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
app.json(1 hunks)app/(tabs)/_layout.tsx(4 hunks)app/(tabs)/index.tsx(1 hunks)app/(tabs)/settings.tsx(6 hunks)app/_layout.tsx(2 hunks)babel.config.js(1 hunks)components/ConnectionErrorBanner.tsx(1 hunks)components/ConnectionErrorOverlay.tsx(1 hunks)components/TabScreenScrollView.tsx(1 hunks)components/UnavailableOverlay.tsx(1 hunks)components/ui/TabBarBackground.ios.tsx(1 hunks)constants/api.ts(1 hunks)contexts/ConnectionContext.tsx(1 hunks)hooks/useApiUrl.ts(1 hunks)hooks/useAuth.ts(7 hunks)hooks/useConnectionMonitor.ts(1 hunks)hooks/useHistory.ts(1 hunks)package.json(1 hunks)utils/api.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
components/ui/TabBarBackground.ios.tsx (1)
hooks/useColorScheme.web.ts (1)
useColorScheme(7-21)
components/TabScreenScrollView.tsx (1)
contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)
utils/api.ts (2)
constants/api.ts (1)
getApiUrl(8-16)contexts/ConnectionContext.tsx (1)
getConnectionContext(31-33)
components/ConnectionErrorBanner.tsx (4)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)constants/api.ts (1)
getApiUrl(8-16)components/ThemedText.tsx (1)
ThemedText(11-34)
hooks/useApiUrl.ts (1)
constants/api.ts (4)
DEFAULT_CHECKOUT_API_URL(47-47)getApiUrl(8-16)setCustomApiUrl(19-31)resetApiUrl(34-41)
components/UnavailableOverlay.tsx (4)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)constants/Colors.ts (1)
Colors(34-83)components/ThemedText.tsx (1)
ThemedText(11-34)
app/_layout.tsx (2)
constants/Colors.ts (1)
Colors(34-83)contexts/ConnectionContext.tsx (1)
ConnectionProvider(35-121)
hooks/useAuth.ts (1)
constants/api.ts (1)
getApiUrl(8-16)
app/(tabs)/settings.tsx (3)
hooks/useSettingsAccount.ts (1)
useSettingsAccount(12-88)hooks/useApiUrl.ts (1)
useApiUrl(4-69)constants/api.ts (1)
getApiUrl(8-16)
app/(tabs)/_layout.tsx (3)
hooks/useConnectionMonitor.ts (1)
useConnectionMonitor(11-112)components/ConnectionErrorBanner.tsx (1)
ConnectionErrorBanner(12-99)components/UnavailableOverlay.tsx (1)
UnavailableOverlay(8-163)
hooks/useConnectionMonitor.ts (2)
contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)utils/api.ts (1)
checkoutApi(20-150)
components/ConnectionErrorOverlay.tsx (4)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)constants/Colors.ts (1)
Colors(34-83)components/ThemedText.tsx (1)
ThemedText(11-34)
app/(tabs)/index.tsx (3)
contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)utils/api.ts (1)
checkoutApi(20-150)components/TabScreenScrollView.tsx (1)
TabScreenScrollView(12-35)
constants/api.ts (1)
utils/storage.ts (1)
storage(33-36)
🔇 Additional comments (19)
hooks/useHistory.ts (1)
127-129: LGTM! Silent refresh improves UX.The change from
fetchHistory(true, true)tofetchHistory(false, true)removes the toast feedback on manual refresh while preserving the reset behavior. This aligns well with the connection-aware auto-refresh pattern introduced in this PR, where silent refreshes are more appropriate.app.json (1)
5-5: LGTM! Version bump is appropriate.The version increment to 1.1.9 correctly reflects the new features added in this PR (connection monitoring, API URL management, and UI improvements).
babel.config.js (1)
1-7: LGTM! Standard Expo Babel configuration.The configuration correctly uses
babel-preset-expoand includes thereact-native-reanimated/pluginas the last plugin, which is required for proper Reanimated functionality.components/TabScreenScrollView.tsx (2)
20-23: LGTM! Banner visibility logic is correct.The banner visibility check correctly filters for
'connection'and'api_error'types, excluding'unavailable'which has its own overlay. This aligns with theConnectionErrorBannercomponent behavior shown in the relevant code snippets.
9-10: BANNER_HEIGHT constant is correct; no issues found.Verification confirms the calculation is accurate. The banner's
paddingVertical: 12yields 24px total padding, and the estimated ~28px content height aligns with the actual component structure (two text lines + retry icon). The 52px constant correctly reflects the rendered banner dimensions, and using "≈" in the comment is appropriate given the content height estimation.app/_layout.tsx (2)
35-48: LGTM! Theme integration is well-structured.The
navThemeobject correctly extends the base React Navigation theme with custom colors from theColorsmodule, ensuring consistent theming across the app. The fallback to'light'whencolorSchemeis null is appropriate.
51-60: LGTM! Provider wrapping is correct.The
ConnectionProviderwrapping enables connection monitoring throughout the app, and theThemeProviderreceives the properly constructednavTheme. The structure is sound.app/(tabs)/_layout.tsx (2)
97-98: LGTM! Connection UI components properly integrated.The
ConnectionErrorBannerandUnavailableOverlayare correctly rendered at the layout level, ensuring they're visible across all tabs when connection issues occur.
27-39: No issues found — refresh parameter is properly consumed by the index tab.The verification confirms that
app/(tabs)/index.tsxcorrectly extracts and monitors the refresh parameter. At line 111, it's typed as{ refresh?: number } | undefined, and at line 119, the refresh value is included in the useEffect dependency array, ensuring that when_layout.tsxsetsrefresh: Date.now(), the effect re-runs to trigger data refresh.hooks/useApiUrl.ts (3)
31-32: LGTM! Trailing slash removal is correct.The regex
/\/+$/correctly removes one or more trailing slashes, ensuring consistent URL formatting.
44-56: LGTM! Reset logic is sound.The
resetToDefaultfunction correctly clears custom storage and reverts to the default URL with proper error handling and loading state management.
58-68: LGTM! Hook API is clean and comprehensive.The returned object provides all necessary functionality: current URL, update/reset operations, loading state, default checking, and the default URL constant. Well-designed API.
components/ui/TabBarBackground.ios.tsx (1)
13-24: BlurView tint values are supported — no issues found.expo-blur's BlurView accepts both "systemUltraThinMaterialLight" and "systemUltraThinMaterialDark" as tint values. The code implementation at lines 13-24 is correct and compatible with expo-blur.
constants/api.ts (1)
7-16: getApiUrl storage + fallback behavior looks solidThe async getter correctly wraps storage access in a try/catch and always returns a usable URL (custom when present, otherwise the default) without leaking storage failures to callers. This is a good, defensive boundary for the rest of the app.
app/(tabs)/settings.tsx (1)
170-236: Dynamic account/data/help links correctly respect the configured API serverUsing
currentResolvedApiUrlto build${currentResolvedApiUrl}/account,/data,/terms-privacy, and/learn-faqensures all external links follow the custom API base instead of a hard‑coded host, which aligns with the new configuration flow.contexts/ConnectionContext.tsx (1)
35-121: Connection state model and global accessor pattern are coherentThe provider cleanly centralizes connection state (type, visibility, timestamps, messages) with small, focused setters, and the
setIsCheckingoptimization avoids unnecessary re‑renders. ExposinggetConnectionContext()backed byglobalConnectionContextgives non‑React code (likeutils/api) a safe way to signal errors while still allowinguseConnectionContext()to enforce proper usage inside components.package.json (3)
39-40: Verify React and react-test-renderer versions are aligned across the ecosystem.You're upgrading to React 19.1.0, react-dom 19.1.0, and react-test-renderer 19.1.0. Ensure that
@types/react(~19.1.10) and all other type definitions are compatible with this React version. Since this is a major version upgrade with breaking changes in React 19, verify that the codebase has been updated to handle:
- Removal of
propTypesanddefaultPropson function componentsrefas a regular prop (noforwardRefneeded)- Ref callback return value restrictions
Review the related code changes in the PR to confirm all necessary updates have been made.
Also applies to: 57-57, 64-65
39-40: Verify React 19 breaking changes are handled in source code.React 19.1.0 introduces breaking changes (removed
propTypes/defaultPropson function components,refas regular prop, strict ref callback returns). Ensure the codebase has been updated accordingly. Cross-check with the related code changes in the PR (home screen rewrite, settings updates, connection monitoring) to confirm:
- No function components use
propTypesordefaultPropsforwardRefhas been replaced withrefas a prop (if applicable)- Ref callbacks don't return values
Also applies to: 57-57, 64-65
22-38: Incorrect claim: react-native-reanimated was not upgraded in this PR.Git diff confirms no changes to
react-native-reanimated. The version remains at ~4.1.0, which was already in the codebase. The review comment incorrectly attributes an upgrade to this PR. The actual changes are limited to Expo ecosystem packages (lines 22-38), which have been verified for peer dependency compatibility vianpm installwith no warnings detected.Likely an incorrect or invalid review comment.
app/(tabs)/index.tsx
Outdated
| if (loading && !isRefreshing) { | ||
| return ( | ||
| <ThemedView style={styles.container}> | ||
| <ActivityIndicator size="large" /> | ||
| </ThemedView> | ||
| ); | ||
| } | ||
|
|
||
| if (error) { | ||
| return ( | ||
| <ThemedView style={styles.container}> | ||
| <ThemedText>Error: {error}</ThemedText> | ||
| </ThemedView> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <ThemedView style={styles.container}> | ||
| <ThemedText style={styles.title}>Code Management</ThemedText> | ||
| <ThemedText style={styles.text}>Enter and view your check-in codes here</ThemedText> | ||
| </ThemedView> | ||
| <TabScreenScrollView | ||
| refreshControl={<RefreshControl refreshing={isRefreshing} onRefresh={onRefresh} />} | ||
| contentContainerStyle={styles.scrollContentContainer}> | ||
| <ThemedView style={styles.container}> | ||
| {data && data.sessionCount === 0 ? ( | ||
| <> | ||
| <ThemedText style={styles.emoji}>😊</ThemedText> | ||
| <ThemedText style={styles.text}>No classes!</ThemedText> | ||
| {data.nextSessions && data.nextSessions.length > 0 && ( | ||
| <ThemedText style={styles.nextSessionText}> | ||
| {formatNextSession(data.nextSessions)} | ||
| </ThemedText> | ||
| )} | ||
| </> | ||
| ) : ( | ||
| <ThemedText style={styles.text}> | ||
| There are {data?.sessionCount} classes in session. | ||
| </ThemedText> | ||
| )} |
There was a problem hiding this comment.
Handle data === null to avoid “undefined classes in session”
When checkoutApi fails but the connection overlay is visible, fetchData clears the local error and leaves data as null. In that state loading is false and error is null, so the component falls through to the final render branch and shows:
There are {data?.sessionCount} classes in session.which renders as “There are undefined classes in session.”
You should add an explicit branch for !data (or default sessionCount to 0 with different copy) so the UI never surfaces undefined to users.
- if (error) {
+ if (error) {
return (
<ThemedView style={styles.container}>
<ThemedText>Error: {error}</ThemedText>
</ThemedView>
);
}
- return (
+ if (!data) {
+ return (
+ <ThemedView style={styles.container}>
+ <ThemedText style={styles.text}>Unable to load classes right now.</ThemedText>
+ </ThemedView>
+ );
+ }
+
+ return (
<TabScreenScrollView
@@
- {data && data.sessionCount === 0 ? (
+ {data.sessionCount === 0 ? (
@@
- There are {data?.sessionCount} classes in session.
+ There are {data.sessionCount} classes in session.🤖 Prompt for AI Agents
In app/(tabs)/index.tsx around lines 214–249, the final render branch can
display "undefined" because data may be null after a failed fetch; add an
explicit check for !data (or normalize sessionCount to 0) before rendering the
"There are ... classes in session" text. Modify the JSX to return a focused UI
for the no-data state (e.g., show "No classes!" or a loading/error-appropriate
message) when data is null, or compute const sessionCount = data?.sessionCount
?? 0 and use conditional copy when sessionCount === 0 so the UI never renders
undefined.
| const handleResetApiUrl = async () => { | ||
| try { | ||
| await resetToDefault(); | ||
| const newResolvedUrl = await getApiUrl(); | ||
| setCurrentResolvedApiUrl(newResolvedUrl); | ||
| toast.success('API server reset to default'); | ||
| } catch (error) { | ||
| console.error('Error resetting API URL:', error); | ||
| toast.error('Failed to reset API server'); | ||
| } | ||
| }; | ||
|
|
||
| const showApiUrlConfig = () => { | ||
| Alert.prompt( | ||
| 'Configure API Server', | ||
| `Current: ${currentApiUrl}\n\nEnter a new API server URL:`, | ||
| [ | ||
| { | ||
| text: 'Cancel', | ||
| style: 'cancel', | ||
| }, | ||
| { | ||
| text: 'Use Default', | ||
| onPress: handleResetApiUrl, | ||
| }, | ||
| { | ||
| text: 'Save', | ||
| onPress: async (inputUrl?: string) => { | ||
| if (inputUrl && inputUrl.trim()) { | ||
| try { | ||
| await updateApiUrl(inputUrl.trim()); | ||
| const newResolvedUrl = await getApiUrl(); | ||
| setCurrentResolvedApiUrl(newResolvedUrl); | ||
| toast.success('API server updated successfully'); | ||
| } catch (error) { | ||
| console.error('Error updating API URL:', error); | ||
| toast.error(error instanceof Error ? error.message : 'Failed to update API server'); | ||
| } | ||
| } else { | ||
| toast.error('Please enter a valid URL'); | ||
| } | ||
| }, | ||
| }, | ||
| ], | ||
| 'plain-text', | ||
| currentApiUrl !== 'https://checkout.ac' ? currentApiUrl : '' | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Guard Alert.prompt for non‑iOS platforms and avoid hard‑coding the default URL
Using Alert.prompt is convenient, but in React Native it has historically only been supported on iOS; on Android it’s either unsupported or throws at runtime. Also, the default URL 'https://checkout.ac' is hard‑coded here instead of reusing the shared constant.
To make this safer and more maintainable:
- Wrap the prompt in a platform check (e.g., only call on iOS and fall back to a different UI or
Alert.alertelsewhere). - Import and use the shared default URL constant instead of duplicating the string.
-import { TouchableOpacity, Linking, View, Alert } from 'react-native';
+import { TouchableOpacity, Linking, View, Alert, Platform } from 'react-native';
@@
-import { getApiUrl } from '@/constants/api';
+import { getApiUrl, DEFAULT_CHECKOUT_API_URL } from '@/constants/api';
@@
- const showApiUrlConfig = () => {
- Alert.prompt(
+ const showApiUrlConfig = () => {
+ if (Platform.OS !== 'ios') {
+ Alert.alert(
+ 'Configure API Server',
+ `Current: ${currentApiUrl}\n\nAPI server configuration prompt is only available on iOS in this build.`
+ );
+ return;
+ }
+
+ Alert.prompt(
@@
- 'plain-text',
- currentApiUrl !== 'https://checkout.ac' ? currentApiUrl : ''
+ 'plain-text',
+ currentApiUrl !== DEFAULT_CHECKOUT_API_URL ? currentApiUrl : ''
);
};🌐 Web query:
Is `Alert.prompt` supported on Android in React Native 0.81, or is it still iOS‑only?
💡 Result:
Short answer: It's iOS-only. The React Native docs state Alert.prompt (the prompt-style alert) is available on iOS only — Android does not support Alert.prompt in RN 0.81. [1][2]
Sources:
- React Native Alert docs (notes that prompts are iOS only). [1]
- React Native 0.81 docs (AlertIOS removed; use Alert — but prompts remain iOS-only). [2]
🏁 Script executed:
#!/bin/bash
# Search for DEFAULT_CHECKOUT_API_URL constant definition
rg -n 'DEFAULT_CHECKOUT_API_URL' --type=ts --type=tsx -B2 -A2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check constants/api file for default URL constants
fd -e ts -e tsx 'api' | grep -i constant | head -20Length of output: 16
🏁 Script executed:
#!/bin/bash
# Look for any file with "constant" or "config" in name that might define API URLs
fd -e ts -e tsx | xargs grep -l 'checkout\.ac' | head -20Length of output: 115
🏁 Script executed:
#!/bin/bash
# View the constants/api.ts file to see what's defined
cat constants/api.tsLength of output: 1432
🏁 Script executed:
#!/bin/bash
# Check settings.tsx around the suspicious line (showing context)
sed -n '60,120p' 'app/(tabs)/settings.tsx'Length of output: 1776
🏁 Script executed:
#!/bin/bash
# Check imports and function definitions in settings.tsx
sed -n '1,50p' 'app/(tabs)/settings.tsx'Length of output: 2110
🏁 Script executed:
#!/bin/bash
# Search for resetToDefault function definition in settings.tsx
rg -n 'resetToDefault|resetApiUrl' 'app/(tabs)/settings.tsx' -A3 -B1Length of output: 512
Add platform check for Alert.prompt and import the default URL constant
Verification confirms both issues:
- React Native's
Alert.promptis available on iOS only — Android does not support it, making the current implementation break on Android devices. - The constant
DEFAULT_CHECKOUT_API_URLis already exported fromconstants/api.tsbut not imported or used insettings.tsx(line 114 hard-codes the string instead).
Apply the suggested changes to:
- Import
Platformfrom React Native andDEFAULT_CHECKOUT_API_URLfromconstants/api - Wrap the prompt in a platform check, providing a fallback
Alert.alertfor Android - Replace the hard-coded string with the constant
| return ( | ||
| <Animated.View | ||
| style={[ | ||
| styles.overlay, | ||
| { | ||
| backgroundColor: overlayColor, | ||
| opacity: fadeAnim, | ||
| }, | ||
| ]} | ||
| pointerEvents="box-none"> | ||
| <Animated.View |
There was a problem hiding this comment.
Overlay does not fully block background interaction
As with UnavailableOverlay, the outer Animated.View uses pointerEvents="box-none", meaning taps outside the inner card will pass through to underlying screens while the error overlay is visible.
If this overlay is meant to act as a blocking modal, consider removing the pointerEvents override or changing it to 'auto'/'box-only' so the full-screen overlay intercepts touches.
🤖 Prompt for AI Agents
In components/ConnectionErrorOverlay.tsx around lines 125 to 135, the outer
Animated.View currently sets pointerEvents="box-none" so touches outside the
inner card pass through; to make the overlay block background interaction change
or remove that prop and use pointerEvents="auto" or "box-only" (or simply omit
pointerEvents) so the full-screen overlay intercepts touches while visible,
keeping the inner view able to receive its own events.
| if (!shouldShow) { | ||
| return null; | ||
| } | ||
|
|
||
| const overlayColor = isDark ? 'rgba(239, 83, 80, 0.95)' : 'rgba(211, 47, 47, 0.95)'; | ||
| const cardBg = isDark ? Colors.dark.contentBackground : Colors.light.contentBackground; | ||
|
|
||
| return ( | ||
| <Animated.View | ||
| style={[ | ||
| styles.overlay, | ||
| { | ||
| backgroundColor: overlayColor, | ||
| opacity: fadeAnim, | ||
| }, | ||
| ]} | ||
| pointerEvents="box-none"> | ||
| <Animated.View |
There was a problem hiding this comment.
Overlay currently allows interaction with content behind it
The root overlay view uses pointerEvents="box-none", so areas outside the inner card remain touchable and can interact with the underlying screen while the “unavailable” overlay is visible.
If this overlay is meant to behave like a modal that blocks interaction, consider removing pointerEvents (default is 'auto') or switching to 'box-only' so the full-screen overlay intercepts touches and prevents accidental background interaction.
🤖 Prompt for AI Agents
In components/UnavailableOverlay.tsx around lines 95 to 112, the root
Animated.View currently uses pointerEvents="box-none" which allows touches to
pass through the overlay to underlying content; change this so the overlay
blocks interaction by removing the pointerEvents prop (default 'auto') or
setting pointerEvents="box-only" on the root Animated.View; if the inner card
should still allow its children to receive touches, set pointerEvents="box-none"
on the inner card view instead.
| const [isLoggingOut, setIsLoggingOut] = useState<boolean>(false); | ||
| const [modalVisible, setModalVisible] = useState(false); |
There was a problem hiding this comment.
Ensure isLoggingOut is always reset even on errors
If any of the storage operations throw, setIsLoggingOut(false) will never run and any UI bound to isLoggingOut could get stuck in a “logging out…” state.
Consider wrapping the body of logout in try/finally so the flag is always cleared:
- const logout = async () => {
- setIsLoggingOut(true);
- await Promise.all([
- storage.deleteItem(API_TOKEN_KEY),
- storage.deleteItem(ACCOUNT_INFO_KEY),
- storage.setItem(IS_LOGGED_IN_KEY, 'false'),
- ]);
- setToken(null);
- setIsLoggedIn(false);
- setUsername(null);
- setAccountInfo(null);
- setIsLoggingOut(false);
- };
+ const logout = async () => {
+ setIsLoggingOut(true);
+ try {
+ await Promise.all([
+ storage.deleteItem(API_TOKEN_KEY),
+ storage.deleteItem(ACCOUNT_INFO_KEY),
+ storage.setItem(IS_LOGGED_IN_KEY, 'false'),
+ ]);
+ setToken(null);
+ setIsLoggedIn(false);
+ setUsername(null);
+ setAccountInfo(null);
+ } finally {
+ setIsLoggingOut(false);
+ }
+ };Also applies to: 186-198, 200-208
🤖 Prompt for AI Agents
In hooks/useAuth.ts around lines 17-18 (and similarly at 186-198 and 200-208),
the logout flow sets isLoggingOut to true but does not guarantee
setIsLoggingOut(false) runs if any awaited storage or network operation throws;
wrap the logout body in a try/finally so the finally always calls
setIsLoggingOut(false) (and any other UI flags that must be reset), moving error
handling into the try/catch as needed so the finally only performs cleanup,
ensuring the UI never remains stuck in the logging-out state.
| try { | ||
| const response = await checkoutApi<AppStateResponse[]>('/api/app/state'); | ||
|
|
||
| // Check for 2xx or 304 status codes | ||
| const isSuccess = | ||
| (response.status >= 200 && response.status < 300) || response.status === 304; | ||
|
|
||
| if (isMountedRef.current) { | ||
| if ( | ||
| isSuccess && | ||
| response.data && | ||
| Array.isArray(response.data) && | ||
| response.data.length > 0 | ||
| ) { | ||
| const appState = response.data[0]; | ||
|
|
||
| // Check if webState is 0 (unavailable) | ||
| if (appState.webState === 0) { | ||
| setUnavailable( | ||
| appState.webStateMsg || 'CheckOut is currently unavailable', | ||
| appState.webStateLink | ||
| ); | ||
| setIsChecking(false); | ||
| previousConnectedRef.current = false; | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // Connection is successful and service is available | ||
| if (isSuccess) { | ||
| const wasDisconnectedBefore = !previousConnectedRef.current; | ||
| if (wasDisconnectedBefore) { | ||
| setWasDisconnected(true); | ||
| // Reset after a short delay | ||
| setTimeout(() => setWasDisconnected(false), 100); | ||
| } | ||
| // Clear any connection errors or unavailable states | ||
| clearConnectionError(); | ||
| previousConnectedRef.current = true; | ||
| } else { | ||
| setConnectionLost(); | ||
| previousConnectedRef.current = false; | ||
| } | ||
|
|
||
| setIsChecking(false); | ||
| return isSuccess; | ||
| } |
There was a problem hiding this comment.
Treat checkoutApi failures (success === false) as connection failures, even on 2xx
isSuccess currently only checks HTTP status:
const isSuccess =
(response.status >= 200 && response.status < 300) || response.status === 304;However, checkoutApi returns success: false for cases like invalid JSON (HTML error pages, etc.) while still reporting a 2xx status. In those cases, utils/api already attempts to set a connection error in the context, but this hook will treat the response as success and immediately clear that error via clearConnectionError(), masking real problems.
You should factor in response.success so only genuinely healthy responses are treated as success.
- // Check for 2xx or 304 status codes
- const isSuccess =
- (response.status >= 200 && response.status < 300) || response.status === 304;
+ // Treat as success only if HTTP status is OK and the API reported success
+ const isSuccess =
+ response.success &&
+ ((response.status >= 200 && response.status < 300) || response.status === 304);🤖 Prompt for AI Agents
In hooks/useConnectionMonitor.ts around lines 30 to 76, the code treats a
response with a 2xx status as success even when checkoutApi returned
response.success === false (e.g. invalid JSON or HTML error pages), which causes
clearConnectionError() to wipe out real errors; update the isSuccess logic to
require both a healthy status and response.success (e.g. include
response.success in the boolean), then only run the appState checks and
clearConnectionError() when that combined success is true; when response.success
is false (regardless of HTTP status) treat it as a connection failure path (call
setConnectionLost(), set previousConnectedRef.current = false,
setIsChecking(false), and return false) so connection errors are not masked.
package.json
Outdated
| "expo": "^54.0.10", | ||
| "expo-auth-session": "~7.0.8", | ||
| "expo-blur": "~15.0.7", | ||
| "expo-clipboard": "~8.0.7", | ||
| "expo-constants": "~18.0.9", | ||
| "expo-crypto": "~15.0.7", | ||
| "expo-font": "~14.0.8", | ||
| "expo-haptics": "~15.0.7", | ||
| "expo-linking": "~8.0.8", | ||
| "expo-random": "^14.0.1", | ||
| "expo-router": "~4.0.17", | ||
| "expo-secure-store": "~14.0.1", | ||
| "expo-splash-screen": "~0.29.22", | ||
| "expo-status-bar": "~2.0.1", | ||
| "expo-symbols": "~0.2.2", | ||
| "expo-system-ui": "~4.0.8", | ||
| "expo-web-browser": "~14.0.2", | ||
| "react": "18.3.1", | ||
| "react-dom": "18.3.1", | ||
| "react-native": "0.76.7", | ||
| "react-native-gesture-handler": "~2.20.2", | ||
| "react-native-reanimated": "~3.16.1", | ||
| "react-native-safe-area-context": "4.12.0", | ||
| "react-native-screens": "~4.4.0", | ||
| "react-native-svg": "^15.11.1", | ||
| "expo-router": "~6.0.8", | ||
| "expo-secure-store": "~15.0.7", | ||
| "expo-splash-screen": "~31.0.10", | ||
| "expo-status-bar": "~3.0.8", | ||
| "expo-symbols": "~1.0.7", | ||
| "expo-system-ui": "~6.0.7", | ||
| "expo-web-browser": "~15.0.7", |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm Expo ecosystem package versions are mutually compatible.
Multiple Expo packages have recommended versions for compatibility with Expo 54 — the documentation specifically lists versions like expo-router ~6.0.8, expo-splash-screen ~31.0.10, expo-symbols ~1.0.7, expo-system-ui ~6.0.7, and expo-web-browser ~15.0.7. Your versions match these recommendations, which is good.
However, verify that all Expo sub-packages have been tested together, especially since you've also upgraded react-native-reanimated to ~4.1.0 (a major version bump). Run npm install or yarn install and check for peer dependency warnings.
🏁 Script executed:
#!/bin/bash
# Check for peer dependency warnings during install
npm ci 2>&1 | grep -i "peer\|warn\|ERR"Length of output: 1614
---
🏁 Script executed:
```shell
#!/bin/bash
# Check full package.json for relevant packages
cat package.json | grep -E "expo-random|expo-crypto|react-native-reanimated"
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Also show the context around these packages if they exist
grep -n -A2 -B2 "expo-random\|expo-crypto\|react-native-reanimated" package.json || echo "Not found in package.json"Length of output: 528
Remove deprecated expo-random package in favor of expo-crypto.
The Expo package versions are mutually compatible and install successfully without peer dependency conflicts. However, the npm output reveals that expo-random@14.0.1 is deprecated and should be replaced with expo-crypto, which is already in your dependencies at line 27.
Remove "expo-random": "^14.0.1" from package.json (line 31) and update all imports throughout the codebase to use expo-crypto instead. Both packages provide the same functionality, so this is a straightforward migration.
🤖 Prompt for AI Agents
In package.json around lines 22 to 38, remove the deprecated dependency
"expo-random": "^14.0.1" (line ~31) and replace its usage with the
already-present "expo-crypto"; then update all project imports that reference
expo-random to import equivalent APIs from expo-crypto (search/replace import
paths and adjust any API names if necessary), run a full test/build to ensure no
breakage, and then run npm/yarn install to update lockfile.
package.json
Outdated
| "react": "19.1.0", | ||
| "react-dom": "19.1.0", | ||
| "react-native": "0.81.4", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Babel configuration for React 19.1.0 owner stacks compatibility.
React 19.1.0 has a known issue where owner stacks do not work as expected in React Native if using the @babel/plugin-transform-function-name Babel plugin, which is enabled by default in the React Native Babel Preset; a fix will ship in a future React Native release. React Native 0.81.4 ships with React 19.1.0.
Ensure your babel.config.js either disables this plugin or includes a workaround. This is particularly important for development debugging using the owner stack feature.
🌐 Web query:
React 19 owner stacks Babel plugin transform-function-name workaround
💡 Result:
Short answer — if Babel's transform-function-name (or a preset that includes it) breaks React 19 owner stacks, the usual workarounds are:
-
Remove/disable @babel/plugin-transform-function-name (or stop using a preset that forces it). This prevents Babel from changing function.name semantics that React's owner-stack detection relies on. [1][2]
-
If you can't remove it, restore correct owner identity by explicitly setting component displayName (or using @babel/plugin-transform-react-display-name) for affected components. [3][1]
-
As a last resort, upgrade to the React patch that fixes compatibility with ES2015 function.name semantics (React 19.1.1+ mentions an owner-stacks fix) — upgrading React may eliminate the issue. [4]
References
- @babel/plugin-transform-function-name docs. [1]
- @babel/plugin-transform-function-name (npm listing). [2]
- @babel/plugin-transform-react-display-name docs. [3]
- React 19 changelog / owner-stacks note and fix. [4]
Let me check the codebase for Babel configuration:
I can see the issue is confirmed. Owner stacks are not working as expected in React Native if you use @babel/plugin-transform-function-name Babel plugin, which is enabled by default in the React Native Babel Preset, with a fix shipping in a future release of React Native. However, React 19.1.1 (released July 28, 2025) fixed Owner Stacks to work with ES2015 function.name semantics, while the package.json is pinned to React 19.1.0.
Let me search the codebase for Babel configuration:
🏁 Script executed:
#!/bin/bash
# Search for Babel configuration files
fd -t f -E node_modules -E '.git' \( -name 'babel.config.*' -o -name '.babelrc*' \)
# Check package.json for babel config
cat package.json | grep -A 10 '"babel"'Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for React 19.1.1+ release information
echo "Checking React versions..."Length of output: 26
🏁 Script executed:
#!/bin/bash
# Search for Babel configuration files in the codebase
fd -t f -E node_modules \( -name 'babel.config.*' -o -name '.babelrc*' \)
# Also check if babel config exists in package.json
echo "--- Checking package.json for babel config ---"
cat package.json | grep -A 5 '"babel"' || echo "No babel config in package.json"Length of output: 309
Upgrade React to 19.1.1 or later to fix owner stacks compatibility issue.
React 19.1.0 has a known incompatibility with the @babel/plugin-transform-function-name Babel plugin (enabled by default in React Native), which breaks owner stacks—a critical debugging feature. React 19.1.1 (released July 28, 2025) includes a fix for this issue.
Since no Babel configuration workaround is present in the codebase, upgrade React from 19.1.0 to 19.1.1 or later in package.json (line 39). Alternatively, add babel.config.js to disable @babel/plugin-transform-function-name or apply the displayName workaround for affected components.
🤖 Prompt for AI Agents
package.json lines 39-41: the project pins "react" to 19.1.0 which is
incompatible with the default React Native Babel plugin and breaks owner stacks;
update "react" to "19.1.1" or later (and keep "react-dom" in sync if used) in
package.json, then run your package manager (npm/yarn/pnpm) to regenerate
lockfile and node_modules and run the test/dev build to verify no regressions;
if updating is not possible, alternatively add a babel.config.js to disable
@babel/plugin-transform-function-name or apply the displayName workaround for
affected components.
🧩 Analysis chain
Verify Babel configuration for React 19.1.0 owner stacks compatibility.
React 19.1.0 has a known issue where owner stacks do not work as expected in React Native if you use the @babel/plugin-transform-function-name Babel plugin, which is enabled by default in the React Native Babel Preset; a fix will ship in a future React Native release. React Native 0.81.4 ships with React 19.1.0.
Ensure your babel.config.js either disables this plugin or has been updated with a workaround. This is particularly important for development debugging.
🌐 Web query:
React 19.1 Babel plugin-transform-function-name owner stacks workaround
💡 Result:
Short answer
- Workaround: remove/disable @babel/plugin-transform-function-name in your Babel config (or override metro-react-native-babel-preset to not apply it) so React 19.1 owner stacks work correctly. [1][2]
- Permanent fix: upgrade to the React 19.1.1 build that restores owner-stack support (shipped in newer React Native releases). [1][3]
What to do (concise)
-
Quick workaround (recommended while waiting for an upgrade)
- In babel.config.js / .babelrc remove or prevent @babel/plugin-transform-function-name from running (or explicitly set plugins without that plugin).
- If you rely on the metro preset, add an override to your Babel config to exclude that plugin or create an explicit plugins list that omits it. (Metro has long-standing notes that transform-function-name is not required in modern JSC builds.) [2][4]
-
Upgrade (preferred)
- Upgrade React to the patched 19.1.1 build (and/or upgrade React Native to a release that includes it, e.g. RN releases after 0.80 that include the fix). That release restores owner-stack behavior even when transform-function-name is present. [1][3]
Sources
- React Native 0.80 release notes (notes owner-stacks bug with @babel/plugin-transform-function-name). [1]
- Announcement/notes describing the 19.1.1 change that fixes owner stacks when that Babel plugin is present. [3]
- Metro / babel preset discussion noting @babel/plugin-transform-function-name is often unnecessary and can be removed/overridden. [2]
If you want, tell me your babel.config.js and I’ll show the exact small change to disable the plugin or add an override.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Babel configuration files
fd -e js -e json -e cjs -e mjs '(babel\.config|\.babelrc)' | head -20
Length of output: 15
🏁 Script executed:
#!/bin/bash
# Look for babel configuration in common locations
find . -maxdepth 2 -name 'babel.config.*' -o -name '.babelrc*' 2>/dev/null | grep -v node_modulesLength of output: 17
🏁 Script executed:
#!/bin/bash
# Extract React version from package.json to confirm 19.1.0 or if already upgraded
rg '"react":\s*"[^"]*"' package.json -A 0 -B 0Length of output: 22
🏁 Script executed:
#!/bin/bash
# Search for any existing Babel plugin configuration or metro preset references
rg -i 'plugin-transform-function-name|metro-react-native-babel-preset' --type json --type js --type typescriptLength of output: 34
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine babel.config.js content
cat babel.config.js
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Search for metro preset or transform-function-name with corrected syntax
rg -i 'plugin-transform-function-name|metro-react-native-babel-preset' --type json --type jsLength of output: 303
---
🌐 Web query:
```web
babel-preset-expo includes plugin-transform-function-name default
💡 Result:
Short answer: Yes — indirectly.
Reason: babel-preset-expo depends on/uses @babel/preset-env (so the transforms that preset includes are available), and @babel/preset-env includes @babel/plugin-transform-function-name. [1][2][3]
Sources:
- babel-preset-expo package.json showing @babel/preset-env as a dependency. [1]
- @babel/plugin-transform-function-name docs (notes the plugin is included in @babel/preset-env). [2]
- Expo docs stating babel-preset-expo is the default preset (for context). [3]
---
🌐 Web query:
```web
React Native 0.81.4 React version shipped
💡 Result:
React Native 0.81.4 uses React 19.1.1 (React is specified as a ^19.1.1 peer dependency). [1][2]
Sources:
[1] React Native v0.81.4 release (GitHub).
[2] packages/react-native/package.json showing peerDependencies for react ^19.1.1.
Upgrade React from 19.1.0 to 19.1.1 to fix owner stacks compatibility with React Native 0.81.4.
React 19.1.0 owner stacks do not work correctly when @babel/plugin-transform-function-name is enabled (which babel-preset-expo enables by default). React Native 0.81.4 ships with React 19.1.1, which restores owner-stack support even when the plugin is present. Update package.json line 39 to use "react": "19.1.1" to align with React Native 0.81.4's requirements and resolve the issue.
🤖 Prompt for AI Agents
In package.json around lines 39 to 41, bump React from "19.1.0" to "19.1.1" to
match React Native 0.81.4 and restore owner-stack compatibility when
@babel/plugin-transform-function-name is enabled; update the "react" dependency
entry to "19.1.1" and run your package manager install (npm/yarn/pnpm) and
re-lock to ensure lockfile consistency.
| "react-dom": "19.1.0", | ||
| "react-native": "0.81.4", | ||
| "react-native-gesture-handler": "~2.28.0", | ||
| "react-native-reanimated": "~4.1.0", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify react-native-worklets-core version compatibility with Reanimated 4.1.0.
Reanimated 4.1.0 requires react-native-worklets package between versions 0.4.0 and 0.4. You've specified react-native-worklets-core: ^1.6.2 (line 51), which is a significantly different version range. Confirm whether this is the correct package or if the requirement has changed.
Also applies to: 51-51
🌐 Web query:
react-native-worklets-core 1.6.2 react-native-reanimated 4.1.0
💡 Result:
Short answer: they’re not a matched pair.
- react-native-reanimated 4.1.x expects the separate package react-native-worklets (v0.5.x/0.6.x) as its worklets dependency and requires the React Native New Architecture. (docs.swmansion.com)
- react-native-worklets-core v1.6.2 is a different package (published separately) and is not the react-native-worklets peer that Reanimated 4 requires. Use react-native-worklets@0.5.x/0.6.x with Reanimated 4.1.x, and ensure you’ve migrated to the New Architecture and matched Reanimated’s babel/plugin and native parts. (npmjs.com)
If you want, tell me your RN version and whether you’re on the New Architecture (Fabric) and I’ll recommend exact package versions and install steps.
Citations:
- 1: https://docs.swmansion.com/react-native-reanimated/docs/guides/compatibility/?utm_source=openai
- 2: https://www.npmjs.com/package/react-native-worklets-core?utm_source=openai
Replace react-native-worklets-core with correct react-native-worklets package.
Reanimated 4.1.x requires the separate package react-native-worklets (v0.5.x/0.6.x), but your package.json specifies react-native-worklets-core@^1.6.2 (line 51). These are different packages and incompatible. Update line 51 to use "react-native-worklets": "^0.5.0" or "^0.6.0" instead.
🤖 Prompt for AI Agents
In package.json around lines 43 to 43 (and update the dependency at line ~51),
replace the incorrect dependency "react-native-worklets-core@^1.6.2" with the
correct package "react-native-worklets" and set an appropriate compatible
version such as "^0.5.0" or "^0.6.0"; update the package name and version in the
dependencies section, run npm/yarn install to refresh lockfile, and verify the
change doesn't break other dependency resolutions.
| "react-native-reanimated": "~4.1.0", | ||
| "react-native-safe-area-context": "~5.6.0", | ||
| "react-native-screens": "~4.16.0", | ||
| "react-native-svg": "15.12.1", | ||
| "react-native-toast-message": "^2.2.1", | ||
| "react-native-web": "~0.19.13", | ||
| "react-native-webview": "13.12.5" | ||
| "@react-native-cookies/cookies": "^6.2.1", | ||
| "react-native-web": "^0.21.0", | ||
| "react-native-webview": "13.15.0", | ||
| "react-native-worklets-core": "^1.6.2" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify react-native-worklets-core version compatibility with Reanimated 4.1.0.
[Reanimated] react-native-worklets package isn't installed; a version between 0.4.0 and 0.4 is needed to use Reanimated 4.1.0. However, you've specified react-native-worklets-core: ^1.6.2, which may be a different package or updated requirement.
Confirm this version is compatible with react-native-reanimated: ~4.1.0.
🌐 Web query:
react-native-worklets-core 1.6.2 react-native-reanimated 4.1.0 compatibility
💡 Result:
Short answer: Not guaranteed — react-native-reanimated 4.1.x expects the react-native-worklets package (v0.5.x / v0.6.x), not react-native-worklets-core 1.6.2. Reanimated's compatibility docs list supported react-native-worklets versions for 4.1.x (0.5.x and 0.6.x) and warn you must install the matching worklets package; react-native-worklets-core is a different package and is not listed as the supported dependency. [1][2]
If you need Reanimated 4.1.0, install a compatible react-native-worklets release (e.g. 0.5.x/0.6.x) or follow the Reanimated docs/troubleshooting steps about worklets; if you must use react-native-worklets-core, test thoroughly — you may encounter initialization/“Unable to find a specification for RNWorklets” errors. [2][3]
Sources:
- Reanimated compatibility (supported worklets versions for 4.1.x). [1]
- Reanimated migration / added react-native-worklets dependency. [2]
- react-native-worklets-core npm page (package vs. react-native-worklets). [3]
Replace react-native-worklets-core with react-native-worklets.
react-native-reanimated 4.1.x expects react-native-worklets (v0.5.x or v0.6.x), not react-native-worklets-core 1.6.2, which is a different package. Using the wrong package may cause "Unable to find a specification for RNWorklets" errors. Change line 51 from "react-native-worklets-core": "^1.6.2" to "react-native-worklets": "~0.6.0" (or ~0.5.0 if needed).
🤖 Prompt for AI Agents
In package.json around lines 43 to 51, the dependency
"react-native-worklets-core": "^1.6.2" is incorrect for react-native-reanimated
4.1.x and will cause RNWorklets spec errors; replace that entry with
"react-native-worklets": "~0.6.0" (or "~0.5.0" if you must match older
reanimated constraints) and run yarn/npm install to update lockfile and pods as
needed.
…-aware tab bar, and connection error handling
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @app/(tabs)/autocheckin.tsx:
- Around line 256-260: The onRefresh callback currently sets refreshing true,
awaits fetchWaitlistStatus, then sets refreshing false but won’t stop the
spinner if fetchWaitlistStatus throws; wrap the await in a try/finally inside
the useCallback for onRefresh so setRefreshing(false) runs in the finally block,
referencing the existing onRefresh, fetchWaitlistStatus and setRefreshing
functions to ensure the spinner always stops even on exceptions.
- Around line 108-133: The computed waitlistState currently allows showing the
join UI when waitlistStatus.status === 'unauthenticated' even if the server says
unauthenticated; update the useMemo that computes waitlistState (the block
referencing statusKey, waitlistStatus and isLoggedIn) to treat statusKey ===
'unauthenticated' as equivalent to signed out—i.e., if statusKey ===
'unauthenticated' return 'SIGNED_OUT' (or combine into a single check like if
(statusKey === 'unauthenticated' || !isLoggedIn) return 'SIGNED_OUT') before any
CAN_JOIN/ON_WAITLIST/ACCEPTED checks so the UI never shows join controls when
the server reports unauthenticated.
- Around line 108-133: The waitlistState useMemo currently falls through to
'CAN_JOIN' even when a logged-in user's waitlistStatus.canJoin is explicitly
false; update the logic inside the useMemo (identifying the function by
waitlistState and the hook useMemo and the object waitlistStatus) to explicitly
handle canJoin === false (for example return a new 'CANNOT_JOIN' state when
isLoggedIn && waitlistStatus?.canJoin === false) before the final fallback, so
the UI can disable or explain the join action instead of showing 'CAN_JOIN'.
- Around line 248-255: handleOpenAutoCheckin and handleSupportLink call
Linking.openURL() but ignore that it returns a Promise and may reject or be
unsupported; update both functions to be async (or return a Promise), call
Linking.canOpenURL(url) first, then await Linking.openURL(url) inside a
try/catch, and handle failures (e.g., show a user-facing alert or fallback
behavior and log the error) so errors are not swallowed; reference the functions
handleOpenAutoCheckin and handleSupportLink and the Linking.openURL /
Linking.canOpenURL APIs when making the change.
🧹 Nitpick comments (9)
hooks/useVersionCheck.ts (1)
16-31: Consider handling non-numeric version segments.If version strings contain non-numeric parts (e.g.,
"1.1.9-beta"),Number()will returnNaN, and comparisons withNaNalways returnfalse, potentially leading to incorrect results.🛠️ Suggested improvement
function compareVersions(version1: string, version2: string): number { - const v1Parts = version1.split('.').map(Number); - const v2Parts = version2.split('.').map(Number); + const v1Parts = version1.split('.').map(p => parseInt(p, 10) || 0); + const v2Parts = version2.split('.').map(p => parseInt(p, 10) || 0);components/ConnectionErrorBanner.tsx (1)
77-92: Consider removingslideAnimfrom dependency array.
slideAnimis created viauseRef(new Animated.Value(-100))and never changes identity. Including it in the dependency array is harmless but unnecessary.🔧 Minor cleanup
- }, [shouldShow, slideAnim]); + }, [shouldShow]);app/(tabs)/settings.tsx (3)
61-63: Empty catch block silently swallows navigation errors.Consider logging the error or removing the try-catch if no handling is needed.
🔧 Suggested fix
try { router.push({ pathname: '/(tabs)' }); - } catch {} + } catch (error) { + console.warn('Navigation after logout failed:', error); + }
84-84: Extract default API URL to a constant.The string
'https://checkout.ac'is duplicated at lines 84 and 121. Consider importingDEFAULT_API_URLfrom@/constants/apiif available, or defining a local constant.
293-434: Consider extracting modal styles to StyleSheet.The inline styles for the modals are verbose. Extracting them to the settings styles module would improve maintainability and consistency.
app/(tabs)/_layout.tsx (1)
98-100: TabLayout wrapper appears redundant.The
TabLayoutfunction simply returns<TabLayoutContent />. If no providers or wrappers are planned, consider exportingTabLayoutContentdirectly as default.app/styles/autocheckin.styles.ts (1)
162-169: Minor:borderRadiusshould be half of width/height for a perfect circle.
featureBullethaswidth: 10, height: 10butborderRadius: 6. For a perfect circle, useborderRadius: 5.🔧 Fix
featureBullet: { width: 10, height: 10, - borderRadius: 6, + borderRadius: 5, marginTop: 6, marginRight: 12, backgroundColor: theme.primary, },app/(tabs)/autocheckin.tsx (2)
15-33: Avoidas { msg?: string }casts; alignWaitlistStatusResponseto actual payload fields.
Right nowWaitlistStatusResponsedeclaresmessage?: stringbut the error path reads(response.data as { msg?: string })?.msg(Line 79-81). This is brittle and easy to regress.Proposed diff
interface WaitlistStatusResponse { success: boolean; status?: string; - message?: string; + message?: string; + msg?: string; // backend sometimes returns msg canJoin?: boolean; onWaitlist?: boolean; username?: string; actionType?: number; } } else { const message = - response.error || (response.data as { msg?: string })?.msg || 'Unable to load status.'; + response.error || + response.data?.message || + response.data?.msg || + 'Unable to load status.'; setWaitlistStatus(null); setWaitlistError(message); }Also applies to: 66-83
264-331: (Optional) Add basic accessibility props to primary touch targets.
KeyTouchableOpacitybuttons/links don’t exposeaccessibilityRole/accessibilityLabel.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/(tabs)/_layout.tsxapp/(tabs)/autocheckin.tsxapp/(tabs)/settings.tsxapp/styles/autocheckin.styles.tscomponents/ConnectionErrorBanner.tsxcomponents/VersionAwareTabBar.tsxcomponents/VersionUpdateBanner.tsxhooks/useVersionCheck.ts
🧰 Additional context used
🧬 Code graph analysis (8)
app/(tabs)/_layout.tsx (7)
hooks/useColorScheme.ts (1)
useColorScheme(1-1)hooks/useConnectionMonitor.ts (1)
useConnectionMonitor(11-112)components/VersionAwareTabBar.tsx (1)
VersionAwareTabBar(7-16)constants/Colors.ts (1)
Colors(34-83)components/ui/TabBarBackground.ios.tsx (1)
TabBarBackground(7-40)components/ConnectionErrorBanner.tsx (1)
ConnectionErrorBanner(12-144)components/UnavailableOverlay.tsx (1)
UnavailableOverlay(8-163)
components/VersionUpdateBanner.tsx (4)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)hooks/useVersionCheck.ts (1)
useVersionCheck(33-102)constants/api.ts (1)
getApiUrl(8-16)components/ThemedText.tsx (1)
ThemedText(11-34)
hooks/useVersionCheck.ts (1)
utils/api.ts (1)
checkoutApi(20-150)
components/ConnectionErrorBanner.tsx (4)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)constants/api.ts (1)
getApiUrl(8-16)components/ThemedText.tsx (1)
ThemedText(11-34)
app/styles/autocheckin.styles.ts (1)
constants/Colors.ts (1)
Colors(34-83)
app/(tabs)/settings.tsx (4)
hooks/useApiUrl.ts (1)
useApiUrl(4-69)constants/api.ts (1)
getApiUrl(8-16)components/ThemedText.tsx (1)
ThemedText(11-34)components/ThemedView.tsx (1)
ThemedView(10-14)
components/VersionAwareTabBar.tsx (1)
components/VersionUpdateBanner.tsx (1)
VersionUpdateBanner(9-60)
app/(tabs)/autocheckin.tsx (8)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)constants/Colors.ts (1)
Colors(34-83)hooks/useToast.ts (1)
useToast(13-48)utils/api.ts (1)
checkoutApi(20-150)components/ThemedText.tsx (1)
ThemedText(11-34)components/TabScreenScrollView.tsx (1)
TabScreenScrollView(12-35)components/ThemedView.tsx (1)
ThemedView(10-14)components/AuthModal.tsx (1)
AuthModal(16-61)
🔇 Additional comments (9)
hooks/useVersionCheck.ts (1)
33-101: Well-structured version check hook.The hook correctly handles:
- Mounted state tracking to prevent state updates after unmount
- Cleanup of the interval on unmount
- Silent error handling to avoid showing stale banners on failures
- Memoization of
checkVersionwithuseCallbackcomponents/VersionUpdateBanner.tsx (2)
13-60: Clean implementation with proper error handling.The component correctly:
- Memoizes the press handler
- Returns
nullearly when not outdated (avoiding unnecessary renders)- Handles URL opening errors gracefully
- Uses themed colors appropriately
9-11: No action needed—VersionUpdateBanner is rendered only once in the app.VersionUpdateBanner is imported and rendered exclusively in VersionAwareTabBar, which is used once as the bottom tab navigation component. The useVersionCheck hook properly cleans up its 5-minute polling interval on unmount, so there's no risk of duplicate API calls with the current architecture. The concern about multiple instances is not applicable here.
components/ConnectionErrorBanner.tsx (1)
12-144: Well-designed connection error banner with good UX.The component handles:
- Animated transitions with spring/timing appropriately
- Success banner on reconnection with auto-dismiss
- Platform-aware positioning above the tab bar
- Clean state management for error/success states
components/VersionAwareTabBar.tsx (1)
7-16: Clean composition pattern for version-aware tab bar.The component correctly composes the
VersionUpdateBannerabove the standardBottomTabBarwhile properly forwarding all props.app/(tabs)/_layout.tsx (2)
41-95: Good integration of connection monitoring and UI components.The layout properly integrates:
- Custom
VersionAwareTabBarfor version update awareness- Global
ConnectionErrorBannerandUnavailableOverlayfor connection state feedback- Tab press listeners to force content refresh
28-39: The logic is correct and does not require changes.The
wasDisconnectedstate fromuseConnectionMonitoris a momentary signal that fires only when connection is successfully restored after being disconnected. It's set totruein the hook (lines 50-56) when:
- An API call succeeds (
isSuccess)- AND the previous state was disconnected (
!previousConnectedRef.current)Then it's immediately reset to
falseafter 100ms. The conditionwasDisconnected && !previousWasDisconnectedRef.currentcorrectly detects this transition and triggers a refresh, matching the comment's intent of detecting "connection was just restored."app/styles/autocheckin.styles.ts (1)
24-218: Well-structured theming module with platform-aware shadows and properly bundled fonts.The styling module correctly:
- Uses platform-specific shadow/elevation patterns
- Provides comprehensive themed styles for the AutoCheckin UI
- Accepts
isDarkoption for conditional styling- References custom font families (SegoeUI, SegoeUIBold, SegoeUISemiBold) that are properly loaded via
useFontsinapp/_layout.tsxand exist inassets/fonts/app/(tabs)/autocheckin.tsx (1)
264-331: UI flow looks cohesive; good state gating + feedback layering.
The state-driven rendering (loader vs error retry vs action area) is clear, and the join/leave actions correctly gate onactionLoadingwhile surfacing toast + inline feedback.Also applies to: 333-437
| const waitlistState: WaitlistUiState = useMemo(() => { | ||
| const statusKey = waitlistStatus?.status?.toLowerCase(); | ||
|
|
||
| if (!isLoggedIn && statusKey !== 'unauthenticated') { | ||
| return 'SIGNED_OUT'; | ||
| } | ||
|
|
||
| if (waitlistStatus?.onWaitlist || statusKey === 'on_waitlist') { | ||
| return 'ON_WAITLIST'; | ||
| } | ||
|
|
||
| if (statusKey && ['accepted', 'active'].includes(statusKey)) { | ||
| return 'ACCEPTED'; | ||
| } | ||
|
|
||
| if (isLoggedIn && waitlistStatus?.canJoin) { | ||
| return 'CAN_JOIN'; | ||
| } | ||
|
|
||
| if (!isLoggedIn) { | ||
| return 'SIGNED_OUT'; | ||
| } | ||
|
|
||
| return 'CAN_JOIN'; | ||
| }, [isLoggedIn, waitlistStatus]); | ||
|
|
There was a problem hiding this comment.
Treat server “unauthenticated” as SIGNED_OUT even if isLoggedIn says otherwise.
Current logic (Line 111-131) can show join UI when the server is explicitly unauthenticated (e.g., expired token) and then bounce the user on action.
Proposed diff
const waitlistState: WaitlistUiState = useMemo(() => {
const statusKey = waitlistStatus?.status?.toLowerCase();
- if (!isLoggedIn && statusKey !== 'unauthenticated') {
+ if (!isLoggedIn || statusKey === 'unauthenticated') {
return 'SIGNED_OUT';
}
@@
- if (!isLoggedIn) {
- return 'SIGNED_OUT';
- }
-
return 'CAN_JOIN';
}, [isLoggedIn, waitlistStatus]);🤖 Prompt for AI Agents
In @app/(tabs)/autocheckin.tsx around lines 108 - 133, The computed
waitlistState currently allows showing the join UI when waitlistStatus.status
=== 'unauthenticated' even if the server says unauthenticated; update the
useMemo that computes waitlistState (the block referencing statusKey,
waitlistStatus and isLoggedIn) to treat statusKey === 'unauthenticated' as
equivalent to signed out—i.e., if statusKey === 'unauthenticated' return
'SIGNED_OUT' (or combine into a single check like if (statusKey ===
'unauthenticated' || !isLoggedIn) return 'SIGNED_OUT') before any
CAN_JOIN/ON_WAITLIST/ACCEPTED checks so the UI never shows join controls when
the server reports unauthenticated.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i autocheckinRepository: jameshaworthcs/Checkout-App
Length of output: 208
🏁 Script executed:
# Check file structure
git ls-files | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 493
🏁 Script executed:
# Search for the entire file to see its size and content
rg "const waitlistState: WaitlistUiState = useMemo" -lRepository: jameshaworthcs/Checkout-App
Length of output: 97
🏁 Script executed:
# Search for WaitlistUiState type definition
rg "type WaitlistUiState" -A 5Repository: jameshaworthcs/Checkout-App
Length of output: 525
🏁 Script executed:
# Find where waitlistStatus is fetched/populated
rg "waitlistStatus" -B 3 -A 3 | head -100Repository: jameshaworthcs/Checkout-App
Length of output: 3890
🏁 Script executed:
# Try to understand the directory structure better
ls -la app/(tabs)/ 2>/dev/null || ls -la | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 209
🏁 Script executed:
# Get all files in the repo
git ls-files | wc -l
git ls-files | head -30Repository: jameshaworthcs/Checkout-App
Length of output: 814
🏁 Script executed:
# Let's search for "SIGNED_OUT" which appears in the code
rg "SIGNED_OUT" -lRepository: jameshaworthcs/Checkout-App
Length of output: 97
🏁 Script executed:
# Check if this is a web app or another type
git ls-files | grep -E "package\.json|tsconfig|next\.config" | head -10Repository: jameshaworthcs/Checkout-App
Length of output: 97
🏁 Script executed:
# Read the complete autocheckin.tsx file
cat "app/(tabs)/autocheckin.tsx"Repository: jameshaworthcs/Checkout-App
Length of output: 14879
🏁 Script executed:
# Search for the WaitlistStatusResponse type
rg "type WaitlistStatusResponse|interface WaitlistStatusResponse" -A 15Repository: jameshaworthcs/Checkout-App
Length of output: 981
🏁 Script executed:
# Search for API endpoint definition
rg "fetchWaitlistStatus|/api/account/waitlist" -B 5 -A 20Repository: jameshaworthcs/Checkout-App
Length of output: 11823
🏁 Script executed:
# Look at the rendering logic around line 319-330
sed -n '310,340p' "app/(tabs)/autocheckin.tsx"Repository: jameshaworthcs/Checkout-App
Length of output: 913
🏁 Script executed:
# Get context around the join button rendering
rg "Join the Waitlist|renderActionButton" -B 3 -A 15Repository: jameshaworthcs/Checkout-App
Length of output: 3496
🏁 Script executed:
# Get beginning of file to see imports and type definitions
head -n 100 "app/(tabs)/autocheckin.tsx"Repository: jameshaworthcs/Checkout-App
Length of output: 3570
🏁 Script executed:
# Get lines 100-150
sed -n '100,150p' "app/(tabs)/autocheckin.tsx"Repository: jameshaworthcs/Checkout-App
Length of output: 1573
🏁 Script executed:
# Look at all lines containing "waitlist" to understand the structure
rg "waitlist" -i "app/(tabs)/autocheckin.tsx" -B 2 -A 2Repository: jameshaworthcs/Checkout-App
Length of output: 8540
🏁 Script executed:
# Get tail of file to see rendering logic
tail -n 200 "app/(tabs)/autocheckin.tsx"Repository: jameshaworthcs/Checkout-App
Length of output: 7266
🏁 Script executed:
# Check if there are other files that handle the waitlist API
git ls-files | xargs grep -l "waitlist" | grep -v node_modulesRepository: jameshaworthcs/Checkout-App
Length of output: 97
Handle the case when the server explicitly prevents joining with canJoin === false.
The waitlistState logic incorrectly defaults to 'CAN_JOIN' when a logged-in user has canJoin === false. The current conditions check if (isLoggedIn && waitlistStatus?.canJoin), but when canJoin is false, this condition fails and the code falls through to the final return 'CAN_JOIN'. This renders the "Join the Waitlist" button even when the server indicates the user cannot join, leading to avoidable POST failures and confusion.
You need to explicitly handle the case where canJoin === false to display appropriate UX (either disable the button, show an explanation, or introduce a new state like 'CANNOT_JOIN').
Current logic
if (isLoggedIn && waitlistStatus?.canJoin) {
return 'CAN_JOIN';
}
if (!isLoggedIn) {
return 'SIGNED_OUT';
}
return 'CAN_JOIN'; // Falls through here when canJoin === false
🤖 Prompt for AI Agents
In @app/(tabs)/autocheckin.tsx around lines 108 - 133, The waitlistState useMemo
currently falls through to 'CAN_JOIN' even when a logged-in user's
waitlistStatus.canJoin is explicitly false; update the logic inside the useMemo
(identifying the function by waitlistState and the hook useMemo and the object
waitlistStatus) to explicitly handle canJoin === false (for example return a new
'CANNOT_JOIN' state when isLoggedIn && waitlistStatus?.canJoin === false) before
the final fallback, so the UI can disable or explain the join action instead of
showing 'CAN_JOIN'.
| const handleOpenAutoCheckin = useCallback(() => { | ||
| Linking.openURL('https://checkout.ac/auto'); | ||
| }, []); | ||
|
|
||
| const handleSupportLink = useCallback(() => { | ||
| Linking.openURL('https://checkout.ac/support?pre=autocheckin'); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
Handle Linking.openURL() failures (and unsupported URLs).
Linking.openURL() is async and can reject; today failures are silent.
Proposed diff
- const handleOpenAutoCheckin = useCallback(() => {
- Linking.openURL('https://checkout.ac/auto');
- }, []);
+ const handleOpenAutoCheckin = useCallback(async () => {
+ const url = 'https://checkout.ac/auto';
+ const supported = await Linking.canOpenURL(url);
+ if (!supported) {
+ toast.error('Unable to open link');
+ return;
+ }
+ await Linking.openURL(url);
+ }, [toast]);
- const handleSupportLink = useCallback(() => {
- Linking.openURL('https://checkout.ac/support?pre=autocheckin');
- }, []);
+ const handleSupportLink = useCallback(async () => {
+ const url = 'https://checkout.ac/support?pre=autocheckin';
+ const supported = await Linking.canOpenURL(url);
+ if (!supported) {
+ toast.error('Unable to open link');
+ return;
+ }
+ await Linking.openURL(url);
+ }, [toast]);🤖 Prompt for AI Agents
In @app/(tabs)/autocheckin.tsx around lines 248 - 255, handleOpenAutoCheckin and
handleSupportLink call Linking.openURL() but ignore that it returns a Promise
and may reject or be unsupported; update both functions to be async (or return a
Promise), call Linking.canOpenURL(url) first, then await Linking.openURL(url)
inside a try/catch, and handle failures (e.g., show a user-facing alert or
fallback behavior and log the error) so errors are not swallowed; reference the
functions handleOpenAutoCheckin and handleSupportLink and the Linking.openURL /
Linking.canOpenURL APIs when making the change.
| const onRefresh = useCallback(async () => { | ||
| setRefreshing(true); | ||
| await fetchWaitlistStatus(); | ||
| setRefreshing(false); | ||
| }, [fetchWaitlistStatus]); |
There was a problem hiding this comment.
Ensure refresh spinner always stops (even on unexpected exceptions).
onRefresh should use try/finally to guarantee setRefreshing(false).
Proposed diff
const onRefresh = useCallback(async () => {
setRefreshing(true);
- await fetchWaitlistStatus();
- setRefreshing(false);
+ try {
+ await fetchWaitlistStatus();
+ } finally {
+ setRefreshing(false);
+ }
}, [fetchWaitlistStatus]);📝 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.
| const onRefresh = useCallback(async () => { | |
| setRefreshing(true); | |
| await fetchWaitlistStatus(); | |
| setRefreshing(false); | |
| }, [fetchWaitlistStatus]); | |
| const onRefresh = useCallback(async () => { | |
| setRefreshing(true); | |
| try { | |
| await fetchWaitlistStatus(); | |
| } finally { | |
| setRefreshing(false); | |
| } | |
| }, [fetchWaitlistStatus]); |
🤖 Prompt for AI Agents
In @app/(tabs)/autocheckin.tsx around lines 256 - 260, The onRefresh callback
currently sets refreshing true, awaits fetchWaitlistStatus, then sets refreshing
false but won’t stop the spinner if fetchWaitlistStatus throws; wrap the await
in a try/finally inside the useCallback for onRefresh so setRefreshing(false)
runs in the finally block, referencing the existing onRefresh,
fetchWaitlistStatus and setRefreshing functions to ensure the spinner always
stops even on exceptions.
…mission Replace WebView-based checkout with fully native implementation: - Add SessionCard, CodeInput, CodeDisplay components for class sessions - Implement code submission with progress indicator and confetti celebration - Add undo functionality with confirmation modal and time-limited undo banner - Create ShareSheet for generating and sharing session links - Add VerificationSheet for viewing code verification details - Implement AutoSplashModal for post-submission actions - Add useLiveClasses hook for real-time session polling and state management - Add useCourseConfig hook for course configuration - Create crypto utilities for secure code generation - Fix ESLint warnings for unused variables and missing dependencies
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @components/checkout/AutoSplashModal.tsx:
- Around line 53-108: The effect can race and re-run unnecessarily: move the
setStatus('waiting') and setMessage('Checking your attendance...') to before the
await call so they don't overwrite a fast websocket response (i.e., call them
immediately inside useEffect before invoking connectAutoSplash), and stop
putting onComplete in the effect deps by storing the latest callback in a ref
(create onCompleteRef = useRef(onComplete) and a small useEffect to keep
onCompleteRef.current = onComplete), then call onCompleteRef.current(...) inside
both wsResult and error callbacks; keep cleanupRef usage and keep visible/jwt in
the dependency array but remove onComplete.
In @components/checkout/Confetti.tsx:
- Around line 119-121: The render uses Math.random() to set borderRadius causing
particles to flip shape on re-renders; add a new property on the Particle type
(e.g., borderRadius or isRounded) and assign its value once when particles are
created (inside the particle creation/init function you added around the ~line
42–50), then replace the inline Math.random() in Confetti's render with that
particle property (use particle.borderRadius or conditional based on
particle.isRounded) so shape is deterministic after creation.
In @components/checkout/OptionsSheet.tsx:
- Around line 28-31: The openExternalLink function lacks error handling for
failures from getApiUrl() and Linking.openURL(); wrap the async work in a
try/catch inside openExternalLink (reference function openExternalLink) and
await Linking.openURL(path) so errors are caught, then handle errors by logging
them (use your logger or console.error) and surface a user-friendly fallback
(e.g., show an Alert or toast) so failures don’t fail silently; ensure you
validate or sanitize the constructed URL before opening to avoid malformed-URL
errors.
In @components/checkout/UndoBanner.tsx:
- Line 10: The declared but unused onDismiss prop on the UndoBanner component
(in the UndoBannerProps interface and props destructuring in UndoBanner) should
be cleaned up: either remove onDismiss from the UndoBannerProps type and from
the component’s props usage and from any callers that pass it, or implement
dismissal behavior by wiring onDismiss to a dismiss control (e.g., add a close
button that calls props.onDismiss or add a useEffect timer that calls
props.onDismiss after a delay) and ensure the component calls it where
appropriate.
In @utils/crypto.ts:
- Around line 41-67: The native btoa implementation in function btoa is wrong
for non-ASCII because it uses charCodeAt/UTF-16 units; convert the input string
to UTF-8 bytes first and then base64-encode those bytes. Use
TextEncoder().encode(data) (or an equivalent UTF-8 byte conversion) to get a
Uint8Array, then on web call window.btoa on a binary string created from those
bytes, and on native platforms use a byte-aware base64 routine (e.g.,
Buffer.from(bytes).toString('base64') or a platform-safe base64 helper) instead
of charCodeAt; keep Platform.OS checks and update btoa to consistently encode
UTF-8 bytes so generateSubmissionHashes/submissionData are not corrupted.
In @utils/submission.ts:
- Around line 128-164: The socket.onmessage handler (socket.onmessage) advances
the step machine on any message; update it to parse/inspect the server response
before incrementing step (e.g., expect a success flag/status or error field in
the parsed message), only call socket.send(...) for submissionData,
getUserAgent(), or jwt and call step++ when the response indicates success, and
on error call cleanup(), close the socket, reject with the server error (include
details) instead of proceeding; ensure the final step still parses
SubmissionResult and rejects with parse error if needed.
🧹 Nitpick comments (41)
app/_layout.tsx (1)
62-75: Consider memoizingnavThemefor minor optimization.The
navThemeobject is recreated on every render. While the impact is minimal, wrapping it inuseMemowould ensure referential stability and prevent potential unnecessary re-renders inThemeProviderchildren.♻️ Optional optimization
+ const navTheme = useMemo(() => { const baseTheme = colorScheme === 'dark' ? DarkTheme : DefaultTheme; - - const navTheme = { - ...baseTheme, - colors: { - ...baseTheme.colors, - primary: Colors[colorScheme ?? 'light'].primary, - background: Colors[colorScheme ?? 'light'].background, - card: Colors[colorScheme ?? 'light'].background, - text: Colors[colorScheme ?? 'light'].text, - border: Colors[colorScheme ?? 'light'].background, - notification: Colors[colorScheme ?? 'light'].primary, - }, - }; + return { + ...baseTheme, + colors: { + ...baseTheme.colors, + primary: Colors[colorScheme ?? 'light'].primary, + background: Colors[colorScheme ?? 'light'].background, + card: Colors[colorScheme ?? 'light'].background, + text: Colors[colorScheme ?? 'light'].text, + border: Colors[colorScheme ?? 'light'].background, + notification: Colors[colorScheme ?? 'light'].primary, + }, + }; + }, [colorScheme]);components/checkout/ShareSheet.tsx (2)
65-76: Consider logging share errors for debugging.The empty catch block silently handles both user cancellation and actual errors. While cancellation is expected, logging unexpected errors could help with debugging.
♻️ Optional improvement
} catch { - // User cancelled or error + // User cancelled - this is expected behavior + // Actual errors are rare but could be logged if needed }
78-79: Use theme colors for consistency.The hardcoded success colors match
theme.goodButtonvalues from Colors.ts. Using the theme object would improve maintainability and ensure consistency if the color palette changes.♻️ Suggested change
- const successBg = isDark ? '#27AE6033' : '#27AE601A'; - const successText = '#27AE60'; + const successBg = theme.goodButton.background; + const successText = theme.goodButton.text;constants/api.ts (1)
19-31: Consider adding URL validation before storing.The function accepts any string without validating it's a properly formatted URL. Storing a malformed URL could cause issues when making API calls.
♻️ Suggested validation
export async function setCustomApiUrl(url: string): Promise<void> { try { + // Basic URL validation + if (url !== DEFAULT_API_URL) { + try { + new URL(url); + } catch { + throw new Error('Invalid URL format'); + } + } + if (url === DEFAULT_API_URL) { // If setting to default, remove the custom URL await storage.deleteItem(CUSTOM_API_URL_KEY); } else { await storage.setItem(CUSTOM_API_URL_KEY, url); } } catch (error) { console.error('Error setting custom API URL:', error); throw error; } }components/checkout/AutoSplashModal.tsx (1)
123-126: Use theme colors for consistency.Similar to the feedback on ShareSheet, these hardcoded colors match the
theme.goodButtonandtheme.badButtonvalues. Using theme properties would improve maintainability.♻️ Suggested change
- const successBg = isDark ? '#27AE6033' : '#27AE601A'; - const successText = '#27AE60'; - const errorBg = isDark ? '#EB575733' : '#EB57571A'; - const errorText = '#E64800'; + const successBg = theme.goodButton.background; + const successText = theme.goodButton.text; + const errorBg = theme.badButton.background; + const errorText = theme.badButton.text;hooks/useCourseConfig.ts (1)
91-104: Consider extracting shared validation logic.The config parsing and validation logic (checking for
inst,crs,yrfields) is duplicated between the hook'sloadConfigand this standalone function. Extracting it to a shared helper would improve maintainability.♻️ Optional extraction
function parseAndValidateConfig(stored: string | null): CourseConfig | null { if (!stored) return null; try { const parsed = JSON.parse(stored) as CourseConfig; if (parsed.inst && parsed.crs && parsed.yr) { return parsed; } } catch { // Invalid JSON } return null; }Then use in both the hook and standalone function.
utils/crypto.ts (2)
7-12: Unused helper functionarrayBufferToHex.This function is defined but never used in this file or referenced in the provided context. If it's intended for future use, consider removing it to avoid dead code, or document its intended purpose.
#!/bin/bash # Search for usage of arrayBufferToHex across the codebase rg -n "arrayBufferToHex" --type ts --type tsx
115-117: Inconsistent user agent format withgetUserAgent().Line 116 uses
CheckOut-App/${Platform.OS}but thegetUserAgent()function at line 139 returnsCheckOut-App/${Platform.OS}/${Platform.Version}. Consider usinggetUserAgent()here for consistency, unless the server-side hash verification specifically expects this simpler format.♻️ Proposed fix
- // Get user agent (simplified for React Native) - const userAgent = `CheckOut-App/${Platform.OS}`; + // Get user agent for hash generation (must match server expectation) + const userAgent = getUserAgent();components/checkout/BottomSheet.tsx (2)
21-22: StaticSCREEN_HEIGHTwon't update on orientation change.
Dimensions.get('window')is called once at module load. If the device rotates or the window resizes (e.g., iPad multitasking),SCREEN_HEIGHTwill be stale, causing incorrect animation targets andmaxHeightcalculations.♻️ Use useWindowDimensions hook instead
-const { height: SCREEN_HEIGHT } = Dimensions.get('window'); const DISMISS_THRESHOLD = 100;Then inside the component:
+import { useWindowDimensions } from 'react-native'; + export function BottomSheet({ visible, onClose, children, height = 'auto' }: BottomSheetProps) { const { theme } = useAppTheme(); + const { height: screenHeight } = useWindowDimensions(); - const translateY = useSharedValue(SCREEN_HEIGHT); + const translateY = useSharedValue(screenHeight);
59-62:setTimeoutinhandleClosemay fire after unmount.If the component unmounts before the 250ms timeout completes,
onClosewill still be called. Consider clearing this timeout on unmount or usingwithTiming's callback viarunOnJSto invokeonClosewhen the animation actually completes.♻️ Alternative using animation callback
const handleClose = useCallback(() => { - animateOut(); - setTimeout(onClose, 250); + translateY.value = withTiming(SCREEN_HEIGHT, { duration: 250 }, finished => { + if (finished) { + runOnJS(onClose)(); + } + }); + backdropOpacity.value = withTiming(0, { duration: 250 }); }, [animateOut, onClose]);components/TabScreenScrollView.tsx (1)
16-17: HardcodedBANNER_HEIGHTmay drift from actual banner styling.If the banner component's padding or content height changes, this constant could become incorrect, causing layout misalignment. Consider deriving this from the actual banner component or using a shared constant.
hooks/useLiveClasses.ts (2)
32-35: Minor:normalizecreates intermediate object that's immediately destructured.The function creates
{id, data}then immediately maps to[s.id, s.data]. This works but is slightly redundant.♻️ Simplified version
- const normalize = (s: Session) => ({ - id: String(s.rejectID || s.activityID), - data: JSON.stringify(s), - }); - - const prevSessions = new Map((prev.sessions ?? []).map(normalize).map(s => [s.id, s.data])); + const toEntry = (s: Session): [string, string] => [ + String(s.rejectID || s.activityID), + JSON.stringify(s), + ]; + + const prevSessions = new Map((prev.sessions ?? []).map(toEntry));
299-301: Silent error swallowing may hide issues.While the comment explains the intent, silently catching all errors during the post-submission fetch makes debugging harder. Consider at least logging in development mode.
♻️ Add development logging
} catch { - // Silently fail - will show success anyway and retry on next poll + // Silently fail in production - will show success anyway and retry on next poll + if (__DEV__) { + console.debug('Post-submission fetch failed, will retry on next poll'); + } }utils/submission.ts (1)
112-115: WebSocket error handler discards error details.The
onerrorhandler receives an error event but only rejects with a generic message. Including available error details would help debugging connection issues.♻️ Include error details if available
- socket.onerror = error => { + socket.onerror = (event) => { cleanup(); - reject(new Error('WebSocket connection error')); + reject(new Error(`WebSocket connection error${event.message ? `: ${event.message}` : ''}`)); };components/checkout/Confetti.tsx (2)
4-4: Static screen dimensions won't update on orientation change.Same issue as in
BottomSheet.tsx-Dimensions.get('window')is called once at module load. Consider usinguseWindowDimensionshook inside the component for dynamic sizing.
105-105:onCompletein dependencies may cause unnecessary re-creation.If the parent passes an inline
onCompletecallback withoutuseCallback, every parent render will reset the confetti animation. Consider documenting this expectation or making the effect more resilient.components/checkout/UndoModal.tsx (3)
18-18: Consider addingonRequestClosehandler for Android back button.The Modal component should handle the Android back button press for better UX. Without
onRequestClose, pressing the back button on Android may not behave as expected.📱 Recommended enhancement for Android support
- <Modal visible={visible} transparent animationType="fade"> + <Modal visible={visible} transparent animationType="fade" onRequestClose={onCancel}>This ensures that pressing the Android back button triggers the cancel action.
44-44: Hard-coded color for destructive action.The red color
#FF3B30is hard-coded for the Undo button. While this is Apple's standard destructive action color and appropriate for the context, consider defining it in the theme or constants for consistency.
14-56: Consider adding accessibility labels.The modal buttons would benefit from accessibility labels for screen readers, especially since this is a destructive action that users should understand clearly.
♿ Proposed accessibility enhancement
<TouchableOpacity style={[styles.cancelButton, { backgroundColor: isDark ? '#3A3A3C' : '#E5E5EA' }]} onPress={onCancel} - activeOpacity={0.7}> + activeOpacity={0.7} + accessibilityRole="button" + accessibilityLabel="Cancel undo operation"> <ThemedText style={styles.cancelButtonText}>Cancel</ThemedText> </TouchableOpacity> <TouchableOpacity style={[styles.confirmButton, { backgroundColor: '#FF3B30' }]} onPress={onConfirm} - activeOpacity={0.7}> + activeOpacity={0.7} + accessibilityRole="button" + accessibilityLabel="Confirm undo submission"> <ThemedText style={styles.confirmButtonText}>Undo</ThemedText> </TouchableOpacity>components/checkout/UndoBanner.tsx (1)
61-67: Consider adding accessibility support.The Undo button would benefit from accessibility properties to improve screen reader support.
♿ Proposed accessibility enhancement
{showUndoButton && onUndo && ( <TouchableOpacity onPress={onUndo} - style={[styles.undoButton, { borderColor: successText }]}> + style={[styles.undoButton, { borderColor: successText }]} + accessibilityRole="button" + accessibilityLabel="Undo this action"> <ThemedText style={[styles.undoText, { color: successText }]}>Undo</ThemedText> </TouchableOpacity> )}components/checkout/SubmissionProgress.tsx (1)
94-96: Consider adding accessibility for the loading state.The ActivityIndicator would benefit from an accessibility label to inform screen reader users about the ongoing operation.
♿ Proposed accessibility enhancement
{!isError && !isSuccess && ( - <ActivityIndicator size="small" color={theme.primary} style={styles.spinner} /> + <ActivityIndicator + size="small" + color={theme.primary} + style={styles.spinner} + accessibilityLabel="Submission in progress" /> )}components/checkout/CodeInput.tsx (4)
34-36: Potential unnecessary re-renders fromonValueChangein dependencies.Including
onValueChangein the dependency array may cause unnecessary re-renders if the parent component doesn't memoize this callback. Consider usinguseCallbackin the parent or wrapping this effect.⚡ Recommended optimization
Option 1: Exclude from dependencies (if parent guarantees stable reference):
useEffect(() => { onValueChange?.(code.length > 0); - }, [code, onValueChange]); + }, [code]); // eslint-disable-line react-hooks/exhaustive-depsOption 2: Use
useRefto store the latest callback:+ const onValueChangeRef = useRef(onValueChange); + useEffect(() => { + onValueChangeRef.current = onValueChange; + }); + useEffect(() => { - onValueChange?.(code.length > 0); + onValueChangeRef.current?.(code.length > 0); - }, [code, onValueChange]); + }, [code]);Note: The parent component should ideally memoize callback props with
useCallback.
63-90: Missing accessibility properties for the input field.The TextInput would significantly benefit from accessibility labels and hints to support screen readers and improve the experience for users with disabilities.
♿ Recommended accessibility enhancement
<TextInput ref={inputRef} style={[...]} value={code} onChangeText={handleChange} onFocus={handleFocus} onBlur={handleBlur} placeholder={placeholder} placeholderTextColor={theme.secondary} maxFontSizeMultiplier={1.2} keyboardType="number-pad" maxLength={6} editable={!isSubmitting && !disabled} autoCorrect={false} returnKeyType="done" onSubmitEditing={handleSubmit} + accessibilityLabel="Enter 6-digit code" + accessibilityHint="Enter a 6-digit numeric code for submission" + accessibilityRole="none" />
92-110: Submit button should have accessibility properties.The submit button needs proper accessibility configuration, especially since its state changes between enabled/disabled and loading/ready.
♿ Recommended accessibility enhancement
<TouchableOpacity style={[...]} onPress={handleSubmit} disabled={!isValid || isSubmitting || disabled} - activeOpacity={0.7}> + activeOpacity={0.7} + accessibilityRole="button" + accessibilityLabel={isSubmitting ? "Submitting code" : "Submit code"} + accessibilityState={{ disabled: !isValid || isSubmitting || disabled }}> {isSubmitting ? ( <ActivityIndicator color={theme.ctaText} size="small" /> ) : (
29-29: Unused ref could support imperative focus control.The
inputRefis declared but not exposed to the parent. If programmatic focus control is needed (e.g., auto-focus on mount or after an action), consider usingforwardRefor exposing a focus method.📋 Example using forwardRef (if needed)
If the parent needs to control focus:
-export function CodeInput({ +export const CodeInput = React.forwardRef<TextInput, CodeInputProps>(function CodeInput({ onSubmit, onFocus, onBlur, onValueChange, isSubmitting = false, disabled = false, placeholder = '6-digit code', -}: CodeInputProps) { +}, ref) { const { theme, isDark } = useAppTheme(); const [code, setCode] = useState(''); const [isFocused, setIsFocused] = useState(false); - const inputRef = useRef<TextInput>(null); + const inputRef = useRef<TextInput>(null); + + React.useImperativeHandle(ref, () => inputRef.current!);This allows parent components to call
inputRef.current?.focus().components/checkout/OptionsSheet.tsx (3)
33-40: Verify error handling in external link construction.The function accesses
session.codes?.[0]andprimaryCode.codeIDs?.[0]safely with optional chaining. However, ifcodeIDsis an empty array, this will pass an empty string to the URL, which may not be the desired behavior.🔍 Improved validation
const handleReportCode = () => { const primaryCode = session.codes?.[0]; - if (primaryCode) { + if (primaryCode?.codeIDs?.[0]) { - const codeId = primaryCode.codeIDs?.[0] || ''; + const codeId = primaryCode.codeIDs[0]; openExternalLink(`/support?pre=report-code&chc=${primaryCode.checkinCode}&codeID=${codeId}`); + } else { + console.warn('Cannot report code: missing code ID'); + // Optionally show user feedback } onClose(); };
42-45: Navigation timing after sheet close.Calling
router.push('/settings')immediately afteronClose()might cause navigation to occur while the sheet is still animating out. This could lead to visual glitches or unexpected behavior.⏱️ Recommended timing fix
Based on the BottomSheet component from the context (which uses a 250ms animation), add a delay:
const handleOpenSettings = () => { onClose(); - router.push('/settings'); + setTimeout(() => { + router.push('/settings'); + }, 300); // Wait for sheet close animation (250ms + buffer) };Alternatively, if the BottomSheet's
onClosecallback fires after animation completion, you could pass the navigation as a callback.
151-151: Consider using a shared time formatting utility instead of inline.slice(0, 5).The type definition guarantees
startTimeis either"HH:MM"or"HH:MM:SS", so.slice(0, 5)correctly extracts"HH:MM"in both cases and is safe. However,SessionCard.tsxalready has aformatTime()function that parses time strings more robustly. For consistency and maintainability, extract the time display logic into a shared utility rather than using inline slicing.components/checkout/VerificationSheet.tsx (2)
16-22: Early return bypasses BottomSheet wrapper.When
codeis null, returningnullat line 21 means the component renders nothing even whenvisibleis true. This could cause a flash or inconsistency since the parent controls visibility but the sheet content disappears. Consider moving the null check inside the BottomSheet or letting BottomSheet handle empty content gracefully.💡 Suggested approach
export function VerificationSheet({ visible, onClose, code }: VerificationSheetProps) { const { theme, isDark } = useAppTheme(); const toast = useToast(); const [copied, setCopied] = useState(false); - if (!code) return null; + if (!code) { + return <BottomSheet visible={visible} onClose={onClose}><View /></BottomSheet>; + }Alternatively, have the parent component control visibility based on code availability.
27-32: Missing timeout cleanup on unmount.The
setTimeoutat line 31 can trigger a state update after the component unmounts, causing a React warning. Consider using a ref or cleanup inuseEffect.♻️ Suggested fix
+import React, { useState, useEffect, useRef } from 'react'; // ... export function VerificationSheet({ visible, onClose, code }: VerificationSheetProps) { const { theme, isDark } = useAppTheme(); const toast = useToast(); const [copied, setCopied] = useState(false); + const timeoutRef = useRef<NodeJS.Timeout | null>(null); + + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []); // ... const handleCopy = async () => { await Clipboard.setStringAsync(String(code.checkinCode)); setCopied(true); toast.success('Code copied to clipboard'); - setTimeout(() => setCopied(false), 2000); + timeoutRef.current = setTimeout(() => setCopied(false), 2000); };components/checkout/SessionCard.tsx (2)
152-164: Non-null assertions onprimaryCodeare safe but could be clearer.The
!assertions on lines 154, 155, and 160 rely on theshouldShowInputcondition (line 94) which checkshasCode. SincehasCodeverifiesprimaryCodeexists, this is safe. However, consider using a type guard or narrowing to make this more explicit and avoid the need for assertions.💡 Alternative approach
- ) : ( + ) : primaryCode ? ( <CodeDisplay - code={primaryCode!} - onConfirm={() => onConfirmCode(primaryCode!)} + code={primaryCode} + onConfirm={() => onConfirmCode(primaryCode)} onViewOtherCodes={ otherCodes.length > 0 ? () => setShowOtherCodes(!showOtherCodes) : undefined } otherCodesCount={otherCodes.length} - showConfirmButton={!primaryCode!.isResponsible} + showConfirmButton={!primaryCode.isResponsible} isConfirming={isSubmitting} showCopyToast={false} /> + ) : null}
170-185: Consider usingcheckinCodealone as the key.The
checkinCodeshould be unique within a session's codes array, making the index suffix unnecessary. Using index in keys can cause issues with list reordering, though in this case it's mitigated by the unique code prefix.- <TouchableOpacity - key={code.checkinCode + index} + <TouchableOpacity + key={code.checkinCode}components/checkout/CodeDisplay.tsx (1)
60-67: Missing timeout cleanup on unmount.Same issue as in
VerificationSheet.tsx- thesetTimeoutat line 66 can cause a state update on an unmounted component. Consider using a cleanup pattern withuseEffector a ref.♻️ Suggested fix
-import React, { useState } from 'react'; +import React, { useState, useEffect, useRef } from 'react'; // ... export function CodeDisplay({ /* props */ }: CodeDisplayProps) { const { theme, isDark } = useAppTheme(); const toast = useToast(); const [copied, setCopied] = useState(false); + const copyTimeoutRef = useRef<NodeJS.Timeout | null>(null); + + useEffect(() => { + return () => { + if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); + }; + }, []); // ... const handleCopy = async () => { await Clipboard.setStringAsync(String(code.checkinCode)); setCopied(true); if (showCopyToast) { toast.success('Code copied to clipboard'); } - setTimeout(() => setCopied(false), 2000); + copyTimeoutRef.current = setTimeout(() => setCopied(false), 2000); };app/(tabs)/settings.tsx (3)
37-55:useFocusEffecthas incomplete dependency array.The
loadCurrentApiUrlfunction usessetCurrentResolvedApiUrlbut it's not included in the dependency array. While React's rules of hooks typically flag this,useFocusEffectfrom React Navigation may not have the same lint integration. The current implementation works becausesetCurrentResolvedApiUrlis a stable setter, but consider adding it for completeness.
57-64: Avoid silently swallowing navigation errors.The empty
catchblock at line 63 suppresses all errors fromrouter.push. If navigation fails (e.g., during race conditions or invalid state), this will hide the issue. Consider logging the error at minimum.♻️ Suggested fix
const handleLogout = async () => { clearAccountInfo(); await logout(); toast.success('Logged out successfully'); try { router.push({ pathname: '/(tabs)' }); - } catch {} + } catch (error) { + console.warn('Navigation after logout failed:', error); + } };
311-365: Consider extracting modal styles to StyleSheet.The web-only modals have extensive inline styles. For consistency with the rest of the codebase and minor performance benefits (style objects won't be recreated on each render), consider moving these to the StyleSheet.
types/liveClasses.ts (1)
11-23: Union type'user' | 'outsource-aci' | stringis redundant.The
sourcefield's type includes specific string literals followed by| string, which makes the literals redundant for type checking (any string will satisfy the type). If the intent is to document expected values while allowing others, consider using a branded type or JSDoc comment instead.💡 Alternative approaches
Option 1: Remove the fallback if only these values are valid:
- source: 'user' | 'outsource-aci' | string; + source: 'user' | 'outsource-aci';Option 2: Keep string but document with JSDoc:
- source: 'user' | 'outsource-aci' | string; + /** Expected values: 'user', 'outsource-aci' */ + source: string;app/(tabs)/checkout.tsx (4)
231-234:setTimeoutforforceUpdatelacks cleanup.The
setTimeoutat line 233 (and similarly at line 262) schedules a state update after 30 seconds. If the component unmounts before then, this will cause a React warning. Consider tracking and cleaning up these timeouts.♻️ Suggested approach
+ const forceUpdateTimeoutsRef = useRef<Set<NodeJS.Timeout>>(new Set()); + + useEffect(() => { + return () => { + forceUpdateTimeoutsRef.current.forEach(clearTimeout); + }; + }, []); // In handleSubmitCode: - setTimeout(() => forceUpdate(n => n + 1), 30000); + const timeout = setTimeout(() => forceUpdate(n => n + 1), 30000); + forceUpdateTimeoutsRef.current.add(timeout);
553-570: MultipleundoBanners.get()calls could be consolidated.The same
undoBanners.get(session.rejectID)is called multiple times (lines 556, 558, 566). Consider caching the result to avoid repeated Map lookups and improve readability.♻️ Suggested refactor
{!isSubmittingThisSession && hasUndoBanner && (() => { + const bannerInfo = undoBanners.get(session.rejectID)!; const submissionTime = submissionTimestampsRef.current.get( session.rejectID ); // ... message logic ... return ( <UndoBanner message={message} - showUndoButton={undoBanners.get(session.rejectID)!.showUndo} + showUndoButton={bannerInfo.showUndo} onUndo={ - undoBanners.get(session.rejectID)!.tk - ? () => - handleUndoPress( - session.rejectID, - undoBanners.get(session.rejectID)!.tk! - ) + bannerInfo.tk + ? () => handleUndoPress(session.rejectID, bannerInfo.tk!) : undefined } - onDismiss={() => dismissUndoBanner(session.rejectID)} + onDismiss={() => dismissUndoBanner(session.rejectID)} connected /> ); })()}
31-68: Consider splitting this large component in future iterations.The
CheckoutScreencomponent (~670 lines) handles many responsibilities: session display, code submission, undo flows, multiple modals, keyboard/scroll management, and polling. While the code is well-organized, future maintainability would benefit from extracting logic into custom hooks (e.g.,useUndoFlow,useKeyboardScroll) or splitting UI into sub-components.
42-42: TheforceUpdatepattern is acceptable but consider alternatives.Using
useStatejust for triggering re-renders (line 42) works, but the intent isn't immediately clear. Consider using a more descriptive state likelastRenderTimeor adding a comment explaining the purpose.- const [, forceUpdate] = useState(0); + // Force re-render to update time-sensitive UI (e.g., "Submitted!" message duration) + const [, forceUpdate] = useState(0);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
app/(tabs)/_layout.tsxapp/(tabs)/checkout.tsxapp/(tabs)/history/index.tsxapp/(tabs)/index.tsxapp/(tabs)/settings.tsxapp/_layout.tsxapp/styles/autocheckin.styles.tscomponents/TabScreenScrollView.tsxcomponents/checkout/AutoSplashModal.tsxcomponents/checkout/BottomSheet.tsxcomponents/checkout/CodeDisplay.tsxcomponents/checkout/CodeInput.tsxcomponents/checkout/Confetti.tsxcomponents/checkout/OptionsSheet.tsxcomponents/checkout/SessionCard.tsxcomponents/checkout/ShareSheet.tsxcomponents/checkout/SubmissionProgress.tsxcomponents/checkout/UndoBanner.tsxcomponents/checkout/UndoModal.tsxcomponents/checkout/VerificationSheet.tsxconstants/api.tshooks/useCourseConfig.tshooks/useLiveClasses.tshooks/useToast.tstypes/liveClasses.tsutils/crypto.tsutils/submission.ts
💤 Files with no reviewable changes (1)
- hooks/useToast.ts
🧰 Additional context used
🧬 Code graph analysis (20)
components/checkout/ShareSheet.tsx (6)
types/liveClasses.ts (1)
Session(26-42)hooks/useAppTheme.ts (1)
useAppTheme(4-12)hooks/useToast.ts (1)
useToast(12-31)utils/submission.ts (1)
generateShareLink(380-424)components/checkout/BottomSheet.tsx (1)
BottomSheet(31-128)components/ThemedText.tsx (1)
ThemedText(11-34)
components/checkout/UndoBanner.tsx (2)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)components/ThemedText.tsx (1)
ThemedText(11-34)
components/checkout/UndoModal.tsx (2)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)components/ThemedText.tsx (1)
ThemedText(11-34)
components/checkout/SessionCard.tsx (7)
types/liveClasses.ts (2)
Session(26-42)Code(12-23)hooks/useAppTheme.ts (1)
useAppTheme(4-12)constants/api.ts (1)
getApiUrl(8-16)components/ThemedView.tsx (1)
ThemedView(10-14)components/ThemedText.tsx (1)
ThemedText(11-34)components/checkout/CodeInput.tsx (1)
CodeInput(17-120)components/checkout/CodeDisplay.tsx (1)
CodeDisplay(19-141)
components/checkout/SubmissionProgress.tsx (2)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)components/ThemedText.tsx (1)
ThemedText(11-34)
components/checkout/OptionsSheet.tsx (5)
types/liveClasses.ts (2)
Session(26-42)Code(12-23)hooks/useAppTheme.ts (1)
useAppTheme(4-12)constants/api.ts (1)
getApiUrl(8-16)components/checkout/BottomSheet.tsx (1)
BottomSheet(31-128)components/ThemedText.tsx (1)
ThemedText(11-34)
app/_layout.tsx (2)
constants/Colors.ts (1)
Colors(34-83)contexts/ConnectionContext.tsx (1)
ConnectionProvider(35-121)
hooks/useCourseConfig.ts (2)
types/liveClasses.ts (1)
CourseConfig(64-68)utils/storage.ts (1)
storage(33-36)
components/checkout/CodeInput.tsx (3)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)utils/submission.ts (1)
validateCode(353-375)components/ThemedText.tsx (1)
ThemedText(11-34)
components/checkout/CodeDisplay.tsx (4)
types/liveClasses.ts (1)
Code(12-23)hooks/useAppTheme.ts (1)
useAppTheme(4-12)hooks/useToast.ts (1)
useToast(12-31)components/ThemedText.tsx (1)
ThemedText(11-34)
hooks/useLiveClasses.ts (4)
types/liveClasses.ts (5)
LiveClassesResponse(52-61)Session(26-42)RefreshBlocker(130-130)SubmissionState(120-127)SubmissionData(71-76)hooks/useCourseConfig.ts (1)
getCourseApiPath(109-112)utils/api.ts (1)
checkoutApi(20-150)utils/submission.ts (1)
validateCode(353-375)
app/styles/autocheckin.styles.ts (1)
constants/Colors.ts (1)
Colors(34-83)
components/checkout/VerificationSheet.tsx (5)
types/liveClasses.ts (1)
Code(12-23)hooks/useAppTheme.ts (1)
useAppTheme(4-12)hooks/useToast.ts (1)
useToast(12-31)components/checkout/BottomSheet.tsx (1)
BottomSheet(31-128)components/ThemedText.tsx (1)
ThemedText(11-34)
app/(tabs)/_layout.tsx (8)
hooks/useColorScheme.web.ts (1)
useColorScheme(7-21)hooks/useColorScheme.ts (1)
useColorScheme(1-1)components/TopBar.tsx (1)
TopBar(8-34)components/VersionAwareTabBar.tsx (1)
VersionAwareTabBar(7-16)components/HapticTab.tsx (1)
HapticTab(5-18)components/ui/CheckOutIcon.tsx (1)
CheckOutIcon(8-17)components/ConnectionErrorBanner.tsx (1)
ConnectionErrorBanner(12-144)components/UnavailableOverlay.tsx (1)
UnavailableOverlay(8-163)
components/TabScreenScrollView.tsx (1)
contexts/ConnectionContext.tsx (1)
useConnectionContext(123-129)
utils/submission.ts (4)
types/liveClasses.ts (5)
SubmitOptionsResponse(79-85)SubmissionData(71-76)ProgressCallback(117-117)SubmissionResult(88-93)AutoSplashResult(110-114)constants/api.ts (1)
getApiUrl(8-16)utils/storage.ts (1)
storage(33-36)utils/crypto.ts (4)
generateSubmissionHashes(103-130)btoa(41-67)getUserAgent(135-140)atob(72-97)
app/(tabs)/settings.tsx (6)
hooks/useAuth.ts (1)
useAuth(14-226)hooks/useSettingsAccount.ts (1)
useSettingsAccount(12-88)hooks/useApiUrl.ts (1)
useApiUrl(4-69)constants/api.ts (1)
getApiUrl(8-16)components/ThemedText.tsx (1)
ThemedText(11-34)components/ThemedView.tsx (1)
ThemedView(10-14)
components/checkout/BottomSheet.tsx (1)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)
constants/api.ts (1)
utils/storage.ts (1)
storage(33-36)
components/checkout/AutoSplashModal.tsx (4)
types/liveClasses.ts (1)
AutoSplashResult(110-114)hooks/useAppTheme.ts (1)
useAppTheme(4-12)utils/submission.ts (1)
connectAutoSplash(271-347)components/ThemedText.tsx (1)
ThemedText(11-34)
🔇 Additional comments (31)
app/(tabs)/history/index.tsx (1)
161-235: LGTM!The change from React fragments to explicit
<View>wrappers is a reasonable structural adjustment. This provides a concrete DOM element for potential styling or layout requirements while maintaining the same rendering behavior.app/_layout.tsx (2)
26-50: Toast configuration looks good.The
useMemodependency oncolorSchemeensures toast styles update correctly when the theme changes. ThedisplayNameassignment is a nice touch for debugging.
77-89: Provider structure is well-organized.The hierarchy correctly places
GestureHandlerRootViewat the root for gesture support,ConnectionProviderfor app-wide connection state, andThemeProviderfor navigation theming. TheToastcomponent receives its configuration and is properly positioned within the theme context.components/checkout/ShareSheet.tsx (2)
40-54: Link regenerated on every sheet open.Since the reset effect (lines 47-54) clears
shareLinkwhen the sheet closes, a new API call togenerateShareLinkwill be made every time the sheet opens. If this is intentional for freshness, the current implementation is correct. If caching the link per session is preferred, consider storing the link in parent state or removing the reset.
81-174: Component structure is well-organized.The conditional rendering for loading, error, and success states is clear and follows the established patterns in the codebase. The integration with
BottomSheetand theming is consistent with other components.app/styles/autocheckin.styles.ts (2)
10-22: Platform-specific shadow helper is well-implemented.The
surfaceShadowfunction correctly handles iOS shadow properties and Android elevation, with appropriate theming adjustments for dark mode. The nullish coalescing fallback ensures a safe default.
24-220: Comprehensive style definitions for Auto Checkin UI.The factory function provides good theme integration and the styles are well-organized. A few inline color values (e.g.,
'#E7F5E9'on line 199,'#27AE60'references) could potentially be centralized in the Colors constant for consistency, but this is acceptable for a styles module.constants/api.ts (1)
43-47: Good backward compatibility approach.The synchronous
CHECKOUT_API_URLexport with the comment explaining the async alternative provides a clear migration path for existing code while enabling the new dynamic URL functionality.hooks/useCourseConfig.ts (2)
56-62: Potential stale closure in rapid successive calls.If
updateFieldis called multiple times in quick succession, each call captures the currentconfigstate at callback creation time. Subsequent calls may overwrite changes from previous calls before they're persisted. Consider using a functional update pattern if this is a concern.If rapid updates are a use case, consider this pattern:
const updateField = useCallback( async (field: keyof CourseConfig, value: string) => { // Get latest config from storage to avoid stale state const currentConfig = await getCourseConfig(); const newConfig = { ...currentConfig, [field]: value }; await setConfig(newConfig); }, [setConfig] );
18-86: Well-structured hook with comprehensive API.The hook provides a clean interface for managing course configuration with proper persistence, loading states, and memoized callbacks. The
isDefaultflag andgetApiPathhelper are useful additions for consumers.components/checkout/BottomSheet.tsx (1)
64-84: LGTM!The pan gesture implementation with threshold and velocity-based dismiss logic is well-structured. Good use of
runOnJSfor the close callback from the worklet.components/TabScreenScrollView.tsx (2)
42-53: Scroll reset on focus may cause unexpected UX.This resets scroll position to top whenever the tab gains focus on iOS. If users scroll down, switch tabs, and return, they'll lose their position. If this is intentional to fix a specific iOS content inset bug, consider adding a comment explaining the specific issue being addressed. Otherwise, users may find this behavior frustrating.
23-23: No issues found. The component is properly wrapped byConnectionProviderat the root layout level, andTabScreenScrollViewis only used in tab screens that are guaranteed to render within the provider. The risk of rendering outside the provider context does not exist in this application architecture.hooks/useLiveClasses.ts (1)
229-373: LGTM!The submission flow is well-structured with clear progress states, proper error handling, grace periods to prevent refresh conflicts, and optimistic UI updates. Good alignment with the documented web version behavior.
utils/submission.ts (2)
271-347: LGTM!The
connectAutoSplashfunction handles timeout and connection close gracefully by treating them as "still processing" rather than errors. Good pattern of returning a cleanup function for the consumer to manage the WebSocket lifecycle.
353-375: LGTM!Validation logic is thorough with clear error messages for each rule. Good defensive check for non-string input.
app/(tabs)/index.tsx (1)
1-5: LGTM!Clean redirect implementation using expo-router's
Redirectcomponent to route the default tab index to the checkout screen.components/checkout/Confetti.tsx (1)
95-98: LGTM!Good use of
Animated.parallelto coordinate all particle animations with a single completion callback. The cleanup logic properly stops animations on unmount.components/checkout/UndoBanner.tsx (1)
30-46: Animation dependencies are complete.The
useEffectcorrectly includes all necessary dependencies (animate,slideAnim,opacityAnim). The animation will properly trigger when needed.components/checkout/SubmissionProgress.tsx (3)
27-41: Entry animation setup is correct.The parallel slide-in and fade-in animations on mount are well-implemented with appropriate spring and timing configurations. Dependencies are properly declared.
43-49: Progress animation correctly usesuseNativeDriver: false.The
useNativeDriver: falsesetting is necessary here because the animation interpolates thewidthproperty, which is not supported by the native driver. This is the correct approach.
56-66: Well-structured conditional color logic.The nested ternary for
backgroundColor,textColor, andprogressBarColorclearly handles error, success, and default states. The logic is readable and maintains theme consistency.components/checkout/OptionsSheet.tsx (2)
60-73: Options correctly gated byprimaryCodeexistence.The conditional rendering ensures action options only appear when a code exists. This prevents errors from accessing undefined code properties and provides a sensible UX.
118-127: Collapsible metadata section is well-implemented.The toggle for showing/hiding additional session information provides a clean UX and reduces visual clutter. The implementation correctly manages the
showMoreInfostate.components/checkout/VerificationSheet.tsx (1)
166-169: Verify "Trust Score" label semantics.The
rejectScorefield is displayed as "Trust Score". Based on the field name, a higherrejectScoremight indicate lower trust (more rejections). Verify this label accurately represents the metric to avoid user confusion.app/(tabs)/_layout.tsx (1)
42-101: LGTM!The tab layout structure with connection banners, version-aware tab bar, and refresh-on-press listeners is well-organized. The pattern of using
navigation.setParams({ refresh: Date.now() })is consistent across tabs.components/checkout/SessionCard.tsx (1)
65-77: LGTM!The time formatting logic handles 12-hour conversion and AM/PM correctly. The code relies on the API providing valid time strings in "HH:MM" format as defined in the
Sessioninterface.components/checkout/CodeDisplay.tsx (1)
36-58: LGTM!The
getSocialProofTextfunction has well-structured logic with proper pluralization (line 47 correctly usescount > 2to determine when to pluralize "other"). The fallback to "Submitted by community" provides a sensible default.app/(tabs)/settings.tsx (1)
82-98: Good security practice with the API URL warning.The confirmation dialog appropriately warns users about social engineering risks before allowing API URL changes. This is an important safeguard for a feature that could be exploited by malicious actors.
types/liveClasses.ts (1)
1-160: LGTM!The type definitions are comprehensive and well-documented. The interfaces provide clear contracts for the Live Classes API responses, submission states, and UI state management.
app/(tabs)/checkout.tsx (1)
126-184: LGTM!The
formatNextSessionfunction provides user-friendly time descriptions with natural language (e.g., "at half past three tomorrow afternoon"). The handling of today/tomorrow/weekday cases is comprehensive.
| // Connect to WebSocket | ||
| useEffect(() => { | ||
| if (visible && jwt) { | ||
| setStatus('connecting'); | ||
| setMessage('Connecting to AutoCheckin...'); | ||
| setResult(null); | ||
|
|
||
| const connect = async () => { | ||
| cleanupRef.current = await connectAutoSplash( | ||
| jwt, | ||
| wsResult => { | ||
| setResult(wsResult); | ||
| if (wsResult.checkedin) { | ||
| setStatus('success'); | ||
| setMessage(wsResult.msg || 'Successfully checked in!'); | ||
| } else { | ||
| setStatus('waiting'); | ||
| setMessage(wsResult.msg || 'Processing...'); | ||
| } | ||
|
|
||
| // Auto close after delay | ||
| setTimeout(() => { | ||
| onComplete(wsResult); | ||
| }, 2000); | ||
| }, | ||
| error => { | ||
| setStatus('error'); | ||
| setMessage(error); | ||
| setResult({ | ||
| success: false, | ||
| checkedin: false, | ||
| msg: error, | ||
| }); | ||
|
|
||
| // Auto close after delay | ||
| setTimeout(() => { | ||
| onComplete({ | ||
| success: false, | ||
| checkedin: false, | ||
| msg: error, | ||
| }); | ||
| }, 2000); | ||
| } | ||
| ); | ||
|
|
||
| setStatus('waiting'); | ||
| setMessage('Checking your attendance...'); | ||
| }; | ||
|
|
||
| connect(); | ||
|
|
||
| return () => { | ||
| cleanupRef.current?.(); | ||
| }; | ||
| } | ||
| }, [visible, jwt, onComplete]); |
There was a problem hiding this comment.
Potential race condition and dependency issue.
Two concerns with this effect:
-
Race condition (lines 97-99): The
setStatus('waiting')call afterawait connectAutoSplashmay overwrite status already set by theonResultcallback if the WebSocket responds very quickly before line 98 executes. -
onCompletein dependency array: If the parent component doesn't memoizeonComplete, this effect will re-run on every render, causing connection cleanup and reconnection cycles.
🐛 Suggested fix
// Connect to WebSocket
useEffect(() => {
if (visible && jwt) {
setStatus('connecting');
setMessage('Connecting to AutoCheckin...');
setResult(null);
+ let isMounted = true;
+
const connect = async () => {
+ // Set waiting status before connecting to avoid race
+ setStatus('waiting');
+ setMessage('Checking your attendance...');
+
cleanupRef.current = await connectAutoSplash(
jwt,
wsResult => {
+ if (!isMounted) return;
setResult(wsResult);
if (wsResult.checkedin) {
setStatus('success');
setMessage(wsResult.msg || 'Successfully checked in!');
} else {
- setStatus('waiting');
setMessage(wsResult.msg || 'Processing...');
}
// Auto close after delay
setTimeout(() => {
- onComplete(wsResult);
+ if (isMounted) onComplete(wsResult);
}, 2000);
},
error => {
+ if (!isMounted) return;
setStatus('error');
setMessage(error);
// ... rest of error handling
}
);
-
- setStatus('waiting');
- setMessage('Checking your attendance...');
};
connect();
return () => {
+ isMounted = false;
cleanupRef.current?.();
};
}
- }, [visible, jwt, onComplete]);
+ }, [visible, jwt]); // Remove onComplete - use ref pattern if neededFor onComplete, consider using a ref to hold the latest callback value to avoid re-running the effect:
const onCompleteRef = useRef(onComplete);
useEffect(() => { onCompleteRef.current = onComplete; }, [onComplete]);
// Then use onCompleteRef.current(...) in callbacks🤖 Prompt for AI Agents
In @components/checkout/AutoSplashModal.tsx around lines 53 - 108, The effect
can race and re-run unnecessarily: move the setStatus('waiting') and
setMessage('Checking your attendance...') to before the await call so they don't
overwrite a fast websocket response (i.e., call them immediately inside
useEffect before invoking connectAutoSplash), and stop putting onComplete in the
effect deps by storing the latest callback in a ref (create onCompleteRef =
useRef(onComplete) and a small useEffect to keep onCompleteRef.current =
onComplete), then call onCompleteRef.current(...) inside both wsResult and error
callbacks; keep cleanupRef usage and keep visible/jwt in the dependency array
but remove onComplete.
| backgroundColor: particle.color, | ||
| borderRadius: Math.random() > 0.5 ? particle.size / 2 : 2, | ||
| transform: [ |
There was a problem hiding this comment.
Math.random() in render causes shape to change on re-renders.
The borderRadius uses Math.random() during render, so particles may randomly change between circular and rectangular shapes if the component re-renders during animation. Move this decision to particle creation time.
🐛 Proposed fix
Update the Particle interface and creation:
interface Particle {
// ... existing fields
color: string;
size: number;
+ isCircle: boolean;
}In particle creation (around line 42-50):
particlesRef.current = Array.from({ length: PARTICLE_COUNT }, () => ({
// ... existing fields
color: CONFETTI_COLORS[Math.floor(Math.random() * CONFETTI_COLORS.length)],
size: Math.random() * 10 + 5,
+ isCircle: Math.random() > 0.5,
}));In render:
- borderRadius: Math.random() > 0.5 ? particle.size / 2 : 2,
+ borderRadius: particle.isCircle ? particle.size / 2 : 2,📝 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.
| backgroundColor: particle.color, | |
| borderRadius: Math.random() > 0.5 ? particle.size / 2 : 2, | |
| transform: [ | |
| backgroundColor: particle.color, | |
| borderRadius: particle.isCircle ? particle.size / 2 : 2, | |
| transform: [ |
🤖 Prompt for AI Agents
In @components/checkout/Confetti.tsx around lines 119 - 121, The render uses
Math.random() to set borderRadius causing particles to flip shape on re-renders;
add a new property on the Particle type (e.g., borderRadius or isRounded) and
assign its value once when particles are created (inside the particle
creation/init function you added around the ~line 42–50), then replace the
inline Math.random() in Confetti's render with that particle property (use
particle.borderRadius or conditional based on particle.isRounded) so shape is
deterministic after creation.
| const openExternalLink = async (path: string) => { | ||
| const apiUrl = await getApiUrl(); | ||
| Linking.openURL(`${apiUrl}${path}`); | ||
| }; |
There was a problem hiding this comment.
Missing error handling for async openExternalLink.
The openExternalLink function doesn't handle potential errors from getApiUrl() or Linking.openURL(). If the URL is malformed or the app doesn't have permission to open URLs, this could fail silently.
🛡️ Recommended error handling
const openExternalLink = async (path: string) => {
- const apiUrl = await getApiUrl();
- Linking.openURL(`${apiUrl}${path}`);
+ try {
+ const apiUrl = await getApiUrl();
+ const url = `${apiUrl}${path}`;
+ const supported = await Linking.canOpenURL(url);
+
+ if (supported) {
+ await Linking.openURL(url);
+ } else {
+ console.error('Cannot open URL:', url);
+ // Optionally show user feedback
+ }
+ } catch (error) {
+ console.error('Error opening external link:', error);
+ // Optionally show user feedback
+ }
};📝 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.
| const openExternalLink = async (path: string) => { | |
| const apiUrl = await getApiUrl(); | |
| Linking.openURL(`${apiUrl}${path}`); | |
| }; | |
| const openExternalLink = async (path: string) => { | |
| try { | |
| const apiUrl = await getApiUrl(); | |
| const url = `${apiUrl}${path}`; | |
| const supported = await Linking.canOpenURL(url); | |
| if (supported) { | |
| await Linking.openURL(url); | |
| } else { | |
| console.error('Cannot open URL:', url); | |
| // Optionally show user feedback | |
| } | |
| } catch (error) { | |
| console.error('Error opening external link:', error); | |
| // Optionally show user feedback | |
| } | |
| }; |
🤖 Prompt for AI Agents
In @components/checkout/OptionsSheet.tsx around lines 28 - 31, The
openExternalLink function lacks error handling for failures from getApiUrl() and
Linking.openURL(); wrap the async work in a try/catch inside openExternalLink
(reference function openExternalLink) and await Linking.openURL(path) so errors
are caught, then handle errors by logging them (use your logger or
console.error) and surface a user-friendly fallback (e.g., show an Alert or
toast) so failures don’t fail silently; ensure you validate or sanitize the
constructed URL before opening to avoid malformed-URL errors.
| message: string; | ||
| showUndoButton: boolean; | ||
| onUndo?: () => void; | ||
| onDismiss: () => void; |
There was a problem hiding this comment.
Unused onDismiss prop.
The onDismiss prop is declared in the interface but is never used in the component. Either implement dismiss functionality or remove this prop to avoid confusion.
🧹 Proposed fix: Remove unused prop
If dismiss functionality is not needed:
interface UndoBannerProps {
message: string;
showUndoButton: boolean;
onUndo?: () => void;
- onDismiss: () => void;
animate?: boolean;
connected?: boolean;
} export function UndoBanner({
message,
showUndoButton,
onUndo,
- onDismiss,
animate = true,
connected = true,
}: UndoBannerProps) {Alternatively, if auto-dismiss is desired, you could implement a timer that calls onDismiss after a few seconds.
🤖 Prompt for AI Agents
In @components/checkout/UndoBanner.tsx at line 10, The declared but unused
onDismiss prop on the UndoBanner component (in the UndoBannerProps interface and
props destructuring in UndoBanner) should be cleaned up: either remove onDismiss
from the UndoBannerProps type and from the component’s props usage and from any
callers that pass it, or implement dismissal behavior by wiring onDismiss to a
dismiss control (e.g., add a close button that calls props.onDismiss or add a
useEffect timer that calls props.onDismiss after a delay) and ensure the
component calls it where appropriate.
| // Initial fetch and polling setup | ||
| useEffect(() => { | ||
| fetchSessions(true); | ||
| startPolling(); | ||
|
|
||
| return () => { | ||
| stopPolling(); | ||
| if (graceTimerRef.current) { | ||
| clearTimeout(graceTimerRef.current); | ||
| } | ||
| }; | ||
| }, [fetchSessions, startPolling, stopPolling]); |
There was a problem hiding this comment.
Potential infinite loop due to fetchSessions in useEffect dependencies.
fetchSessions is in the dependency array and it's recreated whenever sessions.length changes (line 168). When fetchSessions runs and updates sessions, it gets a new identity, triggering the effect again, which calls fetchSessions(true) and startPolling().
This could cause rapid re-fetching on initial load. Consider stabilizing fetchSessions or using a ref-based approach for the initial fetch.
🐛 Suggested fix - separate initial fetch from polling setup
+ const fetchSessionsRef = useRef(fetchSessions);
+ fetchSessionsRef.current = fetchSessions;
+
// Initial fetch and polling setup
useEffect(() => {
- fetchSessions(true);
- startPolling();
+ // Use ref to avoid dependency on fetchSessions identity
+ fetchSessionsRef.current(true);
+
+ // Start polling after initial fetch
+ const pollTimer = setInterval(() => {
+ if (!shouldBlockRefresh()) {
+ fetchSessionsRef.current(false);
+ }
+ }, POLL_MS);
return () => {
- stopPolling();
+ clearInterval(pollTimer);
if (graceTimerRef.current) {
clearTimeout(graceTimerRef.current);
}
};
- }, [fetchSessions, startPolling, stopPolling]);
+ }, []); // Run once on mount| export function btoa(data: string): string { | ||
| if (Platform.OS === 'web') { | ||
| return window.btoa(data); | ||
| } | ||
| // For native platforms, use Buffer-like encoding | ||
| const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/='; | ||
| let output = ''; | ||
| for (let i = 0; i < data.length; i += 3) { | ||
| const byte1 = data.charCodeAt(i); | ||
| const byte2 = i + 1 < data.length ? data.charCodeAt(i + 1) : 0; | ||
| const byte3 = i + 2 < data.length ? data.charCodeAt(i + 2) : 0; | ||
|
|
||
| const enc1 = byte1 >> 2; | ||
| const enc2 = ((byte1 & 3) << 4) | (byte2 >> 4); | ||
| const enc3 = ((byte2 & 15) << 2) | (byte3 >> 6); | ||
| const enc4 = byte3 & 63; | ||
|
|
||
| if (i + 1 >= data.length) { | ||
| output += chars.charAt(enc1) + chars.charAt(enc2) + '=='; | ||
| } else if (i + 2 >= data.length) { | ||
| output += chars.charAt(enc1) + chars.charAt(enc2) + chars.charAt(enc3) + '='; | ||
| } else { | ||
| output += chars.charAt(enc1) + chars.charAt(enc2) + chars.charAt(enc3) + chars.charAt(enc4); | ||
| } | ||
| } | ||
| return output; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for generateSubmissionHashes usage
rg -n "generateSubmissionHashes" --type ts -A3 -B3Repository: jameshaworthcs/Checkout-App
Length of output: 198
🏁 Script executed:
# Also check what the btoa function is used for in context
rg -n "btoa|atob" utils/crypto.ts --type ts -B5 -A5Repository: jameshaworthcs/Checkout-App
Length of output: 1290
🏁 Script executed:
# Get the full utils/crypto.ts file to see the complete context
cat -n utils/crypto.ts | head -100Repository: jameshaworthcs/Checkout-App
Length of output: 3860
🏁 Script executed:
# Find all calls to generateSubmissionHashes to understand what data is passed
rg -n "generateSubmissionHashes" --type ts -B5 -A2Repository: jameshaworthcs/Checkout-App
Length of output: 1362
🏁 Script executed:
# Check if there are any tests or examples showing what data is passed to generateSubmissionHashes
rg -n "submissionData" --type ts -B2 -A2 | head -60Repository: jameshaworthcs/Checkout-App
Length of output: 2975
Native btoa implementation will produce incorrect base64 for non-ASCII input.
The native implementation uses charCodeAt() to read input values, which returns UTF-16 code units (0-65535). Standard base64 encoding expects to process bytes (0-255), typically from UTF-8. For any character with a code point > 255, this produces wrong output. The web version (window.btoa()) correctly throws a RangeError on non-ASCII, but the native version silently produces corrupted base64.
Since submissionData is derived from user-submitted code (line 32: generateSubmissionHashes({ chc: code })), this can silently corrupt the hash chain if submissions contain non-ASCII characters.
🤖 Prompt for AI Agents
In @utils/crypto.ts around lines 41 - 67, The native btoa implementation in
function btoa is wrong for non-ASCII because it uses charCodeAt/UTF-16 units;
convert the input string to UTF-8 bytes first and then base64-encode those
bytes. Use TextEncoder().encode(data) (or an equivalent UTF-8 byte conversion)
to get a Uint8Array, then on web call window.btoa on a binary string created
from those bytes, and on native platforms use a byte-aware base64 routine (e.g.,
Buffer.from(bytes).toString('base64') or a platform-safe base64 helper) instead
of charCodeAt; keep Platform.OS checks and update btoa to consistently encode
UTF-8 bytes so generateSubmissionHashes/submissionData are not corrupted.
| socket.onmessage = event => { | ||
| switch (step) { | ||
| case 1: | ||
| // Received response to step 1, send submission data | ||
| onProgress?.(25, 'Sending code...'); | ||
| socket.send(btoa(JSON.stringify(submissionData))); | ||
| break; | ||
|
|
||
| case 2: | ||
| // Received response to step 2, send user agent | ||
| onProgress?.(50, 'Verifying...'); | ||
| socket.send(btoa(getUserAgent())); | ||
| break; | ||
|
|
||
| case 3: | ||
| // Received response to step 3, send JWT | ||
| onProgress?.(75, 'Finalizing...'); | ||
| socket.send(jwt); | ||
| break; | ||
|
|
||
| case 4: | ||
| // Final response | ||
| completed = true; | ||
| cleanup(); | ||
| onProgress?.(100, 'Complete!'); | ||
| try { | ||
| const result = JSON.parse(atob(event.data)) as SubmissionResult; | ||
| socket.close(); | ||
| resolve(result); | ||
| } catch (parseError) { | ||
| socket.close(); | ||
| reject(new Error('Failed to parse submission response')); | ||
| } | ||
| break; | ||
| } | ||
| step++; | ||
| }; |
There was a problem hiding this comment.
WebSocket handshake doesn't validate server responses between steps.
The state machine advances on any message receipt without checking if the server acknowledged success. If the server sends an error at step 2, the code proceeds to step 3 anyway. Consider validating server responses before advancing.
💡 Example validation approach
socket.onmessage = event => {
+ // Optionally validate server acknowledgment
+ try {
+ const response = JSON.parse(atob(event.data));
+ if (response.error) {
+ completed = true;
+ cleanup();
+ socket.close();
+ reject(new Error(response.error));
+ return;
+ }
+ } catch {
+ // Not JSON or not base64 - continue with handshake
+ }
+
switch (step) {📝 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.
| socket.onmessage = event => { | |
| switch (step) { | |
| case 1: | |
| // Received response to step 1, send submission data | |
| onProgress?.(25, 'Sending code...'); | |
| socket.send(btoa(JSON.stringify(submissionData))); | |
| break; | |
| case 2: | |
| // Received response to step 2, send user agent | |
| onProgress?.(50, 'Verifying...'); | |
| socket.send(btoa(getUserAgent())); | |
| break; | |
| case 3: | |
| // Received response to step 3, send JWT | |
| onProgress?.(75, 'Finalizing...'); | |
| socket.send(jwt); | |
| break; | |
| case 4: | |
| // Final response | |
| completed = true; | |
| cleanup(); | |
| onProgress?.(100, 'Complete!'); | |
| try { | |
| const result = JSON.parse(atob(event.data)) as SubmissionResult; | |
| socket.close(); | |
| resolve(result); | |
| } catch (parseError) { | |
| socket.close(); | |
| reject(new Error('Failed to parse submission response')); | |
| } | |
| break; | |
| } | |
| step++; | |
| }; | |
| socket.onmessage = event => { | |
| // Optionally validate server acknowledgment | |
| try { | |
| const response = JSON.parse(atob(event.data)); | |
| if (response.error) { | |
| completed = true; | |
| cleanup(); | |
| socket.close(); | |
| reject(new Error(response.error)); | |
| return; | |
| } | |
| } catch { | |
| // Not JSON or not base64 - continue with handshake | |
| } | |
| switch (step) { | |
| case 1: | |
| // Received response to step 1, send submission data | |
| onProgress?.(25, 'Sending code...'); | |
| socket.send(btoa(JSON.stringify(submissionData))); | |
| break; | |
| case 2: | |
| // Received response to step 2, send user agent | |
| onProgress?.(50, 'Verifying...'); | |
| socket.send(btoa(getUserAgent())); | |
| break; | |
| case 3: | |
| // Received response to step 3, send JWT | |
| onProgress?.(75, 'Finalizing...'); | |
| socket.send(jwt); | |
| break; | |
| case 4: | |
| // Final response | |
| completed = true; | |
| cleanup(); | |
| onProgress?.(100, 'Complete!'); | |
| try { | |
| const result = JSON.parse(atob(event.data)) as SubmissionResult; | |
| socket.close(); | |
| resolve(result); | |
| } catch (parseError) { | |
| socket.close(); | |
| reject(new Error('Failed to parse submission response')); | |
| } | |
| break; | |
| } | |
| step++; | |
| }; |
🤖 Prompt for AI Agents
In @utils/submission.ts around lines 128 - 164, The socket.onmessage handler
(socket.onmessage) advances the step machine on any message; update it to
parse/inspect the server response before incrementing step (e.g., expect a
success flag/status or error field in the parsed message), only call
socket.send(...) for submissionData, getUserAgent(), or jwt and call step++ when
the response indicates success, and on error call cleanup(), close the socket,
reject with the server error (include details) instead of proceeding; ensure the
final step still parses SubmissionResult and rejects with parse error if needed.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
utils/api.ts (2)
60-117: Removeconsole.logof server error payloads + DRY ConnectionContext error propagation
- Line 66: logging error bodies can leak sensitive info (and contradicts “no need to log” comments).
- Line 72-82 and Line 101-111: repeated dynamic-import + setConnectionError blocks are easy to drift.
Proposed patch
+async function setConnectionErrorSafe(message: string) { + try { + const { getConnectionContext } = await import('@/contexts/ConnectionContext'); + getConnectionContext()?.setConnectionError(message); + } catch { + // Context not available + } +} @@ try { errorData = JSON.parse(responseText); - console.log('Error response data:', errorData); errorMessage = errorData.message || errorMessage; } catch (parseError) { @@ - try { - const { getConnectionContext } = await import('@/contexts/ConnectionContext'); - const context = getConnectionContext(); - if (context) { - context.setConnectionError( - 'Server returned an invalid response. Please check your connection.' - ); - } - } catch (contextError) { - // Context not available, continue with error - } + await setConnectionErrorSafe( + 'Server returned an invalid response. Please check your connection.' + ); errorMessage = 'Server returned an invalid response format'; } @@ try { data = JSON.parse(responseText); } catch (parseError) { @@ - try { - const { getConnectionContext } = await import('@/contexts/ConnectionContext'); - const context = getConnectionContext(); - if (context) { - context.setConnectionError( - 'Server returned an invalid response. Please check your connection.' - ); - } - } catch (contextError) { - // Context not available, continue with error - } + await setConnectionErrorSafe( + 'Server returned an invalid response. Please check your connection.' + ); return { success: false, error: 'Invalid response format from server', status: response.status, }; }
27-49: Normalize method + reconsidermode/credentialsusage across RN vs Web
- Line 33:
options.method !== 'GET'is case-sensitive. While current callers use uppercase'POST', defensive normalization prevents future bugs if lowercase methods are passed.- Lines 47-48:
mode: 'cors'is not a standard React Native fetch option and will be ignored.credentials: 'include'is supported but behaves inconsistently across RN platforms; consider whether cross-platform cookie handling works as intended or if auth tokens in headers are more reliable.Proposed patch
- // Only add Content-Type for non-GET requests or when there's a body - if (options.method !== 'GET' || options.body) { + const method = (options.method || 'GET').toUpperCase(); + + // Only add Content-Type for non-GET requests or when there's a body + if (method !== 'GET' || options.body) { headers['Content-Type'] = 'application/json'; } @@ - const fetchOptions: RequestInit = { - method: options.method || 'GET', + const fetchOptions: RequestInit = { + method, headers, credentials: 'include', // Include cookies if any mode: 'cors', // Enable CORS };
🤖 Fix all issues with AI agents
In @utils/crypto.ts:
- Around line 42-98: The custom btoa/atob implementations (btoa and atob) are
not UTF-8 safe and corrupt non-ASCII input (breaking generateSubmissionHashes
which btoa(JSON.stringify(...)) uses); replace these native-Platform fallback
implementations with a UTF-8–aware base64 implementation (for example import and
use encode/decode from the "js-base64" package) or, if you prefer not to add a
dependency, implement proper UTF-8 encoding/decoding before base64 ops; change
the btoa and atob functions to call the UTF-8-safe routines (or throw a clear
error if non-ASCII input is encountered) and ensure generateSubmissionHashes
continues to call btoa so hashes are correct for Unicode content.
- Around line 100-130: generateSubmissionHashes currently derives dateStr using
the local timezone which can desync with the server; update the weekday
derivation to use UTC so hashKey is timezone-stable (e.g. compute dateStr with
UTC: new Date().toLocaleDateString('en-US', { weekday: 'long', timeZone: 'UTC'
}) or an equivalent UTC weekday lookup), then build keyData and hashKey as
before so hashKey uses the UTC weekday.
In @utils/submission.ts:
- Around line 27-68: The prepareSubmission function (and similarly
submitViaHttp, generateShareLink, and undoSubmission) currently calls await
response.json() before checking response.ok which will throw on non-JSON error
bodies; change each to first await response.text(), then attempt
JSON.parse(text) in a try/catch to extract JSON fields if present, fall back to
the raw text for error messages, check response.ok and data.success, and throw a
clear Error using either parsed data.message/data.msg or the raw text; follow
the same text-then-JSON parsing/error creation pattern used by checkoutApi() so
error responses that are HTML or plain text are handled safely.
- Around line 170-214: submitViaHttp currently calls response.json() unguarded
which will throw on non-JSON or error responses; modify submitViaHttp to first
check response.ok and then wrap parsing in try/catch: if response.ok attempt to
parse JSON in a try block (const data = await response.json()), on parse failure
fall back to await response.text() and treat as error message; if !response.ok
read body via response.text() (or try json then text) and return a failure
SubmissionResult with submit:false and a clear msg including response.status and
the body; ensure you reference the existing response and data variables and
preserve onProgress updates.
- Around line 73-165: The submitViaWebSocket Promise can reject multiple times
because timeout, onerror, and parse-failure paths call reject() without setting
completed first; introduce a helper (e.g., finishError(error)) that atomically
sets completed = true, calls cleanup(), ensures socket.close() is called safely
(guarding against throws), and then calls reject(error); replace the timeout
callback, socket.onerror handler, the onclose unexpected branch, and the JSON
parse failure branch to call finishError(...) instead of calling reject/close
directly so the Promise can only settle once.
In @utils/userAgent.ts:
- Around line 41-55: The docstring incorrectly claims navigator.userAgent
includes the custom CheckOut-App identifier; update getHttpUserAgent to either
return getUserAgentString() for web when the custom app identifier is required
or change the comment to state that navigator.userAgent does NOT include the
custom identifier unless injected elsewhere; specifically modify the
getHttpUserAgent function (and its comment) to either always return
getUserAgentString() on Platform.OS === 'web' when you need the app identifier,
or make the docstring accurate by removing the assertion about the browser UA
containing CheckOut-App-* while keeping the current navigator.userAgent return
path.
🧹 Nitpick comments (3)
utils/api.ts (1)
41-43: Harden URL joining (custom base URLs + trailing slashes)Line 42:
${apiUrl}${path}will produce malformed URLs ifapiUrlends with/orpathdoesn’t start with/(and custom URLs make this more likely).Proposed patch
- const apiUrl = await getApiUrl(); - const url = `${apiUrl}${path}`; + const apiUrl = await getApiUrl(); + const url = new URL(path, apiUrl).toString();utils/crypto.ts (1)
133-140:getUserAgent()is deprecated but still used elsewhereLine 136-139: since utils/submission.ts calls
getUserAgent(), you’re not getting the benefit of the deprecation yet. Consider swapping call sites togetUserAgentString()and removing this export once migrated.utils/submission.ts (1)
15-21: Use safer WS URL construction (avoid double slashes / weird custom base paths)Line 19-20:
apiUrl.replace(/^http/, 'ws')+${wsUrl}${path}can producewss://host//api/...depending on stored base URL.Proposed patch
function getWebSocketUrl(apiUrl: string, path: string): string { - const wsUrl = apiUrl.replace(/^http/, 'ws'); - return `${wsUrl}${path}`; + const u = new URL(apiUrl); + u.protocol = u.protocol === 'https:' ? 'wss:' : 'ws:'; + return new URL(path, u.toString()).toString(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
utils/api.tsutils/crypto.tsutils/submission.tsutils/userAgent.ts
🧰 Additional context used
🧬 Code graph analysis (3)
utils/submission.ts (5)
types/liveClasses.ts (5)
SubmitOptionsResponse(79-85)SubmissionData(71-76)ProgressCallback(117-117)SubmissionResult(88-93)AutoSplashResult(110-114)constants/api.ts (1)
getApiUrl(8-16)utils/storage.ts (1)
storage(33-36)utils/crypto.ts (4)
generateSubmissionHashes(104-131)btoa(42-68)getUserAgent(138-140)atob(73-98)utils/userAgent.ts (1)
getHeadersWithUserAgent(70-80)
utils/api.ts (3)
utils/userAgent.ts (1)
getHeadersWithUserAgent(70-80)constants/api.ts (1)
getApiUrl(8-16)contexts/ConnectionContext.tsx (1)
getConnectionContext(31-33)
utils/crypto.ts (1)
utils/userAgent.ts (1)
getUserAgentString(35-39)
🔇 Additional comments (5)
utils/userAgent.ts (2)
70-79: Verify native support for overridingUser-AgentheaderLine 75-77: some RN fetch stacks ignore/strip
User-Agent, and behavior can differ between iOS/Android. Worth validating with your actual runtime (and consider using a different custom header if needed).
24-26: The current implementation is correct.Constants.expoConfig?.versionwith optional chaining is the supported modern Expo API. The suggested fallbacks toConstants.manifestandConstants.manifest2are deprecated and unreliable in modern Expo SDKs and should be avoided. WhenConstants.expoConfig?.versionis undefined (e.g., in Expo Go or dev environments without a rebuild), the fallback to'1.0.0'is appropriate.Likely an incorrect or invalid review comment.
utils/submission.ts (3)
345-367:validateCode()looks solidClear checks, good error messages, and no obvious edge-case holes.
219-258: Verify intended UX:showAutoSplashforced false on WS fallbackIf
prepareSubmission()succeeds withoptions.autoSplash === true, falling back to HTTP currently returnsshowAutoSplash: falseand drops thejwt. Confirm whether AutoSplash is inherently WebSocket-only or if it should be preserved during HTTP fallback. If AutoSplash is protocol-agnostic and this setting should persist, the fallback at line 255 needs to passshowAutoSplashthrough instead of hardcoding it tofalse.
372-450: Checkresponse.okbefore callingresponse.json()in both functions
generateShareLink()andundoSubmission()should follow the pattern used inprepareSubmission()by checkingresponse.okbefore reading the response body. If the server returns an HTTP error with a non-JSON response body,response.json()will throw, causing both functions to report a generic "Network error" instead of the actual HTTP error details.
| export function btoa(data: string): string { | ||
| if (Platform.OS === 'web') { | ||
| return window.btoa(data); | ||
| } | ||
| // For native platforms, use Buffer-like encoding | ||
| const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/='; | ||
| let output = ''; | ||
| for (let i = 0; i < data.length; i += 3) { | ||
| const byte1 = data.charCodeAt(i); | ||
| const byte2 = i + 1 < data.length ? data.charCodeAt(i + 1) : 0; | ||
| const byte3 = i + 2 < data.length ? data.charCodeAt(i + 2) : 0; | ||
|
|
||
| const enc1 = byte1 >> 2; | ||
| const enc2 = ((byte1 & 3) << 4) | (byte2 >> 4); | ||
| const enc3 = ((byte2 & 15) << 2) | (byte3 >> 6); | ||
| const enc4 = byte3 & 63; | ||
|
|
||
| if (i + 1 >= data.length) { | ||
| output += chars.charAt(enc1) + chars.charAt(enc2) + '=='; | ||
| } else if (i + 2 >= data.length) { | ||
| output += chars.charAt(enc1) + chars.charAt(enc2) + chars.charAt(enc3) + '='; | ||
| } else { | ||
| output += chars.charAt(enc1) + chars.charAt(enc2) + chars.charAt(enc3) + chars.charAt(enc4); | ||
| } | ||
| } | ||
| return output; | ||
| } | ||
|
|
||
| /** | ||
| * Base64 decode a string | ||
| */ | ||
| export function atob(data: string): string { | ||
| if (Platform.OS === 'web') { | ||
| return window.atob(data); | ||
| } | ||
| // For native platforms | ||
| const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/='; | ||
| let output = ''; | ||
| // Remove any padding characters and whitespace | ||
| const cleanData = data.replace(/[=\s]/g, ''); | ||
|
|
||
| for (let i = 0; i < cleanData.length; i += 4) { | ||
| const enc1 = chars.indexOf(cleanData.charAt(i)); | ||
| const enc2 = chars.indexOf(cleanData.charAt(i + 1)); | ||
| const enc3 = i + 2 < cleanData.length ? chars.indexOf(cleanData.charAt(i + 2)) : 64; | ||
| const enc4 = i + 3 < cleanData.length ? chars.indexOf(cleanData.charAt(i + 3)) : 64; | ||
|
|
||
| const byte1 = (enc1 << 2) | (enc2 >> 4); | ||
| const byte2 = ((enc2 & 15) << 4) | (enc3 >> 2); | ||
| const byte3 = ((enc3 & 3) << 6) | enc4; | ||
|
|
||
| output += String.fromCharCode(byte1); | ||
| if (enc3 !== 64) output += String.fromCharCode(byte2); | ||
| if (enc4 !== 64) output += String.fromCharCode(byte3); | ||
| } | ||
| return output; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and its context
cd /tmp && find . -name "crypto.ts" -o -name "crypto.js" 2>/dev/null | head -5Repository: jameshaworthcs/Checkout-App
Length of output: 53
🏁 Script executed:
# Find the crypto file in the repository
git ls-files | grep -i cryptoRepository: jameshaworthcs/Checkout-App
Length of output: 86
🏁 Script executed:
# Search for btoa and atob usage in the codebase
rg "btoa|atob" --type ts --type js -B 2 -A 2Repository: jameshaworthcs/Checkout-App
Length of output: 2621
🏁 Script executed:
# Check if there are any tests for the base64 functions
rg "btoa|atob" --type test -B 3 -A 3Repository: jameshaworthcs/Checkout-App
Length of output: 99
🏁 Script executed:
# Look for UTF-8 or encoding-related comments in the file
rg "UTF|encoding|unicode" utils/crypto.ts -B 2 -A 2Repository: jameshaworthcs/Checkout-App
Length of output: 260
🏁 Script executed:
# Read the entire crypto.ts file to verify the implementation
cat -n utils/crypto.tsRepository: jameshaworthcs/Checkout-App
Length of output: 5498
🏁 Script executed:
# Check what auth tokens look like - examine the storage/auth mechanism
rg "authToken|getToken" --type ts --type js -B 2 -A 2 | head -40Repository: jameshaworthcs/Checkout-App
Length of output: 528
🏁 Script executed:
# Test the base64 implementation with non-ASCII characters using Node.js
cat > /tmp/test_base64.js << 'EOF'
// Simulate the custom btoa from utils/crypto.ts
function customBtoa(data) {
const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=';
let output = '';
for (let i = 0; i < data.length; i += 3) {
const byte1 = data.charCodeAt(i);
const byte2 = i + 1 < data.length ? data.charCodeAt(i + 1) : 0;
const byte3 = i + 2 < data.length ? data.charCodeAt(i + 2) : 0;
const enc1 = byte1 >> 2;
const enc2 = ((byte1 & 3) << 4) | (byte2 >> 4);
const enc3 = ((byte2 & 15) << 2) | (byte3 >> 6);
const enc4 = byte3 & 63;
if (i + 1 >= data.length) {
output += chars.charAt(enc1) + chars.charAt(enc2) + '==';
} else if (i + 2 >= data.length) {
output += chars.charAt(enc1) + chars.charAt(enc2) + chars.charAt(enc3) + '=';
} else {
output += chars.charAt(enc1) + chars.charAt(enc2) + chars.charAt(enc3) + chars.charAt(enc4);
}
}
return output;
}
// Test cases
const testCases = [
'hello',
'café', // é = U+00E9
'你好', // Chinese characters (multibyte UTF-8)
'🚀', // Emoji (4-byte UTF-8)
];
console.log('Testing custom btoa with various inputs:\n');
testCases.forEach(test => {
const custom = customBtoa(test);
const native = Buffer.from(test).toString('base64');
const match = custom === native ? '✓' : '✗ MISMATCH';
console.log(`Input: "${test}"`);
console.log(` Custom: ${custom}`);
console.log(` Native: ${native}`);
console.log(` ${match}\n`);
});
EOF
node /tmp/test_base64.jsRepository: jameshaworthcs/Checkout-App
Length of output: 352
🏁 Script executed:
# Check if user agent strings could contain non-ASCII
rg "getUserAgent|userAgent" utils/crypto.ts -B 5 -A 10Repository: jameshaworthcs/Checkout-App
Length of output: 1397
🏁 Script executed:
# Look for any JSON payload structures that might contain user-generated content
rg "submissionData|JSON.stringify" utils/crypto.ts -B 3 -A 3Repository: jameshaworthcs/Checkout-App
Length of output: 418
Replace custom base64 with UTF-8 safe implementation
The custom btoa/atob implementations (lines 42-98) are broken for non-ASCII input. Testing confirms:
- "café" encodes to
Y2Fm6Q==instead of correctY2Fmw6k= - "你好" encodes to
0=instead of correct5L2g5aW9 - "🚀" encodes to
A=instead of correct8J+agA==
Since generateSubmissionHashes() encodes JSON.stringify(submissionData) via btoa(), any submission containing non-ASCII characters (user names, text, etc.) will produce incorrect hashes. This causes server verification to fail on native platforms.
Use a UTF-8 safe base64 library (e.g., js-base64) or throw an error to prevent silent corruption.
🤖 Prompt for AI Agents
In @utils/crypto.ts around lines 42 - 98, The custom btoa/atob implementations
(btoa and atob) are not UTF-8 safe and corrupt non-ASCII input (breaking
generateSubmissionHashes which btoa(JSON.stringify(...)) uses); replace these
native-Platform fallback implementations with a UTF-8–aware base64
implementation (for example import and use encode/decode from the "js-base64"
package) or, if you prefer not to add a dependency, implement proper UTF-8
encoding/decoding before base64 ops; change the btoa and atob functions to call
the UTF-8-safe routines (or throw a clear error if non-ASCII input is
encountered) and ensure generateSubmissionHashes continues to call btoa so
hashes are correct for Unicode content.
| /** | ||
| * Generate anti-abuse verification hashes for submission | ||
| * Matches the web implementation in home-js.ejs | ||
| */ | ||
| export async function generateSubmissionHashes( | ||
| submissionData: object | ||
| ): Promise<{ hashKey: string; hashValue: string } | null> { | ||
| try { | ||
| const data = JSON.stringify(submissionData); | ||
| const base64Data = btoa(data); | ||
|
|
||
| // Generate hash chain: SHA-256 -> SHA-384 -> SHA-512 -> SHA-256 | ||
| const hash1 = await sha256(base64Data); | ||
| const hash2 = await sha384(hash1); | ||
| const hash3 = await sha512(hash2); | ||
|
|
||
| // Get user agent in format: CheckOut-App-<Platform>/<Version> | ||
| const userAgent = getUserAgentString(); | ||
| const finalData = `reject${hash3}${userAgent}`; | ||
| const finalHash = await sha256(finalData); | ||
|
|
||
| // Generate hash key from date and user agent | ||
| const dateStr = new Date().toLocaleDateString('en-US', { weekday: 'long' }); | ||
| const keyData = `${dateStr}${userAgent}`; | ||
| const hashKey = await sha256(keyData); | ||
|
|
||
| return { hashKey, hashValue: finalHash }; | ||
| } catch (error) { | ||
| console.error('Error generating submission hashes:', error); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "crypto.ts" | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 241
🏁 Script executed:
fd -type f -name "*.ejs" | grep -i home | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 241
🏁 Script executed:
find . -type f -name "*.ts" -path "*/utils/*" | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 165
🏁 Script executed:
cat -n utils/crypto.tsRepository: jameshaworthcs/Checkout-App
Length of output: 5498
🏁 Script executed:
fd -type f -name "*.ejs"Repository: jameshaworthcs/Checkout-App
Length of output: 241
🏁 Script executed:
rg "home-js" --type allRepository: jameshaworthcs/Checkout-App
Length of output: 135
🏁 Script executed:
find . -name "*.ejs" -o -name "*home*" -o -name "*.html" | head -30Repository: jameshaworthcs/Checkout-App
Length of output: 53
🏁 Script executed:
rg "toLocaleDateString|toUTCString|UTC|timezone" --max-count 20Repository: jameshaworthcs/Checkout-App
Length of output: 412
🏁 Script executed:
rg "home\.js|home-js"Repository: jameshaworthcs/Checkout-App
Length of output: 135
🏁 Script executed:
git ls-files | grep -i homeRepository: jameshaworthcs/Checkout-App
Length of output: 53
Use UTC timezone for hash key derivation to match server implementation
Line 122: The weekday derivation uses local timezone, which can differ from the server around midnight and cause intermittent verification failures. The comment on line 111 is accurate, but the hash key generation should be timezone-stable.
Patch (if server expects UTC)
- const dateStr = new Date().toLocaleDateString('en-US', { weekday: 'long' });
+ const dateStr = new Intl.DateTimeFormat('en-US', {
+ weekday: 'long',
+ timeZone: 'UTC',
+ }).format(new Date());📝 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.
| /** | |
| * Generate anti-abuse verification hashes for submission | |
| * Matches the web implementation in home-js.ejs | |
| */ | |
| export async function generateSubmissionHashes( | |
| submissionData: object | |
| ): Promise<{ hashKey: string; hashValue: string } | null> { | |
| try { | |
| const data = JSON.stringify(submissionData); | |
| const base64Data = btoa(data); | |
| // Generate hash chain: SHA-256 -> SHA-384 -> SHA-512 -> SHA-256 | |
| const hash1 = await sha256(base64Data); | |
| const hash2 = await sha384(hash1); | |
| const hash3 = await sha512(hash2); | |
| // Get user agent in format: CheckOut-App-<Platform>/<Version> | |
| const userAgent = getUserAgentString(); | |
| const finalData = `reject${hash3}${userAgent}`; | |
| const finalHash = await sha256(finalData); | |
| // Generate hash key from date and user agent | |
| const dateStr = new Date().toLocaleDateString('en-US', { weekday: 'long' }); | |
| const keyData = `${dateStr}${userAgent}`; | |
| const hashKey = await sha256(keyData); | |
| return { hashKey, hashValue: finalHash }; | |
| } catch (error) { | |
| console.error('Error generating submission hashes:', error); | |
| return null; | |
| } | |
| /** | |
| * Generate anti-abuse verification hashes for submission | |
| * Matches the web implementation in home-js.ejs | |
| */ | |
| export async function generateSubmissionHashes( | |
| submissionData: object | |
| ): Promise<{ hashKey: string; hashValue: string } | null> { | |
| try { | |
| const data = JSON.stringify(submissionData); | |
| const base64Data = btoa(data); | |
| // Generate hash chain: SHA-256 -> SHA-384 -> SHA-512 -> SHA-256 | |
| const hash1 = await sha256(base64Data); | |
| const hash2 = await sha384(hash1); | |
| const hash3 = await sha512(hash2); | |
| // Get user agent in format: CheckOut-App-<Platform>/<Version> | |
| const userAgent = getUserAgentString(); | |
| const finalData = `reject${hash3}${userAgent}`; | |
| const finalHash = await sha256(finalData); | |
| // Generate hash key from date and user agent | |
| const dateStr = new Intl.DateTimeFormat('en-US', { | |
| weekday: 'long', | |
| timeZone: 'UTC', | |
| }).format(new Date()); | |
| const keyData = `${dateStr}${userAgent}`; | |
| const hashKey = await sha256(keyData); | |
| return { hashKey, hashValue: finalHash }; | |
| } catch (error) { | |
| console.error('Error generating submission hashes:', error); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In @utils/crypto.ts around lines 100 - 130, generateSubmissionHashes currently
derives dateStr using the local timezone which can desync with the server;
update the weekday derivation to use UTC so hashKey is timezone-stable (e.g.
compute dateStr with UTC: new Date().toLocaleDateString('en-US', { weekday:
'long', timeZone: 'UTC' }) or an equivalent UTC weekday lookup), then build
keyData and hashKey as before so hashKey uses the UTC weekday.
| export async function prepareSubmission(code: string): Promise<SubmitOptionsResponse> { | ||
| const apiUrl = await getApiUrl(); | ||
| const token = await storage.getItem(API_TOKEN_KEY); | ||
|
|
||
| // Generate anti-abuse hashes | ||
| const hashes = await generateSubmissionHashes({ chc: code }); | ||
|
|
||
| const body: Record<string, string> = { | ||
| chc: code, | ||
| }; | ||
|
|
||
| // Add hash if generated successfully, otherwise use fallback | ||
| if (hashes) { | ||
| body[hashes.hashKey] = hashes.hashValue; | ||
| } else { | ||
| body.fallback = 'true'; | ||
| } | ||
|
|
||
| const headers: Record<string, string> = getHeadersWithUserAgent({ | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json', | ||
| }); | ||
|
|
||
| if (token) { | ||
| headers['x-checkout-key'] = token; | ||
| } | ||
|
|
||
| const response = await fetch(`${apiUrl}/api/submit/options`, { | ||
| method: 'POST', | ||
| headers, | ||
| credentials: 'include', | ||
| body: JSON.stringify(body), | ||
| }); | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| if (!response.ok || !data.success) { | ||
| throw new Error(data.message || data.msg || 'Failed to prepare submission'); | ||
| } | ||
|
|
||
| return data as SubmitOptionsResponse; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find utils/api.ts and examine the checkoutApi() function
find . -name "api.ts" -type f | grep -E "utils|src" | head -5Repository: jameshaworthcs/Checkout-App
Length of output: 85
🏁 Script executed:
# List files in utils directory to understand structure
ls -la utils/ 2>/dev/null || ls -la src/utils/ 2>/dev/null || find . -type d -name "utils" | head -5Repository: jameshaworthcs/Checkout-App
Length of output: 496
🏁 Script executed:
# Look for the utils directory and list its contents
git ls-files | grep "utils/" | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 155
🏁 Script executed:
cat utils/api.tsRepository: jameshaworthcs/Checkout-App
Length of output: 4270
🏁 Script executed:
cat utils/submission.tsRepository: jameshaworthcs/Checkout-App
Length of output: 11657
prepareSubmission() should read response as text before JSON parsing to handle error responses
Lines 57-59: Calling await response.json() before checking response.ok will throw on HTML/text error responses, bypassing your error handling. Adopt the "read text then JSON.parse" pattern from checkoutApi() in utils/api.ts to safely handle both JSON and non-JSON responses.
Also apply this fix to submitViaHttp() (line 160), generateShareLink() (line 390), and undoSubmission() (line 415).
🤖 Prompt for AI Agents
In @utils/submission.ts around lines 27 - 68, The prepareSubmission function
(and similarly submitViaHttp, generateShareLink, and undoSubmission) currently
calls await response.json() before checking response.ok which will throw on
non-JSON error bodies; change each to first await response.text(), then attempt
JSON.parse(text) in a try/catch to extract JSON fields if present, fall back to
the raw text for error messages, check response.ok and data.success, and throw a
clear Error using either parsed data.message/data.msg or the raw text; follow
the same text-then-JSON parsing/error creation pattern used by checkoutApi() so
error responses that are HTML or plain text are handled safely.
| export async function submitViaWebSocket( | ||
| wsUrl: string, | ||
| authToken: string, | ||
| submissionData: SubmissionData, | ||
| jwt: string, | ||
| onProgress?: ProgressCallback | ||
| ): Promise<SubmissionResult> { | ||
| return new Promise((resolve, reject) => { | ||
| try { | ||
| const socket = new WebSocket(wsUrl); | ||
| let step = 1; | ||
| let completed = false; | ||
| let timeoutId: ReturnType<typeof setTimeout> | null = null; | ||
|
|
||
| // Set timeout for the entire WebSocket operation | ||
| timeoutId = setTimeout(() => { | ||
| if (!completed) { | ||
| socket.close(); | ||
| reject(new Error('WebSocket submission timed out')); | ||
| } | ||
| }, 30000); // 30 second timeout | ||
|
|
||
| const cleanup = () => { | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId); | ||
| timeoutId = null; | ||
| } | ||
| }; | ||
|
|
||
| socket.onopen = () => { | ||
| onProgress?.(10, 'Connecting...'); | ||
| // Step 1: Send auth token | ||
| socket.send(btoa(authToken)); | ||
| }; | ||
|
|
||
| socket.onerror = error => { | ||
| cleanup(); | ||
| reject(new Error('WebSocket connection error')); | ||
| }; | ||
|
|
||
| socket.onclose = event => { | ||
| cleanup(); | ||
| if (!completed) { | ||
| reject( | ||
| new Error( | ||
| `WebSocket closed unexpectedly (code ${event.code})${event.reason ? `: ${event.reason}` : ''}` | ||
| ) | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| socket.onmessage = event => { | ||
| switch (step) { | ||
| case 1: | ||
| // Received response to step 1, send submission data | ||
| onProgress?.(25, 'Sending code...'); | ||
| socket.send(btoa(JSON.stringify(submissionData))); | ||
| break; | ||
|
|
||
| case 2: | ||
| // Received response to step 2, send user agent | ||
| onProgress?.(50, 'Verifying...'); | ||
| socket.send(btoa(getUserAgent())); | ||
| break; | ||
|
|
||
| case 3: | ||
| // Received response to step 3, send JWT | ||
| onProgress?.(75, 'Finalizing...'); | ||
| socket.send(jwt); | ||
| break; | ||
|
|
||
| case 4: | ||
| // Final response | ||
| completed = true; | ||
| cleanup(); | ||
| onProgress?.(100, 'Complete!'); | ||
| try { | ||
| const result = JSON.parse(atob(event.data)) as SubmissionResult; | ||
| socket.close(); | ||
| resolve(result); | ||
| } catch (parseError) { | ||
| socket.close(); | ||
| reject(new Error('Failed to parse submission response')); | ||
| } | ||
| break; | ||
| } | ||
| step++; | ||
| }; | ||
| } catch (error) { | ||
| reject(error); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file
fd -t f "submission.ts" --type fileRepository: jameshaworthcs/Checkout-App
Length of output: 90
🏁 Script executed:
# Read the file to verify the code
cat -n utils/submission.ts | head -200Repository: jameshaworthcs/Checkout-App
Length of output: 6818
🏁 Script executed:
# Let me check WebSocket behavior - whether close() triggers onclose
# and verify if there's any mitigation already in place
cd utils && grep -n "completed" submission.tsRepository: jameshaworthcs/Checkout-App
Length of output: 440
WebSocket promise can double-reject; timeout and error handlers must set completed before rejecting
Lines 88–93 (timeout) and 108–111 (error) reject without setting completed = true. Since socket.close() triggers onclose, the guard at line 115 (if (!completed)) will then reject again. Similarly, line 155 rejects after socket.close() without first ensuring completed is set (though line 146 does protect the success path). This violates Promise contract and can cause unhandled rejection warnings or cascading errors.
Refactor with a finishError helper that atomically sets completed, calls cleanup, safely closes the socket, and rejects—ensuring each error path terminates only once.
🤖 Prompt for AI Agents
In @utils/submission.ts around lines 73 - 165, The submitViaWebSocket Promise
can reject multiple times because timeout, onerror, and parse-failure paths call
reject() without setting completed first; introduce a helper (e.g.,
finishError(error)) that atomically sets completed = true, calls cleanup(),
ensures socket.close() is called safely (guarding against throws), and then
calls reject(error); replace the timeout callback, socket.onerror handler, the
onclose unexpected branch, and the JSON parse failure branch to call
finishError(...) instead of calling reject/close directly so the Promise can
only settle once.
| export async function submitViaHttp( | ||
| submissionData: SubmissionData, | ||
| onProgress?: ProgressCallback | ||
| ): Promise<SubmissionResult> { | ||
| onProgress?.(50, 'Finishing up...'); | ||
|
|
||
| const apiUrl = await getApiUrl(); | ||
| const token = await storage.getItem(API_TOKEN_KEY); | ||
|
|
||
| const headers: Record<string, string> = getHeadersWithUserAgent({ | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json', | ||
| }); | ||
|
|
||
| if (token) { | ||
| headers['x-checkout-key'] = token; | ||
| } | ||
|
|
||
| const response = await fetch(`${apiUrl}/api/submit/code`, { | ||
| method: 'POST', | ||
| headers, | ||
| credentials: 'include', | ||
| body: JSON.stringify(submissionData), | ||
| }); | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| onProgress?.(100, 'Complete!'); | ||
|
|
||
| // Normalize response to match WebSocket format | ||
| if (data.success === 'true' || data.success === true) { | ||
| return { | ||
| success: true, | ||
| submit: true, | ||
| tk: data.tk, | ||
| msg: data.message || 'Submitted!', | ||
| }; | ||
| } else { | ||
| return { | ||
| success: false, | ||
| submit: false, | ||
| msg: data.message || data.msg || 'Submission failed.', | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the submission.ts file
fd -type f -name "submission.ts" | head -20Repository: jameshaworthcs/Checkout-App
Length of output: 241
🏁 Script executed:
#!/bin/bash
# Find the submission.ts file
fd "submission.ts"Repository: jameshaworthcs/Checkout-App
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Get the file size first
wc -l utils/submission.tsRepository: jameshaworthcs/Checkout-App
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Read lines 170-214
sed -n '170,214p' utils/submission.tsRepository: jameshaworthcs/Checkout-App
Length of output: 1157
🏁 Script executed:
#!/bin/bash
# Search for calls to submitViaHttp
rg "submitViaHttp" utils/submission.ts -A 3 -B 3Repository: jameshaworthcs/Checkout-App
Length of output: 820
🏁 Script executed:
#!/bin/bash
# Search for WebSocket submission function to compare error handling
rg "submitViaWebSocket|submitViaWs|WebSocket.*submit" utils/submission.ts -A 20 | head -100Repository: jameshaworthcs/Checkout-App
Length of output: 1953
Guard JSON parsing and check response.ok before calling response.json() on line 195
The HTTP fallback lacks error handling for non-JSON responses. If the server returns a non-200 status or an error page (HTML), response.json() will throw an unhandled exception, causing the fallback to fail. Add a response.ok check and wrap the JSON parse in a try-catch.
🤖 Prompt for AI Agents
In @utils/submission.ts around lines 170 - 214, submitViaHttp currently calls
response.json() unguarded which will throw on non-JSON or error responses;
modify submitViaHttp to first check response.ok and then wrap parsing in
try/catch: if response.ok attempt to parse JSON in a try block (const data =
await response.json()), on parse failure fall back to await response.text() and
treat as error message; if !response.ok read body via response.text() (or try
json then text) and return a failure SubmissionResult with submit:false and a
clear msg including response.status and the body; ensure you reference the
existing response and data variables and preserve onProgress updates.
| /** | ||
| * Get the User-Agent header value. | ||
| * On web, returns the browser's native user agent (which includes the custom app identifier). | ||
| * On native platforms, returns the custom CheckOut-App user agent. | ||
| * | ||
| * For WebSocket handshakes and crypto verification, use getUserAgentString() directly. | ||
| */ | ||
| export function getHttpUserAgent(): string { | ||
| if (Platform.OS === 'web') { | ||
| // On web, browsers automatically set User-Agent and we can't override it in fetch | ||
| // Return our custom identifier for logging/tracking purposes if needed | ||
| return typeof navigator !== 'undefined' ? navigator.userAgent : getUserAgentString(); | ||
| } | ||
| return getUserAgentString(); | ||
| } |
There was a problem hiding this comment.
Docstring mismatch: browser UA won’t “include the custom app identifier”
Line 43-44: navigator.userAgent won’t include CheckOut-App-* unless you’re explicitly injecting it somewhere else (not shown here). Either adjust the comment or return getUserAgentString() on web when you need the app identifier.
🤖 Prompt for AI Agents
In @utils/userAgent.ts around lines 41 - 55, The docstring incorrectly claims
navigator.userAgent includes the custom CheckOut-App identifier; update
getHttpUserAgent to either return getUserAgentString() for web when the custom
app identifier is required or change the comment to state that
navigator.userAgent does NOT include the custom identifier unless injected
elsewhere; specifically modify the getHttpUserAgent function (and its comment)
to either always return getUserAgentString() on Platform.OS === 'web' when you
need the app identifier, or make the docstring accurate by removing the
assertion about the browser UA containing CheckOut-App-* while keeping the
current navigator.userAgent return path.
- Added expo-web-browser to app.json dependencies. - Updated various Expo and React Native packages in package.json and package-lock.json. - Improved AutoCheckin screen with enhanced authentication handling and error management. - Added delete account functionality in Settings screen with confirmation dialog. - Refactored styles for AutoCheckin and Settings screens for better organization.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
styles/history.styles.ts (1)
45-51: Object identity comparison will not reliably detect light theme.The comparison
theme === Colors.lightchecks object identity, not whether the theme represents "light mode". Sincethemeis passed in as a parameter (likely a dynamically created or spread object), this comparison will likely always befalse, causing all shadow styles to use the dark-mode values (shadowOpacity: 0.3,elevation: 4).Consider adding a
modeorcolorSchemeproperty toThemeColors, or pass a separate boolean/string parameter to indicate light vs dark mode.Suggested approach
-export const createHistoryStyles = (theme: ColorTheme) => +export const createHistoryStyles = (theme: ColorTheme, isLightTheme: boolean) => StyleSheet.create({ // ... codeItem: { // ... ...Platform.select({ ios: { shadowColor: '#000', shadowOffset: { width: 0, height: 0 }, - shadowOpacity: theme === Colors.light ? 0.1 : 0.3, + shadowOpacity: isLightTheme ? 0.1 : 0.3, shadowRadius: 8, }, android: { - elevation: theme === Colors.light ? 2 : 4, + elevation: isLightTheme ? 2 : 4, }, }), },utils/api.ts (2)
27-49: Add method variable to fix Content-Type logic for default GET requests.When
options.methodis undefined, the conditionoptions.method !== 'GET'evaluates to true, incorrectly addingContent-Type: application/jsonto default GET requests. This triggers unnecessary CORS preflight requests and alters caching behavior. Define the method once upfront and use it consistently in both the Content-Type check and fetch options:Proposed fix
export async function checkoutApi<T = any>( path: string, options: RequestOptions = {} ): Promise<ApiResponse<T>> { try { const token = await storage.getItem(API_TOKEN_KEY); + const method = options.method || 'GET'; const headers: Record<string, string> = getHeadersWithUserAgent({ Accept: 'application/json', ...options.headers, }); // Only add Content-Type for non-GET requests or when there's a body - if (options.method !== 'GET' || options.body) { + if (method !== 'GET' || options.body) { headers['Content-Type'] = 'application/json'; } @@ const fetchOptions: RequestInit = { - method: options.method || 'GET', + method, headers, credentials: 'include', // Include cookies if any mode: 'cors', // Enable CORS };
57-137: Handle empty/204 responses and remove unguarded error logging.
JSON.parse('')throws on empty body; legitimate204 No Contentresponses fail and return an error (lines 110, 126–130).console.log('Error response data:', errorData)at line 66 can leak sensitive data (API keys, tokens, PII) into production logs.Fix:
- Check if response body is empty before attempting to parse.
- Return success for 204 (or any empty body) at line 57–58.
- Remove or conditionally gate the console.log (this codebase doesn't use
__DEV__; useprocess.env.NODE_ENV === 'development'if environment detection is needed, or simply remove for security).Example approach
// Read response as text first to handle JSON parse errors const responseText = await response.text(); + const hasBody = responseText.trim().length > 0; if (!response.ok) { let errorMessage = 'An error occurred while processing your request'; let errorData; try { - errorData = JSON.parse(responseText); - console.log('Error response data:', errorData); + errorData = hasBody ? JSON.parse(responseText) : undefined; errorMessage = errorData.message || errorData.msg || errorMessage;Then guard the success path (after line 106) to handle empty bodies:
+ if (!hasBody) { + return { + success: true, + data: undefined as unknown as T, + status: response.status, + }; + }
🤖 Fix all issues with AI agents
In `@components/ConnectionErrorBanner.tsx`:
- Around line 23-74: The success banner is shown when shouldShowError becomes
false (which also happens when type becomes 'unavailable'), so change the
recovery check in the effect to only trigger when the error UI is actually
dismissed by checking visibility instead: replace the condition "if
(previousShouldShowRef.current && !shouldShowError)" with "if
(previousShouldShowRef.current && !isVisible)" inside the useEffect that manages
successTimeoutRef and setShowSuccessBanner, leaving the rest of the
timeout/cleanup logic intact and keeping previousShouldShowRef.current updated
as before.
In `@contexts/ConnectionContext.tsx`:
- Around line 29-34: The global mutation of globalConnectionContext (currently
set from contextValue during render in ConnectionProvider) must be moved into a
React useEffect to avoid publishing uncommitted render state; update the
ConnectionProvider to set globalConnectionContext = contextValue inside a
useEffect that depends on contextValue, and clear it (set undefined) in the
effect cleanup so async error handlers using getConnectionContext() never
observe transient render state.
♻️ Duplicate comments (8)
app/(tabs)/settings.tsx (1)
146-188:Alert.promptis iOS-only; Android devices will crash.The
elseblock at line 150 usesAlert.promptwhich only works on iOS. On Android, this will throw a runtime error. The web case is handled correctly with the modal, but Android falls through to the iOS-only API.Additionally, the hard-coded URL
'https://checkout.ac'on lines 148 and 185 should use a shared constant.🐛 Proposed fix to handle Android
const showApiUrlConfig = () => { - if (Platform.OS === 'web') { + if (Platform.OS === 'web' || Platform.OS === 'android') { + // Use modal for web and Android (Alert.prompt is iOS-only) setApiUrlInput(currentApiUrl !== 'https://checkout.ac' ? currentApiUrl : ''); setApiUrlModalVisible(true); } else { + // iOS supports Alert.prompt natively Alert.prompt(package.json (3)
32-32: Remove deprecatedexpo-randompackage.
expo-randomis deprecated and should be replaced withexpo-crypto(already present at line 28). Remove this dependency and update any imports throughout the codebase.
40-40: Upgrade React to 19.1.1 to fix owner stacks compatibility.React 19.1.0 has a known issue where owner stacks don't work correctly with
@babel/plugin-transform-function-name(enabled by default in babel-preset-expo). React Native 0.81.x expects React 19.1.1 which includes the fix.🔧 Suggested fix
- "react": "19.1.0", - "react-dom": "19.1.0", + "react": "19.1.1", + "react-dom": "19.1.1",
44-51: Replacereact-native-worklets-corewithreact-native-worklets.
react-native-reanimated@~4.1.0requiresreact-native-worklets(v0.5.x or v0.6.x), notreact-native-worklets-core@^1.6.2. These are different packages, and using the wrong one may cause "Unable to find a specification for RNWorklets" errors at runtime.🐛 Proposed fix
- "react-native-worklets-core": "^1.6.2" + "react-native-worklets": "~0.6.0"app/(tabs)/autocheckin.tsx (4)
67-105: Prevent overlapping status requests + double-fetch (stale UI risk).
fetchWaitlistStatus()is triggered by bothuseFocusEffectanduseEffect([isLoggedIn]), and there’s no “latest request wins” guard—so concurrent requests can apply out of order and also double-fetch on mount.Proposed fix (single fetch trigger + stale-response guard + loader safety)
export default function AutoCheckinScreen() { @@ const [alreadyOnPressed, setAlreadyOnPressed] = useState(false); const alreadyOnTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); + const statusRequestSeqRef = useRef(0); const fetchWaitlistStatus = useCallback(async ({ showLoader = false } = {}) => { + const requestSeq = ++statusRequestSeqRef.current; if (showLoader) { setIsStatusLoading(true); } - const response = await checkoutApi<WaitlistStatusResponse>( - '/api/account/waitlist?service=autocheckin' - ); - - // Check for auth_error - this is expected when not signed in - if (response.data?.auth_error) { - // Treat as unauthenticated state, not an error - setWaitlistStatus({ success: true, status: 'unauthenticated', canJoin: true }); - setWaitlistError(null); - } else if (response.success && response.data?.success) { - setWaitlistStatus(response.data); - setWaitlistError(null); - } else { - const message = - response.error || (response.data as { msg?: string })?.msg || 'Unable to load status.'; - setWaitlistStatus(null); - setWaitlistError(message); - } - - if (showLoader) { - setIsStatusLoading(false); - } + try { + const response = await checkoutApi<WaitlistStatusResponse>( + '/api/account/waitlist?service=autocheckin' + ); + + // Ignore stale responses + if (requestSeq !== statusRequestSeqRef.current) return; + + if (response.data?.auth_error) { + setWaitlistStatus({ success: true, status: 'unauthenticated', canJoin: true }); + setWaitlistError(null); + } else if (response.success && response.data?.success) { + setWaitlistStatus(response.data); + setWaitlistError(null); + } else { + const message = + response.error || (response.data as { msg?: string })?.msg || 'Unable to load status.'; + setWaitlistStatus(null); + setWaitlistError(message); + } + } finally { + if (showLoader && requestSeq === statusRequestSeqRef.current) { + setIsStatusLoading(false); + } + } }, []); useFocusEffect( useCallback(() => { const refreshAndFetch = async () => { await getStoredToken(); // Refresh auth state from storage - fetchWaitlistStatus({ showLoader: true }); + await fetchWaitlistStatus({ showLoader: true }); }; refreshAndFetch(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [fetchWaitlistStatus]) + }, [fetchWaitlistStatus, getStoredToken, isLoggedIn]) ); - useEffect(() => { - fetchWaitlistStatus({ showLoader: true }); - }, [isLoggedIn, fetchWaitlistStatus]); + // Removed: avoid double-fetch; focus effect will re-run while focused when isLoggedIn changes.Also applies to: 115-117
34-35: HandlecanJoin === falseexplicitly (don’t render Join when server forbids it).
waitlistStatefalls through to'CAN_JOIN'even whenwaitlistStatus.canJoinis explicitlyfalse, so the UI can show “Join the Waitlist” and then fail on POST.Proposed fix (new UI state)
-type WaitlistUiState = 'SIGNED_OUT' | 'CAN_JOIN' | 'ON_WAITLIST' | 'ACCEPTED'; +type WaitlistUiState = 'SIGNED_OUT' | 'CAN_JOIN' | 'CANNOT_JOIN' | 'ON_WAITLIST' | 'ACCEPTED'; const waitlistState: WaitlistUiState = useMemo(() => { @@ - if (waitlistStatus?.canJoin) { - return 'CAN_JOIN'; - } - - return 'CAN_JOIN'; + if (waitlistStatus?.canJoin) return 'CAN_JOIN'; + if (waitlistStatus?.canJoin === false) return 'CANNOT_JOIN'; + return 'CAN_JOIN'; // default if server doesn't specify }, [isLoggedIn, waitlistStatus]);You’ll also need to add
'CANNOT_JOIN'handling insubtext,pillText, andrenderActionButton(e.g., disabled button + support CTA).Also applies to: 119-144, 146-176, 276-343
260-266: HandleLinking.openURL()failures (unsupported URLs/rejections).
Linking.openURL()can reject andcanOpenURLcan be false; right now failures are silent.Proposed fix
- const handleOpenAutoCheckin = useCallback(() => { - Linking.openURL('https://checkout.ac/auto'); - }, []); + const handleOpenAutoCheckin = useCallback(async () => { + const url = 'https://checkout.ac/auto'; + try { + const supported = await Linking.canOpenURL(url); + if (!supported) return toast.error('Unable to open link'); + await Linking.openURL(url); + } catch { + toast.error('Unable to open link'); + } + }, [toast]); - const handleSupportLink = useCallback(() => { - Linking.openURL('https://checkout.ac/support?pre=autocheckin'); - }, []); + const handleSupportLink = useCallback(async () => { + const url = 'https://checkout.ac/support?pre=autocheckin'; + try { + const supported = await Linking.canOpenURL(url); + if (!supported) return toast.error('Unable to open link'); + await Linking.openURL(url); + } catch { + toast.error('Unable to open link'); + } + }, [toast]);
268-272: Ensure refresh spinner always stops (even if something throws).Wrap
await fetchWaitlistStatus()intry/finallysosetRefreshing(false)always runs.Proposed fix
const onRefresh = useCallback(async () => { setRefreshing(true); - await fetchWaitlistStatus(); - setRefreshing(false); + try { + await fetchWaitlistStatus(); + } finally { + setRefreshing(false); + } }, [fetchWaitlistStatus]);
🧹 Nitpick comments (7)
app/(tabs)/history/index.tsx (2)
35-35: Type cast indicates a mismatch betweenThemeColorsandtypeof Colors.light.The cast
theme as typeof Colors.lightsuggests thethemeobject fromuseAppTheme()doesn't match the expected type forcreateHistoryStyles. This is related to the type alias change inhistory.styles.ts.If you address the
theme === Colors.lightcomparison issue inhistory.styles.tsby passing anisLightThemeboolean, this cast would become unnecessary and you could passthemedirectly.
161-161: Consider using<>(Fragment) instead of<View>when no styling is needed.The wrapper
<View>on lines 161 and 235 doesn't apply any styles or props. Using<React.Fragment>(or<>...</>) avoids creating an extra native view node, which is slightly more performant.Suggested change
- <View> + <> {historyData.pastCodes.map(code => ( // ... content ... ))} {/* Loading indicator at bottom */} {historyData.pagination.hasMore && ( // ... content ... )} - </View> + </>Also applies to: 235-235
app/(tabs)/settings.tsx (2)
91-91: Consider removing debug log before production release.This
console.logappears to be a debug artifact that logs potentially sensitive account deletion response data. Consider removing it or gating it behind a development environment check.
361-414: Consider extracting inline styles to StyleSheet for performance.The modals use inline style objects that are recreated on every render. While functional, extracting these to the existing
stylesobject or a dedicated modal styles module would improve performance and maintainability.app/(tabs)/_layout.tsx (1)
30-41: Theas anycast could be improved with proper typing.Line 34 uses
as anyfor route params. Consider defining a proper type for the expected params structure, or use a more specific assertion if the params shape is known.Additionally, the logic relies on
wasDisconnectedwhich istruewhen connection is restored (not when disconnected), which may cause confusion for future maintainers. A comment clarifying this inverse naming could help.🔧 Suggested improvement
+ // Note: wasDisconnected is true momentarily when connection is RESTORED + // (signaling we were disconnected and are now back online) useEffect(() => { if (wasDisconnected && !previousWasDisconnectedRef.current) { // Connection was just restored - refresh the current tab - const currentParams = (route.params as any) || {}; + const currentParams = (route.params as Record<string, unknown>) || {}; navigation.setParams({ ...currentParams, refresh: Date.now(), });components/ConnectionErrorBanner.tsx (1)
9-31: Hard-coded tab bar height + URL parsing assumptions.
DEFAULT_TAB_BAR_HEIGHT = 49can drift (custom tab bars / Android nav variations). Consider wiring actual tab bar height (e.g., via a prop oruseBottomTabBarHeight()if available).new URL(apiUrl)depends onapiUrlincluding a scheme; you already catch, but if custom URLs are common, consider normalizing before parsing.Also applies to: 33-45
contexts/ConnectionContext.tsx (1)
45-78: Clear irrelevant fields when switching error types (prevents stale UI state).When switching to
'api_error'/'unavailable'/'connection', consider clearing fields that no longer apply (e.g.,statusCode,unavailableMessage,errorMessage) to avoid consumers accidentally rendering stale info.Example cleanup
const setConnectionError = useCallback((error: string, statusCode?: number) => { setConnectionState(prev => ({ ...prev, type: 'api_error', isVisible: true, errorMessage: error, + unavailableMessage: undefined, + unavailableLink: undefined, disconnectedAt: prev.disconnectedAt || Date.now(), statusCode, })); }, []); const setUnavailable = useCallback((message: string, link?: string) => { setConnectionState(prev => ({ ...prev, type: 'unavailable', isVisible: true, + errorMessage: undefined, + statusCode: undefined, unavailableMessage: message, unavailableLink: link, disconnectedAt: prev.disconnectedAt || Date.now(), })); }, []); const setConnectionLost = useCallback(() => { setConnectionState(prev => ({ ...prev, type: 'connection', isVisible: true, errorMessage: undefined, unavailableMessage: undefined, unavailableLink: undefined, + statusCode: undefined, disconnectedAt: prev.disconnectedAt || Date.now(), })); }, []);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
app.jsonapp/(tabs)/_layout.tsxapp/(tabs)/autocheckin.tsxapp/(tabs)/history/help.tsxapp/(tabs)/history/index.tsxapp/(tabs)/settings.tsxcomponents/ConnectionErrorBanner.tsxcomponents/ConnectionErrorOverlay.tsxcomponents/ExternalLink.tsxcomponents/ui/IconSymbol.tsxcontexts/ConnectionContext.tsxhooks/useThemeColor.tspackage.jsonstyles/autocheckin.styles.tsstyles/history.styles.tsstyles/settings.styles.tsstyles/shared.styles.tsutils/api.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app.json
- components/ConnectionErrorOverlay.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
styles/autocheckin.styles.ts (1)
constants/Colors.ts (1)
Colors(34-83)
app/(tabs)/_layout.tsx (5)
hooks/useConnectionMonitor.ts (1)
useConnectionMonitor(11-112)components/VersionAwareTabBar.tsx (1)
VersionAwareTabBar(7-16)components/ConnectionErrorBanner.tsx (1)
ConnectionErrorBanner(12-125)components/UnavailableOverlay.tsx (1)
UnavailableOverlay(8-163)components/ConnectionErrorOverlay.tsx (1)
ConnectionErrorOverlay(16-280)
components/ConnectionErrorBanner.tsx (4)
hooks/useAppTheme.ts (1)
useAppTheme(4-12)contexts/ConnectionContext.tsx (1)
useConnectionContext(126-132)constants/api.ts (1)
getApiUrl(8-16)components/ThemedText.tsx (1)
ThemedText(11-34)
app/(tabs)/settings.tsx (2)
hooks/useApiUrl.ts (1)
useApiUrl(4-69)constants/api.ts (1)
getApiUrl(8-16)
styles/history.styles.ts (1)
constants/Colors.ts (1)
ThemeColors(9-32)
utils/api.ts (3)
utils/userAgent.ts (1)
getHeadersWithUserAgent(70-80)constants/api.ts (1)
getApiUrl(8-16)contexts/ConnectionContext.tsx (1)
getConnectionContext(32-34)
hooks/useThemeColor.ts (1)
constants/Colors.ts (1)
Colors(34-83)
🔇 Additional comments (14)
hooks/useThemeColor.ts (1)
9-15: Good type safety improvement.The
SimpleColorKeystype correctly excludes nested object keys (goodButton,badButton) ensuring the hook always returns astringrather than potentially returning an object. This aligns with the actual runtime behavior and improves type safety.One minor consideration: the exclusion list is hardcoded, so if new nested objects are added to
Colors, this type would need manual updates. An alternative would be to derive this dynamically:type SimpleColorKeys = { [K in keyof typeof Colors.light]: typeof Colors.light[K] extends string ? K : never }[keyof typeof Colors.light];However, given the current simplicity and clear documentation, the explicit exclusion is acceptable.
styles/history.styles.ts (1)
2-4: Good alignment with centralized theming.The import and type alias update to use
ThemeColorsimproves consistency with the broader theming approach across the codebase.app/(tabs)/history/help.tsx (1)
4-4: LGTM!The import path update aligns with the relocated styles module.
app/(tabs)/history/index.tsx (1)
13-13: LGTM!The import path update is consistent with the styles module relocation.
components/ui/IconSymbol.tsx (1)
6-6: Correct type fix for font-based icon component.The change from
ViewStyletoTextStyleis appropriate sinceMaterialIconsfrom@expo/vector-iconsis a font-based icon component built onText. This provides more accurate typing. The iOS variant (IconSymbol.ios.tsx) correctly retainsViewStylefor theSymbolViewcomponent, which is View-based.Verification confirms no breaking changes: the only existing usage in
Collapsible.tsxpasses atransformproperty, which is valid forTextStyle.components/ExternalLink.tsx (1)
1-13: LGTM!The type cast to
Hrefis appropriate for expo-router compatibility. The component acceptshrefasstringfor a cleaner external API while correctly satisfying theLinkcomponent's type requirements internally.app/(tabs)/settings.tsx (2)
22-36: LGTM!The component setup is well-organized with clear state management for API URL configuration and modals. The use of
useApiUrlhook for centralized API URL management is a good pattern.
38-55: LGTM!The focus effect correctly refreshes both auth state and API URL when the screen gains focus. The async loading pattern with error handling is appropriate.
styles/autocheckin.styles.ts (2)
1-22: LGTM!The theming module is well-structured with proper platform-specific shadow handling. The
surfaceShadowhelper correctly usesPlatform.selectfor iOS shadows vs Android elevation, with a safe fallback.
24-220: LGTM!The
createAutoCheckinStylesfactory provides comprehensive, theme-aware styles with proper dark mode support via theisDarkoption. The approach is consistent with other style modules in the codebase.app/(tabs)/_layout.tsx (2)
54-71: LGTM!The tab configuration is well-structured:
href: nullcorrectly hides the index tab from navigation- The refresh listener pattern using
Date.now()as a param effectively triggers screen updates on tab press
100-115: LGTM!The connection overlay components are properly positioned at the end of the layout, ensuring they render above the tab content. The
flex: 1container style correctly allows the layout to fill available space.app/(tabs)/autocheckin.tsx (2)
64-113: Timeout lifecycle management looks solid.Clearing the timeout on unmount (and before setting a new one) prevents leaks and state updates after unmount.
15-15: Verify if backend treatsservice=autocheckinandwaitlist='AutoCheckin'as equivalent identifiers.Status fetch uses query parameter
service=autocheckin(lowercase), while join/leave operations use POST bodywaitlist: 'AutoCheckin'(PascalCase). Confirm with backend code or API documentation whether case sensitivity or parameter name differences cause identifier mismatches that prevent users from joining after checking status.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Only show for connection and api_error types (not unavailable) | ||
| const shouldShowError = isVisible && (type === 'connection' || type === 'api_error'); | ||
|
|
||
| // Show either error banner or success banner | ||
| const shouldShow = shouldShowError || showSuccessBanner; | ||
|
|
||
| // Calculate tab bar height: default height + safe area bottom inset | ||
| const tabBarHeight = DEFAULT_TAB_BAR_HEIGHT + (Platform.OS === 'ios' ? insets.bottom : 0); | ||
|
|
||
| // Extract domain from API URL | ||
| useEffect(() => { | ||
| const fetchDomain = async () => { | ||
| try { | ||
| const apiUrl = await getApiUrl(); | ||
| const url = new URL(apiUrl); | ||
| setApiDomain(url.hostname); | ||
| } catch { | ||
| // Fallback to default if parsing fails | ||
| setApiDomain('checkout.ac'); | ||
| } | ||
| }; | ||
| fetchDomain(); | ||
| }, []); | ||
|
|
||
| // Detect when connection is restored (error -> success transition) | ||
| useEffect(() => { | ||
| // If we had an error showing and now it's gone, show success banner | ||
| if (previousShouldShowRef.current && !shouldShowError) { | ||
| // Clear any existing timeout | ||
| if (successTimeoutRef.current) { | ||
| clearTimeout(successTimeoutRef.current); | ||
| } | ||
|
|
||
| // Show success banner | ||
| setShowSuccessBanner(true); | ||
|
|
||
| // Hide success banner after 2.5 seconds | ||
| successTimeoutRef.current = setTimeout(() => { | ||
| setShowSuccessBanner(false); | ||
| successTimeoutRef.current = null; | ||
| }, 2500); | ||
| } | ||
|
|
||
| previousShouldShowRef.current = shouldShowError; | ||
|
|
||
| // Cleanup timeout on unmount | ||
| return () => { | ||
| if (successTimeoutRef.current) { | ||
| clearTimeout(successTimeoutRef.current); | ||
| } | ||
| }; | ||
| }, [shouldShowError]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ConnectionErrorBanner.tsx" -type fRepository: jameshaworthcs/Checkout-App
Length of output: 109
🏁 Script executed:
cat -n ./components/ConnectionErrorBanner.tsxRepository: jameshaworthcs/Checkout-App
Length of output: 6620
"Back online" displays incorrectly when type switches to 'unavailable' (not an actual recovery).
The current condition previousShouldShowRef.current && !shouldShowError triggers when shouldShowError becomes false. Since shouldShowError filters out type === 'unavailable', it becomes false even when isVisible remains true. This shows the success banner for a visibility state change that isn't a genuine connection recovery.
The fix should check !isVisible instead to only show the success banner when the error UI is actually dismissed:
Proposed fix
- useEffect(() => {
- // If we had an error showing and now it's gone, show success banner
- if (previousShouldShowRef.current && !shouldShowError) {
+ useEffect(() => {
+ // Only show success when the error UI is actually dismissed (not when switching to "unavailable")
+ if (previousShouldShowRef.current && !isVisible) {
// Clear any existing timeout
if (successTimeoutRef.current) {
clearTimeout(successTimeoutRef.current);
}
@@
previousShouldShowRef.current = shouldShowError;
@@
- }, [shouldShowError]);
+ }, [isVisible, shouldShowError]);🤖 Prompt for AI Agents
In `@components/ConnectionErrorBanner.tsx` around lines 23 - 74, The success
banner is shown when shouldShowError becomes false (which also happens when type
becomes 'unavailable'), so change the recovery check in the effect to only
trigger when the error UI is actually dismissed by checking visibility instead:
replace the condition "if (previousShouldShowRef.current && !shouldShowError)"
with "if (previousShouldShowRef.current && !isVisible)" inside the useEffect
that manages successTimeoutRef and setShowSuccessBanner, leaving the rest of the
timeout/cleanup logic intact and keeping previousShouldShowRef.current updated
as before.
| // Global reference to access context outside React components | ||
| let globalConnectionContext: ConnectionContextType | undefined; | ||
|
|
||
| export function getConnectionContext(): ConnectionContextType | undefined { | ||
| return globalConnectionContext; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i "connectioncontext"Repository: jameshaworthcs/Checkout-App
Length of output: 101
🏁 Script executed:
cat -n contexts/ConnectionContext.tsx | head -150Repository: jameshaworthcs/Checkout-App
Length of output: 4818
🏁 Script executed:
rg "getConnectionContext" --type typescript --type tsx -nRepository: jameshaworthcs/Checkout-App
Length of output: 105
🏁 Script executed:
rg "getConnectionContext" -nRepository: jameshaworthcs/Checkout-App
Length of output: 860
🏁 Script executed:
cat -n utils/api.ts | sed -n '70,95p; 110,125p; 140,155p'Repository: jameshaworthcs/Checkout-App
Length of output: 2815
Move global context mutation into useEffect to prevent concurrent render hazards.
The assignment globalConnectionContext = contextValue on line 121 executes during render, which can publish state from an interrupted render in React concurrent mode. Since getConnectionContext() is called from async error handlers in utils/api.ts, the global reference could temporarily contain uncommitted state. Move this mutation into a useEffect with proper cleanup.
Suggested fix
-import React, { createContext, useContext, useState, useCallback, ReactNode, useRef } from 'react';
+import React, { createContext, useContext, useState, useCallback, ReactNode, useEffect } from 'react';
export function ConnectionProvider({ children }: { children: ReactNode }) {
const [connectionState, setConnectionState] = useState<ConnectionErrorState>({
@@ -118,8 +118,15 @@
setLastCheckedAt,
};
- // Update global reference
- globalConnectionContext = contextValue;
+ // Update global reference after commit
+ useEffect(() => {
+ globalConnectionContext = contextValue;
+ return () => {
+ if (globalConnectionContext === contextValue) {
+ globalConnectionContext = undefined;
+ }
+ };
+ }, [contextValue]);
return <ConnectionContext.Provider value={contextValue}>{children}</ConnectionContext.Provider>;
}🤖 Prompt for AI Agents
In `@contexts/ConnectionContext.tsx` around lines 29 - 34, The global mutation of
globalConnectionContext (currently set from contextValue during render in
ConnectionProvider) must be moved into a React useEffect to avoid publishing
uncommitted render state; update the ConnectionProvider to set
globalConnectionContext = contextValue inside a useEffect that depends on
contextValue, and clear it (set undefined) in the effect cleanup so async error
handlers using getConnectionContext() never observe transient render state.
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.