chore: trim lint warnings and ignore dev paths #897
Conversation
WalkthroughReplaces many empty TypeScript interfaces with type aliases and removes unused props from numerous React components, relaxes ESLint overrides for multiple file groups, applies optional catch binding in many locations, lazy-loads dev screens and tightens navigation typing, and makes small dev-only conditional UI changes and package.json types additions. Changes
Files explicitly changed (patterns): Sequence Diagram(s)sequenceDiagram
participant App as AppNavigation
participant NavIndex as navigation/index.tsx
participant DevModule as app/src/navigation/dev.ts
participant DevScreen as DevPrivateKeyScreen (lazy)
App->>NavIndex: initialize navigation
NavIndex->>NavIndex: assemble navigationScreens
alt __DEV__ true
NavIndex->>DevModule: runtime require('@/navigation/dev')
DevModule->>DevModule: export devScreens (includes React.lazy component)
DevModule-->>NavIndex: devScreens (merged)
else production
DevModule-->>NavIndex: devScreens = {}
end
NavIndex-->>App: navigationScreens (includes devScreens if dev)
App->>DevScreen: navigate -> React.lazy loads module when route rendered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (43)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (11)
app/src/screens/settings/PassportDataInfoScreen.tsx (1)
101-111: Fix falsy value handling to avoid showing “None” for 0Using logical OR will render 0 as “None”. Prefer nullish coalescing so 0 and empty strings are preserved if intentional.
Apply:
- : (metadata?.[key as keyof PassportMetadata] as - | string - | number) || 'None' + : ((metadata?.[key as keyof PassportMetadata] as + | string + | number) ?? 'None')app/src/screens/dev/MockDataScreen.tsx (1)
481-486: Bug: Age decrement button disables on the wrong stateThe “Age” minus button uses expiryYears to control disabled state. This can allow age to go negative and disable at the wrong time.
Apply:
- onPress={() => { - buttonTap(); - setAge(age - 1); - trackEvent(MockDataEvents.DECREASE_AGE); - }} - disabled={expiryYears <= 0} + onPress={() => { + buttonTap(); + setAge(prev => Math.max(0, prev - 1)); + trackEvent(MockDataEvents.DECREASE_AGE); + }} + disabled={age <= 0}app/src/screens/recovery/RecoverWithPhraseScreen.tsx (1)
60-79: Harden recovery flow: handle missing/invalid stored dataJSON.parse on possibly undefined data will throw, leading to a broken recovery flow. Add guards and try/catch to fail gracefully and reset UI state.
Apply:
- const passportDataAndSecret = (await loadPassportDataAndSecret()) as string; - const { passportData, secret } = JSON.parse(passportDataAndSecret); + const passportDataAndSecret = await loadPassportDataAndSecret(); + if (!passportDataAndSecret) { + console.warn('No stored passport data found'); + navigation.navigate('Launch'); + setRestoring(false); + return; + } + let parsed: any; + try { + parsed = JSON.parse(passportDataAndSecret); + } catch (e) { + console.error('Failed to parse stored passport data:', e); + navigation.navigate('Launch'); + setRestoring(false); + return; + } + const { passportData, secret } = parsed; @@ - const { isRegistered, csca } = await isUserRegisteredWithAlternativeCSCA( + const { isRegistered, csca } = await isUserRegisteredWithAlternativeCSCA( passportData, secret as string, ); if (!isRegistered) { console.warn( 'Secret provided did not match a registered passport. Please try again.', ); - reStorePassportDataWithRightCSCA(passportData, csca as string); + if (csca) { + // ensure data is aligned with the correct CSCA before relaunch + await reStorePassportDataWithRightCSCA(passportData, csca as string); + } navigation.navigate('Launch'); setRestoring(false); return; }If reStorePassportDataWithRightCSCA is not async, drop the await.
app/src/screens/prove/ViewFinderScreen.tsx (1)
64-66: Guard against missing QR payload (defensive parsing)Even though we’re in the non-error branch, uri can still be undefined depending on the scanner implementation. Add a guard to avoid a crash.
Apply:
- const validatedParams = parseAndValidateUrlParams(uri!); + if (!uri) { + trackEvent(ProofEvents.QR_SCAN_FAILED, { reason: 'no_uri' }); + setDoneScanningQR(false); + navigation.navigate('QRCodeTrouble'); + return; + } + const validatedParams = parseAndValidateUrlParams(uri);app/src/screens/settings/SettingsScreen.tsx (1)
203-205: Avoid unsafe cast to never — narrow menuRoute before navigatingCasting
menuRoutetoneverbypasses TypeScript and can hide real typing issues. Add a type guard that confirms the route is a stack route (keyof RootStackParamList) before calling navigation.navigate, and remove the unsafe cast.
- File: app/src/screens/settings/SettingsScreen.tsx
- RouteLinks defined around line ~92
- Unsafe cast at the switch default: navigation.navigate(menuRoute as never) (lines ~203-205)
Suggested diff:
+// Narrow menu routes to actual stack routes +const isStackRoute = (route: RouteLinks): route is keyof RootStackParamList => + route !== 'share' && route !== 'email_feedback'; ... case 'ManageDocuments': navigation.navigate('ManageDocuments'); break; default: - navigation.navigate(menuRoute as never); + if (isStackRoute(menuRoute)) { + navigation.navigate(menuRoute); + } else { + console.warn('Unhandled menu route', menuRoute); + } break;Verified RootStackParamList is exported and the referenced route keys (PassportDataInfo, ShowRecoveryPhrase, CloudBackupSettings, ManageDocuments, DevSettings) exist in navigation. Apply the guard to restore type safety.
app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx (2)
117-122: Biometrics hint shows under the wrong conditionThe message says biometrics aren’t supported/are disabled, but it renders when
biometricsAvailableis true. Invert the condition.- {biometricsAvailable && ( + {!biometricsAvailable && ( <> Your device doesn't support biometrics or is disabled for apps and is required for cloud storage. </> )}
41-84: Don't throw from onPress; centralize restoring state with finallyThrowing inside an async onPress can surface as an unhandled rejection; multiple early setRestoring(false) calls are brittle. Centralize setRestoring(false) in a finally, avoid re-throwing (navigate or surface an error instead), and guard against missing CSCA before calling reStorePassportDataWithRightCSCA.
- File to update: app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx (the onRestoreFromCloudPress handler around lines ~41–84)
Suggested change:
const onRestoreFromCloudPress = useCallback(async () => { setRestoring(true); - try { + try { const mnemonic = await download(); const result = await restoreAccountFromMnemonic(mnemonic.phrase); if (!result) { console.warn('Failed to restore account'); trackEvent(BackupEvents.CLOUD_RESTORE_FAILED_UNKNOWN); navigation.navigate('Launch'); - setRestoring(false); return; } const passportDataAndSecret = (await loadPassportDataAndSecret()) as string; const { passportData, secret } = JSON.parse(passportDataAndSecret); const { isRegistered, csca } = await isUserRegisteredWithAlternativeCSCA( passportData, secret, ); if (!isRegistered) { console.warn( 'Secret provided did not match a registered ID. Please try again.', ); trackEvent(BackupEvents.CLOUD_RESTORE_FAILED_PASSPORT_NOT_REGISTERED); navigation.navigate('Launch'); - setRestoring(false); return; } if (!cloudBackupEnabled) { toggleCloudBackupEnabled(); } - reStorePassportDataWithRightCSCA(passportData, csca as string); + if (csca) { + reStorePassportDataWithRightCSCA(passportData, csca as string); + } else { + console.warn('Missing CSCA for restored passport; aborting.'); + trackEvent(BackupEvents.CLOUD_RESTORE_FAILED_UNKNOWN); + navigation.navigate('Launch'); + return; + } trackEvent(BackupEvents.CLOUD_RESTORE_SUCCESS); trackEvent(BackupEvents.ACCOUNT_RECOVERY_COMPLETED); onRestoreFromCloudNext(); - setRestoring(false); } catch (e: unknown) { console.error(e); trackEvent(BackupEvents.CLOUD_RESTORE_FAILED_UNKNOWN); - setRestoring(false); - throw new Error('Something wrong happened during cloud recovery'); + navigation.navigate('Launch'); + // do not re-throw from an event handler + } finally { + setRestoring(false); + } }, [ download, restoreAccountFromMnemonic, cloudBackupEnabled, onRestoreFromCloudNext, navigation, toggleCloudBackupEnabled, ]);Tests: I could not find existing tests for this screen — please add/verify Jest tests that cover success, known failure (e.g., unregistered passport), and exception paths (ensuring restoring disables and navigation/fallback occurs). I can scaffold those tests if you want.
app/src/screens/passport/PassportNFCScanScreen.tsx (2)
215-221: Avoid sending sensitive or large passport data to analyticsThe analytics payload includes DSC material and key identifiers. Even if not directly PII, these are security-sensitive and can be large/unbounded. This risks analytics quota overages and potential leakage of certificate internals.
Consider logging only minimal, non-identifying signals (e.g., booleans, sizes) instead of full/partial objects. Example refactor:
- let dscObject; - try { - dscObject = { dsc: passportMetadata.dsc }; - } catch (error) { - console.error('Failed to parse dsc:', error); - dscObject = {}; - } + const dscPresent = Boolean(passportMetadata.dsc);trackEvent(PassportEvents.PASSPORT_PARSED, { success: true, data_groups: passportMetadata.dataGroups, dg1_size: passportMetadata.dg1Size, dg1_hash_size: passportMetadata.dg1HashSize, dg1_hash_function: passportMetadata.dg1HashFunction, dg1_hash_offset: passportMetadata.dg1HashOffset, dg_padding_bytes: passportMetadata.dgPaddingBytes, e_content_size: passportMetadata.eContentSize, e_content_hash_function: passportMetadata.eContentHashFunction, e_content_hash_offset: passportMetadata.eContentHashOffset, signed_attr_size: passportMetadata.signedAttrSize, signed_attr_hash_function: passportMetadata.signedAttrHashFunction, signature_algorithm: passportMetadata.signatureAlgorithm, salt_length: passportMetadata.saltLength, curve_or_exponent: passportMetadata.curveOrExponent, signature_algorithm_bits: passportMetadata.signatureAlgorithmBits, country_code: passportMetadata.countryCode, csca_found: passportMetadata.cscaFound, csca_hash_function: passportMetadata.cscaHashFunction, csca_signature_algorithm: passportMetadata.cscaSignatureAlgorithm, csca_salt_length: passportMetadata.cscaSaltLength, csca_curve_or_exponent: passportMetadata.cscaCurveOrExponent, csca_signature_algorithm_bits: passportMetadata.cscaSignatureAlgorithmBits, - dsc: dscObject, - dsc_aki: passportData.dsc_parsed?.authorityKeyIdentifier, - dsc_ski: passportData.dsc_parsed?.subjectKeyIdentifier, + dsc_present: dscPresent, });If you absolutely need identifiers for deduplication, prefer hashing them client-side and sending only fixed-length digests; otherwise omit entirely.
Also applies to: 223-251
315-319: Implement NFC scan cancellation to avoid orphan sessionsThere’s a TODO to cancel an in-flight scan. Implement it to prevent stuck sessions and improve UX.
- const _cancelScanIfRunning = useCallback(async () => { - // // TODO: cancel if scanning - // setIsNfcSheetOpen(false); - }, []); + const _cancelScanIfRunning = useCallback(async () => { + try { + // Best-effort cancellation; available in react-native-nfc-manager + await NfcManager.cancelTechnologyRequest(); + } catch { + // noop + } finally { + setIsNfcSheetOpen(false); + } + }, []);app/src/screens/settings/ManageDocumentsScreen.tsx (1)
38-41: Remove unused state to avoid unnecessary work and re-renders
_allDocumentsis never read; fetching and setting it twice does extra work and triggers rerenders without UI benefit. You can drop the state and calls togetAllDocuments()here.- const [_allDocuments, setAllDocuments] = useState< - Record<string, { metadata: DocumentMetadata }> - >({}); + // Removed unused state: all documents are not directly rendered here- const docs = await getAllDocuments(); setDocumentCatalog(catalog); - setAllDocuments(docs);- }, [ - loadDocumentCatalog, - getAllDocuments, - setDocumentCatalog, - setAllDocuments, - ]); + }, [loadDocumentCatalog, setDocumentCatalog]);- const docs = await getAllDocuments(); setDocumentCatalog(catalog); - setAllDocuments(docs);Also applies to: 46-49, 56-61, 71-74
app/src/screens/settings/CloudBackupScreen.tsx (1)
72-97: Ensure pending state is cleared and toggle only on successful uploadIf
uploadthrows,pendingremains true and the toggle/“done” event may be skipped inconsistently. Wrap in try/catch/finally and only toggle after a successful upload.const enableCloudBackups = useCallback(async () => { buttonTap(); if (cloudBackupEnabled) { return; } trackEvent(BackupEvents.CLOUD_BACKUP_ENABLE_STARTED); - setPending(true); - - const storedMnemonic = await getOrCreateMnemonic(); - if (!storedMnemonic) { - setPending(false); - return; - } - await upload(storedMnemonic.data); - toggleCloudBackupEnabled(); - trackEvent(BackupEvents.CLOUD_BACKUP_ENABLED_DONE); - setPending(false); + setPending(true); + try { + const storedMnemonic = await getOrCreateMnemonic(); + if (!storedMnemonic) { + return; + } + await upload(storedMnemonic.data); + toggleCloudBackupEnabled(); + trackEvent(BackupEvents.CLOUD_BACKUP_ENABLED_DONE); + } catch (e) { + // Consider tracking a failure event and/or surfacing a user-visible error + console.error('Cloud backup enable failed:', e); + } finally { + setPending(false); + } }, [ cloudBackupEnabled, getOrCreateMnemonic, upload, toggleCloudBackupEnabled, ]);
🧹 Nitpick comments (18)
app/src/screens/settings/PassportDataInfoScreen.tsx (2)
83-86: Stabilize focus effect to avoid unnecessary re-subscriptionsWrap the focus effect in useCallback to prevent it from re-subscribing on every render. This reduces redundant effect runs when the screen regains focus.
Apply:
- useFocusEffect(() => { - trackEvent(DocumentEvents.PASSPORT_INFO_OPENED); - loadData(); - }); + useFocusEffect( + useCallback(() => { + trackEvent(DocumentEvents.PASSPORT_INFO_OPENED); + loadData(); + }, [loadData]), + );
67-81: Add defensive error handling around data loadgetData() can fail or return unexpected shapes. Add try/catch and minimal logging so failures don’t silently no-op.
Apply:
const loadData = useCallback(async () => { if (metadata) { return; } - - const result = await getData(); - - if (!result || !result.data) { - // maybe handle error instead - return; - } - - setMetadata(result.data.passportMetadata!); - trackEvent(DocumentEvents.PASSPORT_METADATA_LOADED); + try { + const result = await getData(); + if (!result?.data?.passportMetadata) { + console.warn('Passport metadata not found in result'); + return; + } + setMetadata(result.data.passportMetadata); + trackEvent(DocumentEvents.PASSPORT_METADATA_LOADED); + } catch (err) { + console.error('Failed to load passport metadata:', err); + } }, [metadata, getData]);app/src/screens/dev/MockDataScreen.tsx (2)
531-536: Optional: enforce non-negative “Expires In” yearsYou already disable the button at 0, but guarding in state update avoids accidental negatives if logic changes.
Apply:
- onPress={() => { - buttonTap(); - setExpiryYears(expiryYears - 1); - trackEvent(MockDataEvents.DECREASE_EXPIRY_YEARS); - }} + onPress={() => { + buttonTap(); + setExpiryYears(prev => Math.max(0, prev - 1)); + trackEvent(MockDataEvents.DECREASE_EXPIRY_YEARS); + }}
337-341: Consider making SKI environment configurable (dev safety)Hardcoding 'staging' can lead to accidental writes to shared environments. Since this is a dev screen, consider reading the environment from a config/env var and defaulting to 'staging' locally.
app/src/screens/recovery/RecoverWithPhraseScreen.tsx (1)
103-108: Avoid passing undefined to TextArea valueTextInput/TextArea expects a string. Passing undefined can cause warnings. Default to an empty string.
Apply:
- value={mnemonic} + value={mnemonic ?? ''}app/src/layouts/ExpandableBottomLayout.tsx (1)
75-75: Make backgroundColor required on FullSectionProps for consistencyTopSection/BottomSection/Layout require backgroundColor, but FullSection now accepts ViewProps only, making backgroundColor optional. This can lead to subtle UI inconsistencies if callers omit it. Recommend enforcing it in the type.
Apply:
-type FullSectionProps = ViewProps; +type FullSectionProps = ViewProps & { backgroundColor: string };app/src/screens/settings/SettingsScreen.tsx (2)
81-83: Typo: “feeback” → “feedback”Minor copy fix for the web-only routes.
- [Feedback, 'Send feeback', 'email_feedback'], + [Feedback, 'Send feedback', 'email_feedback'],
266-268: Use a stable key for social linksAvoid index keys to reduce unnecessary re-renders if order changes.
- {social.map(([Icon, href], i) => ( - <SocialButton key={i} Icon={Icon} href={href} /> + {social.map(([Icon, href]) => ( + <SocialButton key={href} Icon={Icon} href={href} /> ))}app/src/screens/passport/PassportCameraScreen.tsx (1)
4-4: Track scan duration per focus to improve analytics accuracyInitialize/refresh the timer when the screen becomes focused. This avoids overstating durations across revisits or long-lived mounts.
-import React, { useCallback, useRef } from 'react'; +import React, { useCallback, useRef, useEffect } from 'react'; ... const scanStartTimeRef = useRef(Date.now()); + useEffect(() => { + if (isFocused) { + scanStartTimeRef.current = Date.now(); + } + }, [isFocused]);Also applies to: 36-38
app/src/screens/passport/PassportNFCScanScreen.tsx (1)
147-158: Make the PACE polling heuristic robust to case/undefined valuesAs written, the concatenation assumes uppercase values and defined strings. Small guard and normalization improves reliability across locales and data sources.
- const usePacePolling = (): boolean => { - const { usePacePolling: usePacePollingParam } = route.params ?? {}; - const shouldUsePacePolling = documentType + countryCode === 'IDFRA'; + const usePacePolling = (): boolean => { + const { usePacePolling: usePacePollingParam } = route.params ?? {}; + const docKey = `${documentType ?? ''}${countryCode ?? ''}`.toUpperCase(); + const shouldUsePacePolling = docKey === 'IDFRA';app/src/screens/settings/CloudBackupScreen.tsx (1)
175-181: Type the navigation hook for better safetyTyping
useNavigationhere preserves route correctness and avoids accidental navigation to invalid routes.- const navigation = useNavigation(); + const navigation = useNavigation<NativeStackNavigationProp<RootStackParamList>>();Add this import at the top of the file:
import type { NativeStackNavigationProp } from '@react-navigation/native-stack';app/src/screens/prove/ConfirmBelongingScreen.tsx (2)
26-30: Prefer no-prop component signature; drop redundant prop type aliasThis screen doesn’t consume route props. To reduce noise and stay consistent with the rest of the PR (e.g., PassportOnboardingScreen), simplify to React.FC without the unused StaticScreenProps alias.
Apply this diff:
-type ConfirmBelongingScreenProps = StaticScreenProps<Record<string, never>>; +// No route params expected -const ConfirmBelongingScreen: React.FC<ConfirmBelongingScreenProps> = () => { +const ConfirmBelongingScreen: React.FC = () => {Follow-up:
- Remove the now-unused import
StaticScreenPropson Line 6 to avoid lint complaints.
46-75: Prevent double-submission while requesting notification permissionYou already track
_requestingPermissionbut it isn’t used to gate user interaction. This can lead to duplicate presses and repeated navigation while permission is being requested.Consider:
- Disabling the button when
_requestingPermissionis true (in addition to!isReadyToProve).- Optionally reflect a spinner/message for the requesting state.
Example (outside selected lines):
<PrimaryButton trackEvent={PassportEvents.OWNERSHIP_CONFIRMED} onPress={onOkPress} disabled={!isReadyToProve || _requestingPermission} > {isReadyToProve && !_requestingPermission ? ( 'Confirm' ) : ( <View style={{ flexDirection: 'row', alignItems: 'center' }}> <ActivityIndicator color={black} style={{ marginRight: 8 }} /> <Description color={black}> { _requestingPermission ? 'Requesting permission' : 'Preparing verification' } </Description> </View> )} </PrimaryButton>app/src/layouts/SimpleScrolledTitleLayout.tsx (2)
14-21: Rename prop type to match component for clarityThe alias name DetailListProps doesn’t align with the component name SimpleScrolledTitleLayout. Renaming improves readability and discoverability.
Apply this diff here:
-type DetailListProps = PropsWithChildren<{ +type SimpleScrolledTitleLayoutProps = PropsWithChildren<{ title: string; onDismiss: () => void; secondaryButtonText?: string; onSecondaryButtonPress?: () => void; header?: React.ReactNode; footer?: React.ReactNode; }>;And update the usage (outside selected lines):
}): SimpleScrolledTitleLayoutProps) {
14-21: Stronger typing for secondary action couplingModel the dependency between secondaryButtonText and onSecondaryButtonPress to catch mismatches at compile time.
Example (outside selected lines):
type SecondaryAction = | { secondaryButtonText?: never; onSecondaryButtonPress?: never } | { secondaryButtonText: string; onSecondaryButtonPress: () => void }; type SimpleScrolledTitleLayoutProps = PropsWithChildren< { title: string; onDismiss: () => void; header?: React.ReactNode; footer?: React.ReactNode; } & SecondaryAction >;This ensures if a text is provided, the handler is mandatory, and vice versa.
app/src/screens/passport/PassportOnboardingScreen.tsx (2)
44-50: Avoid dangling timers by clearing the replay timeout on unmountEach animation completion schedules a setTimeout. While the optional chaining will prevent crashes post-unmount, it’s cleaner to clear pending timers to avoid stray callbacks.
Example (outside selected lines):
const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); useEffect(() => () => { if (timeoutRef.current) clearTimeout(timeoutRef.current); }, []); <LottieView onAnimationFinish={() => { timeoutRef.current = setTimeout(() => { animationRef.current?.play(); }, 5000); }} ... />
24-25: Optional: strengthen navigation typingConsider providing a typed navigation hook for this screen to benefit from route name/params safety.
Example (outside selected lines):
import type { NativeStackNavigationProp } from '@react-navigation/native-stack'; import type { RootStackParamList } from '@/navigation'; const navigation = useNavigation<NativeStackNavigationProp<RootStackParamList>>();app/src/navigation/index.tsx (1)
76-85: Optional: memoize trackScreen callbackMinor perf nit: wrap trackScreen with useCallback and depend on navigationRef.isReady()/getCurrentRoute to avoid re-creating the handler each render. Low impact but keeps renders tidy.
Example (outside selected lines):
const trackScreen = useCallback(() => { const currentRoute = navigationRef.getCurrentRoute(); if (currentRoute) { if (__DEV__) console.log(`Screen View: ${currentRoute.name}`); trackScreenView(`${currentRoute.name}`, { screenName: currentRoute.name }); } }, []);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (31)
app/.eslintrc.cjs(2 hunks)app/scripts/mobile-deploy-confirm.cjs(2 hunks)app/src/components/typography/Additional.tsx(1 hunks)app/src/components/typography/Caution.tsx(1 hunks)app/src/components/typography/Description.tsx(1 hunks)app/src/hooks/useRecoveryPrompts.ts(1 hunks)app/src/layouts/AppLayout.tsx(1 hunks)app/src/layouts/ExpandableBottomLayout.tsx(1 hunks)app/src/layouts/SimpleScrolledTitleLayout.tsx(1 hunks)app/src/navigation/index.tsx(1 hunks)app/src/screens/aesop/PassportOnboardingScreen.tsx(1 hunks)app/src/screens/dev/DevPrivateKeyScreen.tsx(1 hunks)app/src/screens/dev/MockDataScreen.tsx(1 hunks)app/src/screens/misc/ModalScreen.tsx(1 hunks)app/src/screens/passport/PassportCameraScreen.tsx(1 hunks)app/src/screens/passport/PassportNFCScanScreen.tsx(1 hunks)app/src/screens/passport/PassportNFCScanScreen.web.tsx(1 hunks)app/src/screens/passport/PassportOnboardingScreen.tsx(1 hunks)app/src/screens/prove/ConfirmBelongingScreen.tsx(1 hunks)app/src/screens/prove/ViewFinderScreen.tsx(1 hunks)app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx(1 hunks)app/src/screens/recovery/AccountRecoveryScreen.tsx(1 hunks)app/src/screens/recovery/RecoverWithPhraseScreen.tsx(1 hunks)app/src/screens/recovery/SaveRecoveryPhraseScreen.tsx(1 hunks)app/src/screens/settings/CloudBackupScreen.tsx(1 hunks)app/src/screens/settings/ManageDocumentsScreen.tsx(1 hunks)app/src/screens/settings/PassportDataInfoScreen.tsx(1 hunks)app/src/screens/settings/SettingsScreen.tsx(1 hunks)app/src/screens/settings/ShowRecoveryPhraseScreen.tsx(1 hunks)app/src/vite-env.d.ts(0 hunks)app/tamagui.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/vite-env.d.ts
🧰 Additional context used
📓 Path-based instructions (1)
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit Configuration File
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/aesop/PassportOnboardingScreen.tsxapp/src/components/typography/Description.tsxapp/src/screens/dev/DevPrivateKeyScreen.tsxapp/src/screens/dev/MockDataScreen.tsxapp/src/screens/recovery/SaveRecoveryPhraseScreen.tsxapp/src/screens/prove/ConfirmBelongingScreen.tsxapp/src/screens/prove/ViewFinderScreen.tsxapp/src/screens/recovery/AccountRecoveryChoiceScreen.tsxapp/src/screens/settings/ManageDocumentsScreen.tsxapp/src/layouts/ExpandableBottomLayout.tsxapp/src/screens/recovery/RecoverWithPhraseScreen.tsxapp/src/screens/passport/PassportNFCScanScreen.tsxapp/src/screens/settings/SettingsScreen.tsxapp/src/screens/passport/PassportNFCScanScreen.web.tsxapp/src/screens/recovery/AccountRecoveryScreen.tsxapp/src/screens/settings/CloudBackupScreen.tsxapp/src/screens/misc/ModalScreen.tsxapp/src/screens/passport/PassportOnboardingScreen.tsxapp/src/components/typography/Caution.tsxapp/src/hooks/useRecoveryPrompts.tsapp/src/layouts/AppLayout.tsxapp/src/components/typography/Additional.tsxapp/src/screens/settings/PassportDataInfoScreen.tsxapp/src/layouts/SimpleScrolledTitleLayout.tsxapp/src/screens/settings/ShowRecoveryPhraseScreen.tsxapp/src/screens/passport/PassportCameraScreen.tsxapp/src/navigation/index.tsx
🔇 Additional comments (26)
app/src/components/typography/Additional.tsx (1)
11-11: Switching to a type alias for props is the right callReplacing an empty interface with
type AdditionalProps = TextPropsremoves an unnecessary symbol with no behavioral change. Consistent with the PR’s objective to trim lint warnings.app/tamagui.config.ts (1)
76-76: Whitespace-only change — safe to mergeNo functional impact; aligns with style cleanup goals.
app/src/hooks/useRecoveryPrompts.ts (1)
46-50: Catch-without-binding is safe — TS targets ES2020+; adding a dev-only trace is fineBrief check: repo tsconfig targets are ES2020/ESNext across packages (so optional catch binding is supported). No Babel config found that would downlevel this. Optional dev-only logging is safe and low-noise.
- File: app/src/hooks/useRecoveryPrompts.ts — around lines 46–50
Suggested diff (apply if you want the low-noise dev trace):
- } catch { - // Silently fail to avoid breaking the hook - // If we can't get documents, we shouldn't show the prompt - return; - } + } catch { + // Silently fail to avoid breaking the hook + // If we can't get documents, we shouldn't show the prompt + if (__DEV__) { + // Helps debug the "pops up too often" TODO without noisy prod logs + console.debug('[useRecoveryPrompts] getAllDocuments failed; suppressing prompt'); + } + return; + }app/scripts/mobile-deploy-confirm.cjs (1)
65-69: Approve: optional catch binding is safe — Node >=22 confirmedOptional catch binding (catch {}) is fine here — it removes the unused error binding while preserving the warning behavior.
Files reviewed
- app/scripts/mobile-deploy-confirm.cjs — lines 65–69 and 92–98 (catch blocks simplified; console.warn preserved)
Verification evidence
- package.json (repo root): "engines".node = ">=22 <23"
- .nvmrc (repo root): 22
- CI workflows pin Node 22 (examples: .github/workflows/npm-publish.yml, .github/workflows/contracts.yml, various .github/actions/* action.yml entries use node-version: 22)
No changes required; safe to approve and merge.
app/src/screens/misc/ModalScreen.tsx (1)
48-50: Type alias for screen props is consistent and clearerReplacing an empty interface with
type ModalScreenProps = StaticScreenProps<...>matches the repo-wide pattern and removes a lint smell without changing behavior.app/src/screens/settings/PassportDataInfoScreen.tsx (1)
62-62: LGTM on prop-less component conversionSwitching to React.FC with no props is consistent with the PR’s goal and avoids maintaining empty prop types. No runtime impact.
app/src/screens/dev/MockDataScreen.tsx (1)
245-245: LGTM on prop-less component conversionConforms to the repo-wide simplification. No behavior change.
app/src/components/typography/Caution.tsx (1)
10-10: No export needed — CautionProps is only used locallyI ran ripgrep for "CautionProps"; matches are only in app/src/components/typography/Caution.tsx (type declaration and the local component usage on lines ~10 and ~12). There are no external imports, so exporting the alias isn't required. Keep as-is unless you intend to expose this type from the package — then apply the suggested diff.
Likely an incorrect or invalid review comment.
app/src/screens/recovery/RecoverWithPhraseScreen.tsx (1)
30-30: LGTM on prop-less component conversionSignature simplification aligns with the PR goals. No behavior change.
app/src/screens/prove/ViewFinderScreen.tsx (1)
32-32: LGTM on prop-less component conversionMatches the repo-wide cleanup and has no behavioral side effects.
app/src/components/typography/Description.tsx (1)
12-12: No external usages found — exporting not requiredRepo-wide search for
DescriptionPropsonly matched inside the same file:
- app/src/components/typography/Description.tsx — line 12 (type) and line 14 (local usage)
Action: no change required. Keep it non-exported if you intend it to be internal; otherwise export it with the diff below.
Optional export (only if you want it public):
-type DescriptionProps = TextProps; +export type DescriptionProps = TextProps;app/src/screens/aesop/PassportOnboardingScreen.tsx (1)
23-23: LGTM: Props interface removal aligns with PR intentClean no-props component signature; no runtime impact. Consistent with other screens in this PR.
app/src/layouts/AppLayout.tsx (1)
7-7: ConnectedAppLayoutProps is internal — no export neededRepo search found only local uses in app/src/layouts/AppLayout.tsx:
- app/src/layouts/AppLayout.tsx:7 — declaration
- app/src/layouts/AppLayout.tsx:11 — usage (function param)
Conclusion: the original suggestion to export the type is incorrect for the current codebase; no change required. If you do want it public later, apply:
-type ConnectedAppLayoutProps = PropsWithChildren; +export type ConnectedAppLayoutProps = PropsWithChildren;Likely an incorrect or invalid review comment.
app/src/screens/recovery/AccountRecoveryScreen.tsx (1)
16-16: Props removal LGTMConverting to a no-props component aligns with the PR’s goal and keeps the surface clean.
app/src/screens/settings/SettingsScreen.tsx (1)
142-142: Props removal LGTMSignature simplification to a no-props component is consistent with the PR-wide refactor. No runtime implications here.
app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx (1)
30-30: Props removal LGTMNo-props signature matches the broader cleanup. Good.
app/src/screens/passport/PassportCameraScreen.tsx (1)
31-31: Props removal LGTMNo-props component signature matches the rest of the refactor. No behavior change.
app/src/screens/passport/PassportNFCScanScreen.web.tsx (1)
18-18: Props removal LGTMSimplifying to a no-props component is consistent with the PR goals. No runtime risk on web.
app/src/screens/passport/PassportNFCScanScreen.tsx (1)
74-74: Props removal aligns with PR goal — good cleanupSwitching to a no-props React.FC here is consistent with the repo-wide refactor and removes dead typings. No runtime impact.
app/src/screens/settings/ManageDocumentsScreen.tsx (1)
266-266: Props cleanup looks goodConverting to a no-props React.FC simplifies the component and aligns with the PR’s objective.
app/src/screens/settings/ShowRecoveryPhraseScreen.tsx (1)
10-10: No-props component refactor acknowledgedSignature change is straightforward and safe; behavior unchanged.
app/src/screens/settings/CloudBackupScreen.tsx (1)
30-35: Interface-to-type refactor is correctSwitching to a type alias for props is consistent with the PR and keeps the external API unchanged.
app/src/screens/recovery/SaveRecoveryPhraseScreen.tsx (1)
18-18: No-props React.FC migration looks goodSignature simplification is consistent with the PR; no behavior change.
app/src/screens/passport/PassportOnboardingScreen.tsx (1)
23-23: LGTM: removing empty props simplifies the componentGood cleanup. The component doesn’t use props, so simplifying the signature eliminates noise.
app/.eslintrc.cjs (1)
172-179: Docs/examples override — glob matches real filesThe override in app/.eslintrc.cjs (files: ['docs/examples/**/*.ts']) is valid — there are .ts files under app/docs/examples/tree-shaking:
- app/docs/examples/tree-shaking/optimal-pattern-example.ts
- app/docs/examples/tree-shaking/granular-circuits-example.ts
- app/docs/examples/tree-shaking/level2-optimal-example.ts
- app/docs/examples/tree-shaking/level3-migration-guide.ts
- app/docs/examples/tree-shaking/level3-optimal-example.ts
No action required.
app/src/navigation/index.tsx (1)
50-54: LGTM: stronger typing for RootStack and navigationRefTying navigationRef to RootStackParamList improves type safety across navigation calls. The ReactNavigation global augmentation with the lint suppression is appropriate here.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
app/.eslintrc.cjs (2)
199-201: Nice: Jest env added for tests override (previously requested).This prevents false “undefined” for describe/it/expect.
216-220: Add Node/CommonJS env and parserOptions for scripts//*.cjs to avoid parsing issues.**Currently only no-console is disabled; without env/parserOptions, ESLint may misparse CJS in scripts, which conflicts with the PR goal of reducing noise.
Apply this diff:
{ // Allow console logging in scripts files: ['scripts/**/*.cjs', 'scripts/*.cjs'], + env: { + node: true, + commonjs: true, + es6: true, + }, + parserOptions: { + ecmaVersion: 2020, + sourceType: 'script', + }, rules: { 'no-console': 'off', + 'no-unused-vars': 'off', + '@typescript-eslint/no-unused-vars': 'off', + '@typescript-eslint/no-explicit-any': 'off', }, },
🧹 Nitpick comments (7)
app/src/utils/logger/nativeLoggerBridge.ts (1)
49-56: Trim or sanitize logged payload on JSON.parse failure to reduce PII leakage and log noise.When parse fails, we currently dump the full string. Consider trimming long messages and avoiding logging potentially sensitive content verbatim.
Apply this diff to bound the logged length:
eventEmitter.addListener('logEvent', (event: NativeLogEvent | string) => { if (typeof event === 'string') { try { const parsedEvent = JSON.parse(event); handleNativeLogEvent(parsedEvent); - } catch { - console.warn('Failed to parse logEvent string:', event); + } catch { + const MAX_LEN = 2000; + const preview = + event.length > MAX_LEN ? event.slice(0, MAX_LEN) + '…<truncated>' : event; + console.warn('Failed to parse logEvent string:', preview); } } else { handleNativeLogEvent(event); } });app/src/utils/proving/provingMachine.ts (1)
425-429: Bare catch is fine; add a small breadcrumb for observability.Fallback navigation is correct, but having a tiny breadcrumb helps diagnose unexpected failures without reintroducing an unused error variable.
Apply this diff to add a generic breadcrumb while keeping bare catch:
- } catch { - if (navigationRef.isReady()) { - navigationRef.navigate('Launch'); - } - } + } catch { + console.warn( + '_handleRegisterErrorOrFailure fallback: hasAnyValidRegisteredDocument failed; navigating to Launch.', + ); + if (navigationRef.isReady()) { + navigationRef.navigate('Launch'); + } + }app/src/providers/passportDataProvider.tsx (1)
277-284: Bare catch drops error context; log details in dev and avoid leaking identifiersCatching without binding hides real failure modes (permissions, keychain corruption, platform-specific errors). Given these are destructive paths, retaining error context in development is valuable, while production logs should avoid leaking document identifiers (ids are content hashes and can be considered sensitive).
Recommend:
- Bind the error and conditionally include it in logs in development.
- Keep production logs generic and avoid printing full ids.
Example patch for the four catch sites:
- } catch { - console.log(`Document ${doc.id} not found or already cleared`); + } catch (err) { + if (__DEV__) { + console.warn(`Document ${doc.id} not found or already cleared`, err); + } else { + console.log('Document not found or already cleared'); + } }- } catch { - console.log('Document catalog not found or already cleared'); + } catch (err) { + if (__DEV__) { + console.warn('Document catalog not found or already cleared', err); + } else { + console.log('Document catalog not found or already cleared'); + } }- } catch { - console.log(`Document ${documentId} not found or already cleared`); + } catch (err) { + if (__DEV__) { + console.warn(`Document ${documentId} not found or already cleared`, err); + } else { + console.log('Document not found or already cleared'); + } }If you prefer a single pattern, I can factor a tiny helper to standardize dev/production logging across this file.
Also applies to: 288-293, 306-311, 346-350
app/src/screens/prove/ProofRequestStatusScreen.tsx (1)
54-61: Avoid orphaned timeout on unmount by tracking and clearing the delayed cleanSelfApponOkPress schedules a 2s timeout that survives unmount/navigation changes; track it so it can be cleared on blur/unmount. Reusing the existing timerRef (used by the countdown) risks interference; keep a dedicated ref.
Apply within this block:
- const onOkPress = useCallback(() => { + const onOkPress = useCallback(() => { buttonTap(); goHome(); - setTimeout(() => { - cleanSelfApp(); - }, 2000); // Wait 2 seconds to user coming back to the home screen. If we don't wait the appname will change and user will see it. + cleanupRef.current = setTimeout(() => { + cleanSelfApp(); + cleanupRef.current = null; + }, 2000) as unknown as NodeJS.Timeout; // wait for Home to render before cleanup }, [goHome, cleanSelfApp]);And add the following supporting code outside this range:
// near Line 52, alongside timerRef const cleanupRef = useRef<NodeJS.Timeout | null>(null); // add a one-time unmount cleanup effect useEffect(() => { return () => { if (cleanupRef.current) { clearTimeout(cleanupRef.current); cleanupRef.current = null; } }; }, []); // optionally, extend cancelCountdown() to also clear cleanupRef // so blur cancels both timers function cancelCountdown() { if (timerRef.current) { clearTimeout(timerRef.current); timerRef.current = null; } if (cleanupRef.current) { clearTimeout(cleanupRef.current); cleanupRef.current = null; } setCountdown(null); }app/src/screens/dev/DevSettingsScreen.tsx (1)
32-46: Remove unused DevSettingsScreenProps to align with “no-prop components” goalThe screen doesn’t use any props; keeping this interface adds noise and import churn. Simplify the component to a no-prop FC and drop PropsWithChildren import.
-import { cloneElement, isValidElement, type PropsWithChildren } from 'react'; +import { cloneElement, isValidElement } from 'react'; @@ -interface DevSettingsScreenProps extends PropsWithChildren { - color?: string; - width?: number; - justifyContent?: - | 'center' - | 'unset' - | 'flex-start' - | 'flex-end' - | 'space-between' - | 'space-around' - | 'space-evenly'; - userSelect?: 'all' | 'text' | 'none' | 'contain'; - textAlign?: 'center' | 'left' | 'right'; - style?: StyleProp<TextStyle | ViewStyle>; -} +// No propsAnd update the component signature:
-const DevSettingsScreen: React.FC<DevSettingsScreenProps> = ({}) => { +const DevSettingsScreen: React.FC = () => {app/src/screens/settings/ManageDocumentsScreen.tsx (1)
38-41: Remove unused _allDocuments state and reads to trim warnings and unnecessary work.The _allDocuments state is never read; only set. This adds avoidable work and contradicts the PR goal to reduce lint noise.
Apply this diff:
- const [_allDocuments, setAllDocuments] = useState< - Record<string, { metadata: DocumentMetadata }> - >({}); + // Removed unused _allDocuments state const loadPassportDataInfo = useCallback(async () => { setLoading(true); const catalog = await loadDocumentCatalog(); - const docs = await getAllDocuments(); setDocumentCatalog(catalog); - setAllDocuments(docs); trackEvent(DocumentEvents.DOCUMENTS_FETCHED, { count: catalog.documents.length, }); @@ const handleDocumentSelection = async (documentId: string) => { await setSelectedDocument(documentId); // Reload to update UI without loading state for quick operations const catalog = await loadDocumentCatalog(); - const docs = await getAllDocuments(); setDocumentCatalog(catalog); - setAllDocuments(docs); trackEvent(DocumentEvents.DOCUMENT_SELECTED); };Also applies to: 46-49, 71-74
app/.eslintrc.cjs (1)
247-251: **Scope check: broad .cjs and .js override disables several rules.This applies only at the repo root due to the glob, not subfolders. If the intent was to relax rules in nested JS/CJS too, expand to '**/*.{js,cjs}'. Otherwise, consider narrowing to specific config files to avoid muting real issues elsewhere.
Would you like me to propose a more targeted override (e.g., jest.config.js, metro.config.js, babel.config.js)?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (16)
app/.eslintrc.cjs(4 hunks)app/docs/examples/tree-shaking/level2-optimal-example.ts(1 hunks)app/docs/examples/tree-shaking/level3-optimal-example.ts(1 hunks)app/src/navigation/dev.ts(2 hunks)app/src/navigation/index.tsx(3 hunks)app/src/providers/passportDataProvider.tsx(4 hunks)app/src/screens/dev/DevSettingsScreen.tsx(1 hunks)app/src/screens/prove/ProofRequestStatusScreen.tsx(3 hunks)app/src/screens/settings/ManageDocumentsScreen.tsx(2 hunks)app/src/screens/settings/SettingsScreen.tsx(1 hunks)app/src/utils/cloudBackup/helpers.ts(1 hunks)app/src/utils/logger/nativeLoggerBridge.ts(1 hunks)app/src/utils/proving/provingMachine.ts(1 hunks)app/src/utils/testingUtils.ts(2 hunks)app/tamagui.config.ts(1 hunks)app/tests/src/providers/passportDataProvider.test.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/tamagui.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/screens/settings/SettingsScreen.tsx
🧰 Additional context used
📓 Path-based instructions (2)
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit Configuration File
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/providers/passportDataProvider.tsxapp/src/utils/cloudBackup/helpers.tsapp/src/utils/logger/nativeLoggerBridge.tsapp/src/screens/dev/DevSettingsScreen.tsxapp/src/utils/testingUtils.tsapp/src/utils/proving/provingMachine.tsapp/src/screens/prove/ProofRequestStatusScreen.tsxapp/src/screens/settings/ManageDocumentsScreen.tsxapp/src/navigation/dev.tsapp/src/navigation/index.tsx
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit Configuration File
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
app/tests/src/providers/passportDataProvider.test.tsx
🧬 Code Graph Analysis (3)
app/docs/examples/tree-shaking/level2-optimal-example.ts (5)
common/index.ts (2)
PASSPORT_ATTESTATION_ID(36-36)hash(91-91)common/src/constants/index.ts (1)
PASSPORT_ATTESTATION_ID(22-22)common/src/utils/hash/sha.ts (1)
hash(1-1)common/src/utils/index.ts (1)
hash(27-27)app/scripts/version.cjs (1)
data(12-12)
app/src/screens/prove/ProofRequestStatusScreen.tsx (1)
app/src/utils/haptic/index.ts (1)
buttonTap(16-16)
app/docs/examples/tree-shaking/level3-optimal-example.ts (5)
common/index.ts (3)
PASSPORT_ATTESTATION_ID(36-36)hash(91-91)flexiblePoseidon(89-89)common/src/constants/index.ts (1)
PASSPORT_ATTESTATION_ID(22-22)common/src/utils/hash/sha.ts (1)
hash(1-1)common/src/utils/index.ts (2)
hash(27-27)flexiblePoseidon(24-24)common/src/utils/hash/poseidon.ts (1)
flexiblePoseidon(1-1)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (15)
app/src/utils/testingUtils.ts (1)
29-31: Bare catch is fine — toolchain looks okay but a few tsconfig issues need attentionRan your checks. The bare-catch change preserves behavior (error not used) and there are no leftover unused catch bindings.
Confirmed OK:
- app/src/utils/testingUtils.ts — lines 29-31 and 38-40: bare catch blocks are safe to keep.
- app/package.json -> react-native 0.75.4; Babel preset present.
- Hermes appears enabled (app/android/gradle.properties contains hermesEnabled=true; Podfile references Hermes).
Action items (please verify/fix):
- Add an explicit compilerOptions.target (ES2019+) or ensure Babel/packager transpiles these packages:
- app/tests/tsconfig.json
- app/tsconfig.json
- app/tsconfig.test.json
- circuits/tsconfig.cjs.json
- circuits/tsconfig.types.json
- common/tsconfig.cjs.json
- packages/mobile-sdk-alpha/tsconfig.cjs.json
- sdk/qrcode/tsconfig.cjs.json
- sdk/qrcode/tsconfig.react.json
- Fix/unblock the unparseable file: sdk/qrcode/tsconfig.json (jq reported "unparseable tsconfig").
Snippet for context (unchanged):
} catch { console.log(`Document ${doc.id} not found or already cleared`); }app/src/utils/cloudBackup/helpers.ts (1)
41-43: LGTM on bare catch migration.Behavior is preserved (we map any JSON.parse failure to a clear, domain-specific error), and removing the unused binding reduces lint noise.
app/docs/examples/tree-shaking/level3-optimal-example.ts (3)
30-30: Relying on docs-wide ESLint overrides instead of inline disables is the right move.Inline suppression is removed, and console usage remains appropriate for examples.
35-35: Same here — good cleanup.Keeps example code readable while adhering to the new lint policy for docs.
40-40: And here — consistent with the pattern applied across docs examples.No runtime impact; lint signal is cleaner.
app/docs/examples/tree-shaking/level2-optimal-example.ts (1)
19-25: LGTM: removal of inline ESLint disables in example is aligned with repo-wide overridesConsole usage in docs/examples is fine given the new ESLint overrides; no runtime impact.
app/tests/src/providers/passportDataProvider.test.tsx (2)
93-95: LGTM: optional catch binding in test componentThis mirrors the production change while preserving behavior.
449-453: Good call re-binding to the re-imported module after mocking Keychain to undefinedRe-acquiring loadDocumentCatalog from the freshly required module ensures the test hits the mocked "undefined Keychain" branch reliably.
app/src/screens/prove/ProofRequestStatusScreen.tsx (2)
4-4: Importing useCallback is correct for the onOkPress memoizationMatches the refactor below.
94-99: LGTM: optional catch binding on deep-link validationBehavior unchanged, and the warning still includes the provided URL for diagnostics.
app/src/screens/dev/DevSettingsScreen.tsx (1)
350-359: Gating “Display your private key” behind DEV is the right callKeeps sensitive UI out of production. Ensure the DevPrivateKey route is excluded from production navigation to prevent accidental deep links; the PR notes lazy dev screens, which should cover this.
If helpful, I can scan navigation wiring to confirm the dev route is excluded from prod builds.
app/src/navigation/dev.ts (1)
20-22: Lazy-loading DevPrivateKeyScreen is correct and consistent with dev-only loading.Good move to keep dev-only UI out of the main bundle. I also see Suspense wrapping at the app root, so this will render safely during load.
app/src/screens/settings/ManageDocumentsScreen.tsx (1)
141-143: Optional catch binding is fine here.Catching without binding avoids an unused variable and is appropriate since the error isn’t used.
app/src/navigation/index.tsx (1)
105-108: Suspense wrapper for lazy screens looks good.Covers lazy-loaded screens (including dev ones) with a safe fallback.
app/.eslintrc.cjs (1)
228-234: Reasonable: override to allow require() in navigation for conditional dev loading.Matches the new dynamic devScreens loading pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sdk/qrcode/package.json (1)
20-48: Optional: add a TS <4.7 fallback via typesVersions.Consumers still on older TS won’t understand the
typesexport condition. If you want broader compatibility, add a minimaltypesVersionsmap so deep imports still get declarations.Add this at the top level of package.json (outside “exports”):
"typesVersions": { "*": { "*": ["dist/esm/index.d.ts"], "components/*": ["dist/esm/components/*.d.ts"], "utils/*": ["dist/esm/utils/*.d.ts"] } }Also consider documenting the minimum supported TypeScript version (4.7+) in the README to set expectations for consumers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/src/navigation/index.tsx(3 hunks)app/src/screens/settings/ManageDocumentsScreen.tsx(4 hunks)sdk/qrcode/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/navigation/index.tsx
- app/src/screens/settings/ManageDocumentsScreen.tsx
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-ios
🔇 Additional comments (2)
sdk/qrcode/package.json (2)
18-49: Good move: export-map “types” per subpath improves DX without runtime impact.Adding
typesconditions alongsideimport/requirefor each public subpath is a clean, modern approach (TS 4.7+ supports this). No runtime behavior is altered; consumers get accurate typings for deep imports.
20-48: Export map type/require consistency — Verified ✅Ran the provided check against sdk/qrcode/package.json. All export entries are consistent: .types correspond to .import (.js → .d.ts) and .require maps to the CJS paths (dist/esm → dist/cjs, .js → .cjs). No changes required.
- File validated: sdk/qrcode/package.json (entries: ./, ./components/LED, ./components/SelfQRcode, ./utils/utils, ./utils/styles, ./utils/websocket)
Optional suggestion: add this script as a CI check to guard against future drift.
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
Summary
Testing
yarn niceyarn lintyarn buildyarn workspace @selfxyz/contracts build(fails: Invalid account in Hardhat config)yarn types(fails: command failed in @selfxyz/mobile-app)yarn workspace @selfxyz/mobile-app testhttps://chatgpt.com/codex/tasks/task_b_689e07502374832daa70e8b74659f031
Summary by CodeRabbit
New Features
Refactor
Style
Chores