chore: tighten types across mobile surface areas #1209
Conversation
WalkthroughAdds typed navigation and logging surfaces, refactors PassportReader/NFC scanning and exports, introduces a Button onLayout prop, converts many imports to type-only, and adjusts several UI wrappers, error handlers, and utility typings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as NFC Scan Screen
participant PR as PassportReader (typed)
participant Android as RNPassportReader (Android)
participant iOS as PassportReader (iOS)
User->>Screen: Start scan(options{quality?, ...})
Screen->>PR: scan(options)
alt Platform == Android
PR->>Android: scan(options)
Android-->>PR: AndroidScanResponse
PR-->>Screen: AndroidScanResponse (normalized)
else Platform == iOS
PR->>iOS: scanPassport(options)
iOS-->>PR: string | object
PR-->>Screen: parsed/normalized result
else Unsupported
PR-->>Screen: warn and return null/reject
end
sequenceDiagram
autonumber
participant App as App code (console.*)
participant Interceptor as consoleInterceptor
participant AppLogger as LoggerExtension
participant Transport as lokiTransport
App->>Interceptor: console.log/info/warn/error(...args)
Interceptor->>AppLogger: corresponding method(...args)
AppLogger->>Transport: enqueue {level,message,timestamp,data?}
Transport-->>AppLogger: stored/flush
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/screens/home/HomeScreen.tsx (1)
56-56: Remove unnecessaryas nevercasts on navigation.navigate.The generated
RootStackParamList(viaStaticParamList<typeof AppNavigation>) already includesLaunchwith no params—casting tonever bypasses type safety. Delete the casts to restore correct typing.
🧹 Nitpick comments (11)
app/src/utils/locale.web.ts (1)
20-26: Type augmentation correctly removes@ts-expect-error.The
LocaleWithTextInfotype properly handles the missing TypeScript definitions forIntl.Locale.textInfo, which is a standard but relatively new property in the spec. The optional chaining on line 32 appropriately guards against browsers that may not support it yet.Consider moving the type definition to module level (before
getLocales) to avoid redeclaring it on every map iteration and improve reusability:+type LocaleWithTextInfo = Intl.Locale & { + textInfo?: { + direction?: string; + }; +}; + export function getLocales(): Locale[] { return navigator.languages.map(lang => { - type LocaleWithTextInfo = Intl.Locale & { - textInfo?: { - direction?: string; - }; - }; - const locale = new Intl.Locale(lang) as LocaleWithTextInfo;app/src/screens/dev/DevLoadingScreen.tsx (2)
61-76: Move constant arrays outside the component.These arrays contain hardcoded constant values that never change. Using
useMemowith empty dependencies adds unnecessary overhead and complexity. The standard pattern is to define such constants at module level, which also eliminates the need to include them as useEffect dependencies.Move these declarations above the component:
+const terminalStates: ProvingStateType[] = [ + 'completed', + 'error', + 'failure', + 'passport_not_supported', + 'account_recovery_choice', + 'passport_data_not_found', +]; + +const safeToCloseStates: ProvingStateType[] = [ + 'proving', + 'post_proving', + 'completed', +]; + const DevLoadingScreen: React.FC = () => { const [currentState, setCurrentState] = useState<ProvingStateType>('idle'); // ... - const terminalStates = useMemo<ProvingStateType[]>( - () => [ - 'completed', - 'error', - 'failure', - 'passport_not_supported', - 'account_recovery_choice', - 'passport_data_not_found', - ], - [], - ); - - const safeToCloseStates = useMemo<ProvingStateType[]>( - () => ['proving', 'post_proving', 'completed'], - [], - );
104-104: Remove constant arrays from dependencies.Once
terminalStatesandsafeToCloseStatesare defined as module-level constants, they should be removed from the dependency array since their identity never changes.- }, [currentState, documentType, safeToCloseStates, terminalStates]); + }, [currentState, documentType]);app/src/stores/settingStore.ts (1)
92-97: Prefer destructuring over shallow copy + delete for partialize.The current approach of spreading state, casting to
Partial<SettingsState>, and deleting properties is less idiomatic and requires a type cast to work around TypeScript constraints.Consider using destructuring for clearer intent and better type safety:
partialize: state => { - const persistedState = { ...state }; - delete (persistedState as Partial<SettingsState>).hideNetworkModal; - delete (persistedState as Partial<SettingsState>).setHideNetworkModal; - return persistedState; + const { hideNetworkModal, setHideNetworkModal, ...persistedState } = state; + return persistedState; },This eliminates the type cast, avoids mutation, and more clearly expresses the exclusion pattern. Note that deleting
setHideNetworkModal(a function) is technically redundant sinceJSON.stringifydrops functions automatically, but the explicit exclusion improves readability.app/src/screens/document/ConfirmBelongingScreen.tsx (1)
82-89: Consider logging initialization errors for debugging.The bare catch silently swallows errors during document metadata initialization, while the
onOkPresshandler at line 112 properly logs errors. For consistency and easier debugging, consider at minimum logging a message when initialization fails.If security concerns prevent logging the error object (PII risk), a simple message would help:
- } catch { + } catch { + console.log('Failed to load document metadata, using defaults'); // setting defaults on error setDocumentMetadata({app/src/screens/system/Loading.tsx (1)
101-103: Reconsider error logging approach.The
console.errorcall without the actual error object limits debugging capability when document loading fails. Consider either:
- If this is an unexpected error that needs investigation, capture and log the error object (verify it doesn't contain PII)
- If this is an expected/handled failure case, use
console.loginstead ofconsole.errorto match the pattern inkeychainSecurity.tsOption 1 - Log the error if safe:
- } catch { - console.error('Error loading selected document:'); + } catch (error) { + console.error('Error loading selected document:', error);Option 2 - Downgrade to log if this is expected:
- console.error('Error loading selected document:'); + console.log('Could not load selected document, using defaults');app/src/screens/dev/DevFeatureFlagsScreen.tsx (1)
47-47: Timer cleanup fix works correctly.The ref + sync pattern ensures all active timers are cleared on unmount, fixing the stale closure issue.
Optional: Consider using ref-only pattern.
You could eliminate the
debounceTimersstate entirely and manage timers solely viadebounceTimersRef. This would:
- Remove the need for the sync effect
- Prevent re-renders when timers change
- Simplify the code
In
debouncedSave, directly access and modifydebounceTimersRef.currentinstead of using state, and removedebounceTimersfrom the dependency array (line 171).Since this is a dev screen, the performance impact of the current approach is minimal, so this refactor is purely optional.
Also applies to: 190-192
app/src/utils/logger.ts (1)
16-34: Double cast suggests type misalignment.Line 22 uses
as unknown as transportFunctionType<object>, which typically indicates a type incompatibility. ThelokiTransportis defined withtransportFunctionType<LokiTransportOptions>whereLokiTransportOptions = Record<string, never>.Consider investigating whether:
- The cast is necessary due to library typing limitations, or
LokiTransportOptionsshould beobjectorRecord<string, unknown>for better alignment with the config's expected union typeIf the library typing is the issue and the cast is unavoidable, add a comment explaining why it's necessary.
app/src/utils/logger/lokiTransport.ts (1)
168-171: Consider more flexible type for future extensibility.
LokiTransportOptions = Record<string, never>enforces an empty object, which is technically correct for current usage but prevents future option additions. Consider usingRecord<string, unknown>orobjectfor better extensibility without sacrificing type safety.Current implementation is correct and the choice may be intentional to signal no options are supported.
app/src/utils/nfcScanner.ts (1)
97-143: Consider simplifying the iOS type narrowing.The iOS-specific type augmentation (lines 97-113) is functionally correct but verbose. You could define a reusable type alias in passportReader.ts or use a simpler guard:
const scanPassport = (PassportReader as IOSPassportReaderModule | null)?.scanPassport; if (!scanPassport) { // error handling } return await Promise.resolve(scanPassport(...));This would avoid the inline type assertion while maintaining type safety. However, the current implementation is safe and works correctly.
app/src/utils/passportReader.ts (1)
147-151: Consider logging JSON parse failures.The try-catch at lines 147-150 silently returns the unparsed result when JSON parsing fails. While this provides a fallback, logging the error would help diagnose issues in production:
try { return typeof result === 'string' ? JSON.parse(result) : result; } catch (error) { if (__DEV__) console.warn('Failed to parse iOS scan result as JSON:', error); return result; }This is a minor observability improvement for debugging iOS scan result formats.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
app/src/components/buttons/AbstractButton.tsx(2 hunks)app/src/components/buttons/PrimaryButtonLongHold.tsx(0 hunks)app/src/components/buttons/PrimaryButtonLongHold.web.tsx(0 hunks)app/src/components/homeScreen/SvgXmlWrapper.native.tsx(1 hunks)app/src/components/homeScreen/SvgXmlWrapper.web.tsx(2 hunks)app/src/components/native/PassportCamera.tsx(1 hunks)app/src/components/native/QRCodeScanner.tsx(1 hunks)app/src/hooks/useAppUpdates.ts(1 hunks)app/src/hooks/useAppUpdates.web.ts(1 hunks)app/src/hooks/useModal.ts(2 hunks)app/src/navigation/document.ts(1 hunks)app/src/navigation/home.ts(1 hunks)app/src/navigation/index.tsx(2 hunks)app/src/navigation/system.tsx(2 hunks)app/src/providers/selfClientProvider.tsx(5 hunks)app/src/screens/dev/CreateMockScreen.tsx(3 hunks)app/src/screens/dev/CreateMockScreenDeepLink.tsx(2 hunks)app/src/screens/dev/DevFeatureFlagsScreen.tsx(3 hunks)app/src/screens/dev/DevLoadingScreen.tsx(1 hunks)app/src/screens/document/ConfirmBelongingScreen.tsx(1 hunks)app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx(4 hunks)app/src/screens/document/DocumentNFCScanScreen.tsx(3 hunks)app/src/screens/document/aadhaar/AadhaarUploadScreen.tsx(4 hunks)app/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsx(2 hunks)app/src/screens/home/DisclaimerScreen.tsx(2 hunks)app/src/screens/home/HomeScreen.tsx(3 hunks)app/src/screens/home/ProofHistoryList.tsx(2 hunks)app/src/screens/home/ProofHistoryScreen.tsx(2 hunks)app/src/screens/prove/ProveScreen.tsx(3 hunks)app/src/screens/prove/QRCodeViewFinderScreen.tsx(2 hunks)app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx(3 hunks)app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx(2 hunks)app/src/screens/recovery/RecoverWithPhraseScreen.tsx(3 hunks)app/src/screens/settings/CloudBackupScreen.tsx(2 hunks)app/src/screens/system/Loading.tsx(1 hunks)app/src/stores/settingStore.ts(1 hunks)app/src/types/elliptic.d.ts(1 hunks)app/src/utils/analytics.ts(2 hunks)app/src/utils/keychainSecurity.ts(3 hunks)app/src/utils/locale.web.ts(1 hunks)app/src/utils/logger.ts(4 hunks)app/src/utils/logger/consoleInterceptor.ts(1 hunks)app/src/utils/logger/lokiTransport.ts(4 hunks)app/src/utils/logger/nativeLoggerBridge.ts(2 hunks)app/src/utils/nfcScanner.ts(4 hunks)app/src/utils/notifications/notificationService.shared.ts(1 hunks)app/src/utils/notifications/notificationService.ts(2 hunks)app/src/utils/passportReader.ts(1 hunks)app/src/utils/proving/validateDocument.ts(2 hunks)app/tests/utils/nfcScanner.test.ts(1 hunks)app/tsconfig.json(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)
💤 Files with no reviewable changes (2)
- app/src/components/buttons/PrimaryButtonLongHold.tsx
- app/src/components/buttons/PrimaryButtonLongHold.web.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/tests/utils/nfcScanner.test.tsapp/src/screens/dev/DevFeatureFlagsScreen.tsxapp/src/navigation/document.tsapp/src/utils/locale.web.tsapp/src/hooks/useAppUpdates.tsapp/src/screens/recovery/RecoverWithPhraseScreen.tsxapp/src/providers/selfClientProvider.tsxapp/src/components/native/QRCodeScanner.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tsapp/src/hooks/useModal.tsapp/src/utils/analytics.tsapp/src/screens/dev/CreateMockScreen.tsxapp/src/screens/recovery/AccountRecoveryChoiceScreen.tsxapp/src/utils/logger/nativeLoggerBridge.tsapp/src/utils/proving/validateDocument.tsapp/src/screens/home/DisclaimerScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/utils/keychainSecurity.tsapp/src/screens/home/HomeScreen.tsxapp/src/screens/home/ProofHistoryScreen.tsxapp/src/screens/settings/CloudBackupScreen.tsxapp/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/utils/notifications/notificationService.shared.tsapp/src/screens/prove/ProveScreen.tsxapp/src/screens/document/ConfirmBelongingScreen.tsxapp/src/navigation/index.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/stores/settingStore.tsapp/src/utils/logger/consoleInterceptor.tsapp/src/utils/logger/lokiTransport.tsapp/src/screens/document/DocumentNFCScanScreen.tsxapp/src/navigation/system.tsxapp/src/screens/system/Loading.tsxapp/src/utils/notifications/notificationService.tsapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/document/aadhaar/AadhaarUploadScreen.tsxapp/src/types/elliptic.d.tsapp/src/components/native/PassportCamera.tsxapp/src/utils/nfcScanner.tsapp/src/screens/prove/QRCodeViewFinderScreen.tsxapp/src/components/homeScreen/SvgXmlWrapper.web.tsxapp/src/components/buttons/AbstractButton.tsxapp/src/screens/document/DocumentNFCMethodSelectionScreen.tsxapp/src/utils/passportReader.tsapp/src/utils/logger.tsapp/src/components/homeScreen/SvgXmlWrapper.native.tsxapp/src/screens/home/ProofHistoryList.tsxapp/src/navigation/home.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/tests/utils/nfcScanner.test.tsapp/src/screens/dev/DevFeatureFlagsScreen.tsxapp/src/navigation/document.tsapp/src/utils/locale.web.tsapp/src/hooks/useAppUpdates.tsapp/src/screens/recovery/RecoverWithPhraseScreen.tsxapp/src/providers/selfClientProvider.tsxapp/src/components/native/QRCodeScanner.tsxapp/src/hooks/useModal.tsapp/src/utils/analytics.tsapp/src/screens/dev/CreateMockScreen.tsxapp/src/screens/recovery/AccountRecoveryChoiceScreen.tsxapp/src/utils/logger/nativeLoggerBridge.tsapp/src/utils/proving/validateDocument.tsapp/src/screens/home/DisclaimerScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/utils/keychainSecurity.tsapp/src/screens/home/HomeScreen.tsxapp/src/screens/home/ProofHistoryScreen.tsxapp/src/screens/settings/CloudBackupScreen.tsxapp/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/utils/notifications/notificationService.shared.tsapp/src/screens/prove/ProveScreen.tsxapp/src/screens/document/ConfirmBelongingScreen.tsxapp/src/navigation/index.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/stores/settingStore.tsapp/src/utils/logger/consoleInterceptor.tsapp/src/utils/logger/lokiTransport.tsapp/src/screens/document/DocumentNFCScanScreen.tsxapp/src/navigation/system.tsxapp/src/screens/system/Loading.tsxapp/src/utils/notifications/notificationService.tsapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/document/aadhaar/AadhaarUploadScreen.tsxapp/src/types/elliptic.d.tsapp/src/components/native/PassportCamera.tsxapp/src/utils/nfcScanner.tsapp/src/screens/prove/QRCodeViewFinderScreen.tsxapp/src/components/homeScreen/SvgXmlWrapper.web.tsxapp/src/components/buttons/AbstractButton.tsxapp/src/screens/document/DocumentNFCMethodSelectionScreen.tsxapp/src/utils/passportReader.tsapp/src/utils/logger.tsapp/src/components/homeScreen/SvgXmlWrapper.native.tsxapp/src/screens/home/ProofHistoryList.tsxapp/src/navigation/home.ts
**/*.{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/utils/nfcScanner.test.ts
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/dev/DevFeatureFlagsScreen.tsxapp/src/navigation/document.tsapp/src/utils/locale.web.tsapp/src/hooks/useAppUpdates.tsapp/src/screens/recovery/RecoverWithPhraseScreen.tsxapp/src/providers/selfClientProvider.tsxapp/src/components/native/QRCodeScanner.tsxapp/src/hooks/useModal.tsapp/src/utils/analytics.tsapp/src/screens/dev/CreateMockScreen.tsxapp/src/screens/recovery/AccountRecoveryChoiceScreen.tsxapp/src/utils/logger/nativeLoggerBridge.tsapp/src/utils/proving/validateDocument.tsapp/src/screens/home/DisclaimerScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/utils/keychainSecurity.tsapp/src/screens/home/HomeScreen.tsxapp/src/screens/home/ProofHistoryScreen.tsxapp/src/screens/settings/CloudBackupScreen.tsxapp/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/utils/notifications/notificationService.shared.tsapp/src/screens/prove/ProveScreen.tsxapp/src/screens/document/ConfirmBelongingScreen.tsxapp/src/navigation/index.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/stores/settingStore.tsapp/src/utils/logger/consoleInterceptor.tsapp/src/utils/logger/lokiTransport.tsapp/src/screens/document/DocumentNFCScanScreen.tsxapp/src/navigation/system.tsxapp/src/screens/system/Loading.tsxapp/src/utils/notifications/notificationService.tsapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/document/aadhaar/AadhaarUploadScreen.tsxapp/src/types/elliptic.d.tsapp/src/components/native/PassportCamera.tsxapp/src/utils/nfcScanner.tsapp/src/screens/prove/QRCodeViewFinderScreen.tsxapp/src/components/homeScreen/SvgXmlWrapper.web.tsxapp/src/components/buttons/AbstractButton.tsxapp/src/screens/document/DocumentNFCMethodSelectionScreen.tsxapp/src/utils/passportReader.tsapp/src/utils/logger.tsapp/src/components/homeScreen/SvgXmlWrapper.native.tsxapp/src/screens/home/ProofHistoryList.tsxapp/src/navigation/home.ts
app/**/*.{ios,android,web}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Explain platform-specific code paths in the PR description (files with .ios, .android, or .web extensions)
Files:
app/src/utils/locale.web.tsapp/src/hooks/useAppUpdates.web.tsapp/src/components/homeScreen/SvgXmlWrapper.web.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/**/*.{ts,tsx} : Type checking must pass before PRs (yarn types)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
app/tests/utils/nfcScanner.test.tsapp/src/utils/nfcScanner.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Applied to files:
app/tests/utils/nfcScanner.test.tsapp/tsconfig.json
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Follow ESLint TypeScript-specific rules
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
app/src/screens/recovery/RecoverWithPhraseScreen.tsxapp/src/providers/selfClientProvider.tsxapp/src/screens/recovery/AccountRecoveryChoiceScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/home/HomeScreen.tsxapp/src/screens/settings/CloudBackupScreen.tsxapp/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/screens/prove/ProveScreen.tsxapp/src/navigation/index.tsxapp/src/screens/document/DocumentNFCScanScreen.tsxapp/src/screens/prove/QRCodeViewFinderScreen.tsxapp/src/screens/document/DocumentNFCMethodSelectionScreen.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/**/*.{js,ts,tsx,jsx} : Use `react-navigation/native` with `createStaticNavigation` for type-safe navigation in React Native.
Applied to files:
app/src/screens/dev/CreateMockScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/screens/home/HomeScreen.tsxapp/src/screens/home/ProofHistoryScreen.tsxapp/src/screens/settings/CloudBackupScreen.tsxapp/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/screens/prove/ProveScreen.tsxapp/src/navigation/index.tsxapp/src/screens/document/DocumentNFCScanScreen.tsxapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/prove/QRCodeViewFinderScreen.tsxapp/src/screens/document/DocumentNFCMethodSelectionScreen.tsxapp/src/screens/home/ProofHistoryList.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
app/src/utils/proving/validateDocument.ts
📚 Learning: 2025-09-10T14:47:40.945Z
Learnt from: shazarre
PR: selfxyz/self#1041
File: app/src/providers/passportDataProvider.tsx:297-301
Timestamp: 2025-09-10T14:47:40.945Z
Learning: In app/src/providers/passportDataProvider.tsx: The deleteDocumentDirectlyFromKeychain function is a low-level utility used by the DocumentsAdapter and should not include error handling since callers like deleteDocument() already implement appropriate try/catch with logging for Keychain operations.
Applied to files:
app/src/utils/keychainSecurity.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.
Applied to files:
app/src/screens/home/HomeScreen.tsx
🧬 Code graph analysis (27)
app/src/hooks/useAppUpdates.ts (2)
app/src/hooks/useAppUpdates.web.ts (1)
useAppUpdates(15-45)app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/recovery/RecoverWithPhraseScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/providers/selfClientProvider.tsx (1)
app/src/navigation/index.tsx (2)
RootStackParamList(49-49)navigationRef(54-54)
app/src/hooks/useModal.ts (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/utils/analytics.ts (1)
app/src/mocks/react-native-passport-reader.ts (1)
PassportReader(8-28)
app/src/screens/dev/CreateMockScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/utils/proving/validateDocument.ts (1)
common/src/utils/types.ts (1)
PassportData(68-79)
app/src/screens/home/DisclaimerScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/hooks/useAppUpdates.web.ts (2)
app/src/hooks/useAppUpdates.ts (1)
useAppUpdates(17-57)app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/home/HomeScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/home/ProofHistoryScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/settings/CloudBackupScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/prove/ProveScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/dev/DevLoadingScreen.tsx (1)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
ProvingStateType(341-364)
app/src/screens/document/DocumentNFCScanScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/navigation/system.tsx (1)
app/src/screens/system/ModalScreen.tsx (1)
ModalNavigationParams(35-38)
app/src/utils/notifications/notificationService.ts (2)
app/src/utils/notifications/notificationService.shared.ts (1)
RemoteMessage(11-19)app/tests/__setup__/notificationServiceMock.js (2)
RemoteMessage(5-5)RemoteMessage(5-5)
app/src/screens/dev/CreateMockScreenDeepLink.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/utils/nfcScanner.ts (1)
app/src/mocks/react-native-passport-reader.ts (1)
PassportReader(8-28)
app/src/screens/prove/QRCodeViewFinderScreen.tsx (2)
app/src/hooks/useConnectionModal.ts (1)
useConnectionModal(35-67)app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
app/src/utils/passportReader.ts (4)
app/src/utils/nfcScanner.ts (1)
scan(39-64)app/android/react-native-passport-reader/android/src/main/java/io/tradle/nfc/RNPassportReaderModule.kt (2)
scan(204-246)reset(940-946)app/android/react-native-passport-reader/index.android.js (1)
NativeModules(5-5)app/ios/PassportReader.swift (2)
scanPassport(144-360)scanPassport(500-514)
app/src/components/homeScreen/SvgXmlWrapper.native.tsx (2)
app/src/components/homeScreen/SvgXmlWrapper.web.tsx (1)
SvgXml(20-39)app/src/mocks/react-native-svg.ts (1)
SvgXml(42-62)
app/src/screens/home/ProofHistoryList.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(49-49)
🪛 Biome (2.1.2)
app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx
[error] 23-23: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ 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). (4)
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
🔇 Additional comments (50)
app/tsconfig.json (1)
17-24: LGTM! Path alias additions follow established pattern.The source path additions for onboarding and disclosing flows maintain consistency with the existing SDK alias structure (src before dist). This enables TypeScript to resolve to source files during development while preserving the fallback to compiled dist files.
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (2)
14-15: LGTM! Type-only export separation improves TypeScript hygiene.Splitting the combined export into separate type and value exports is a best practice that eliminates runtime overhead and clarifies intent. This change aligns well with the PR's objective to tighten types across the workspace.
32-94: LGTM! Proper error handling and PII-safe analytics.The
useReadMRZhook correctly:
- Handles error cases with appropriate analytics tracking
- Validates scanned data before proceeding
- Logs only metadata (lengths, durations) without exposing PII
- Emits SDK events for success/failure flows
As per coding guidelines, the implementation properly avoids logging sensitive data (passport numbers, DOB, etc.) and only tracks safe metadata.
Note: The TODO on line 45 is already flagged for future error handling improvements.
app/src/utils/keychainSecurity.ts (3)
48-51: LGTM!The bare catch is appropriate here since the error object isn't needed and a clear fallback message is logged.
67-70: LGTM!The bare catch is appropriate here since the error object isn't needed and a clear fallback message is logged.
190-193: LGTM!The bare catch is appropriate here since the error object isn't needed and a clear fallback message is logged.
app/src/components/buttons/AbstractButton.tsx (2)
6-10: LGTM! Type-safe import added.The
LayoutChangeEventtype import is properly added using TypeScript's type-only import syntax, which aligns with the PR's goal of tightening types across the workspace.
23-23: Approve onLayout Prop AdditionResolves previous
@ts-expect-errorsuppressions by typingonLayoutexplicitly and forwards the callback via spread.
Verify that tamagui’sViewPropsdoesn’t already includeonLayoutto avoid redundant definitions.app/src/screens/dev/DevFeatureFlagsScreen.tsx (1)
197-201: LGTM!The cleanup now correctly accesses current timers via the ref, and the defensive null check is good practice.
app/src/utils/logger/consoleInterceptor.ts (1)
14-46: LGTM! Clean type safety improvement.The introduction of
LoggerMethodsand the consistent use ofunknown[]for variadic arguments correctly eliminatesanywhile maintaining flexibility. The implementation properly delegates to both the app logger and original console methods.app/src/utils/logger/nativeLoggerBridge.ts (4)
7-7: LGTM! Proper type imports.Type-only imports prevent runtime overhead and clearly signal the intent.
12-17: LGTM! Appropriate use of unknown for log data.Changing
datafromanytounknowncorrectly enforces type checking at usage sites while allowing arbitrary log payloads.
21-31: LGTM! Strongly typed logger injection.The explicit types for
LoggerExtensionandRootLoggereliminateanywhile maintaining clear contracts for the injected loggers.
67-106: LGTM! Type-safe logger routing.The logger variable is correctly typed as
LoggerExtension, and all assignments in the switch statement (including theLogger.extend()call on line 86) properly return compatible types.app/src/utils/logger.ts (2)
38-39: LGTM! Well-defined type exports.The
RootLoggerandLoggerExtensiontypes provide clear contracts for consumers and eliminate the need foranyin dependent modules.
59-85: LGTM! Enhanced public API.Exporting
logLevelsalongside the logger types strengthens the public API and enables type-safe usage across the codebase.app/src/utils/logger/lokiTransport.ts (2)
194-207: LGTM! Properly typed log object.The explicit structure with
unknownfor optional data correctly eliminatesanywhile maintaining flexibility for arbitrary log payloads.
220-225: LGTM! Clean export organization.The consolidated export block with type exports improves discoverability and maintains a clear public API surface.
app/src/components/native/PassportCamera.tsx (1)
72-80: LGTM! Clearer error object construction.The refactoring to rename
errortonativeErrormakes the code more explicit about which error string is being assigned toError.name. This is consistent with the parallel change in QRCodeScanner.tsx.app/src/components/native/QRCodeScanner.tsx (1)
56-64: LGTM! Consistent error handling pattern.This mirrors the error handling improvement in PassportCamera.tsx. The explicit
nativeErrornaming clarifies which value is being assigned toError.name.app/tests/utils/nfcScanner.test.ts (1)
7-14: LGTM! Clarified mock usage.The comment appropriately explains that
configureNfcAnalyticsis mocked and doesn't need to be imported directly. The mock setup at lines 12-14 ensures the function is available during tests.app/src/utils/analytics.ts (2)
194-201: LGTM! Defensive guards prevent crashes on module unavailability.The added null checks for
PassportReaderand its methods are appropriate. The PassportReader module can be null on web or when native modules fail to load (as reflected in the type changes in passportReader.ts). These guards prevent runtime crashes while gracefully degrading functionality.
281-283: LGTM! Consistent guarding for NFC event tracking.The guard pattern here matches the one at line 194, ensuring
trackEventis only called when available.app/src/utils/nfcScanner.ts (2)
13-13: LGTM! Type reuse from passportReader.ts.Importing
AndroidScanResponsefrom the centralized module follows DRY principles and ensures type consistency.
228-228: LGTM! Explicit radix prevents parseInt surprises.Specifying radix
10explicitly is a best practice that avoids potential octal interpretation of strings with leading zeros.app/src/utils/passportReader.ts (2)
21-66: LGTM! Strong typing for platform-specific modules.The new
AndroidScanResponse,AndroidPassportReaderModule,IOSPassportReaderModule, andPassportReaderModuleunion types significantly improve type safety. The nullablePassportReader(line 69) correctly reflects that native modules may not be available, which aligns with the defensive guards added in analytics.ts.
73-157: LGTM! Platform-specific module loading with proper type guards.The restructured loading logic correctly handles Android (via
RNPassportReader) and iOS (viaPassportReader) native modules with appropriate type assertions and fallback warnings. The scan wrappers appropriately normalize platform differences.app/src/utils/notifications/notificationService.shared.ts (1)
18-18: LGTM! Type safety improvement.Changing the index signature from
anytounknownis a solid defensive move that forces consumers to perform type narrowing before accessing dynamic properties. This aligns well with the PR's goal of tightening types across the workspace.app/src/utils/notifications/notificationService.ts (1)
6-10: LGTM! Clean import structure.The addition of
FirebaseMessagingTypesand the use ofimport typeforDeviceTokenRegistrationare appropriate for the type-safety improvements in this file.app/src/components/homeScreen/SvgXmlWrapper.web.tsx (1)
5-5: LGTM! DOMPurify usage is correct.The change from default import to
createDOMPurifyfactory function is the correct pattern. The sanitization configuration with SVG profiles is appropriate for safely rendering SVG content.Also applies to: 23-26
app/src/components/homeScreen/SvgXmlWrapper.native.tsx (2)
9-9: Good: Props type derived from underlying component.Using
ComponentProps<typeof RNSvgXml>is more maintainable than manually defining props. Any changes to the underlying component's props are automatically reflected.
11-11: No ref usage detected—forwardRef removal is safe. Search under app/src for<SvgXml … ref=…>,useRef<…SvgXml>, andcreateRef<…SvgXml>returned no hits; you can dropforwardRefon native without breaking refs.app/src/hooks/useAppUpdates.web.ts (1)
7-7: LGTM! Type-safe navigation implemented correctly.The typed navigation prop provides compile-time safety for navigation calls and aligns with the PR's objective to eliminate
anytypes across the workspace.Also applies to: 12-12, 16-17
app/src/screens/home/DisclaimerScreen.tsx (1)
10-10: LGTM! Consistent type-safe navigation.Type refinement ensures navigation calls are validated at compile time without altering runtime behavior.
Also applies to: 19-19, 25-26
app/src/screens/dev/CreateMockScreenDeepLink.tsx (1)
12-12: LGTM! Type-safe navigation applied.Navigation typing is consistent with the project-wide pattern for type safety.
Also applies to: 24-24, 31-32
app/src/hooks/useModal.ts (1)
7-7: LGTM! Type-safe navigation in modal hook.Strongly-typed navigation provides compile-time safety for modal navigation flows.
Also applies to: 9-9, 19-20
app/src/screens/home/ProofHistoryList.tsx (1)
14-14: LGTM! Type-safe navigation implemented.Navigation typing ensures route parameters are validated at compile time.
Also applies to: 18-18, 67-68
app/src/screens/home/ProofHistoryScreen.tsx (1)
15-15: LGTM! Consistent typed navigation.Type refinement provides compile-time validation for navigation calls.
Also applies to: 19-19, 62-63
app/src/screens/dev/CreateMockScreen.tsx (1)
23-23: LGTM! Type-safe navigation applied.Navigation typing aligns with the project-wide pattern for type safety.
Also applies to: 41-41, 164-165
app/src/screens/document/aadhaar/AadhaarUploadedSuccessScreen.tsx (1)
8-8: LGTM! Type-safe navigation implemented.Navigation typing is consistent with the project-wide refactor. Ensure
yarn typespasses to validate the type safety improvements.Based on learnings: Type checking must pass before PRs (yarn types).
Also applies to: 17-17, 23-24
app/src/screens/recovery/RecoverWithPhraseScreen.tsx (1)
11-11: LGTM! Type-safe navigation successfully implemented.The addition of typed navigation props strengthens type safety without altering runtime behavior.
Also applies to: 20-20, 36-37
app/src/screens/recovery/AccountRecoveryChoiceScreen.tsx (1)
8-8: LGTM! Type-safe navigation successfully implemented.The typed navigation hook provides better compile-time safety for route parameters.
Also applies to: 23-23, 42-43
app/src/hooks/useAppUpdates.ts (1)
9-9: LGTM! Type-safe navigation successfully implemented.The hook now benefits from compile-time type checking for navigation parameters.
Also applies to: 14-14, 18-19
app/src/screens/document/DocumentNFCScanScreen.tsx (1)
31-31: LGTM! Type-safe navigation successfully implemented.The typed navigation provides better type safety for NFC scan flow navigation.
Also applies to: 52-52, 97-98
app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx (1)
9-9: LGTM! Type-safe navigation successfully implemented.The typed navigation hook enhances type safety for the recovery flow.
Also applies to: 18-18, 24-25
app/src/navigation/document.ts (1)
66-69: LGTM! Explicit route params improve type safety.The
paramstype definition provides compile-time validation for route parameters.app/src/screens/document/aadhaar/AadhaarUploadScreen.tsx (1)
6-6: LGTM! Proper image typing eliminatesas anycast.The use of
ImageSourcePropTypefor the Aadhaar image is a good improvement over the previousas anyapproach.Also applies to: 35-35, 156-156
app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (3)
9-9: LGTM! Typed navigation import added.Adding
NativeStackNavigationPropenables proper type inference for the navigation hook.
20-20: LGTM! RootStackParamList import added.This import provides the centralized navigation param types for type-safe navigation.
95-96: LGTM! Navigation hook properly typed.The typed
useNavigationhook correctly usesNativeStackNavigationProp<RootStackParamList>.
Resolved merge conflicts: - Updated imports to use new @/screens/app/* paths - Removed screens moved to documents directory - Accepted deletion of document.ts (replaced by documents.ts) - Maintained type safety with RootStackParamList imports
- Changed from React.forwardRef to forwardRef import - Resolves linting warning about named exports
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
app/src/screens/app/LoadingScreen.tsx (1)
101-106: Bare catch removes critical error diagnostics.The bare
catchon line 101 discards the error object, and line 102'sconsole.errorlogs a message without including the actual error details. This makes debugging initialization failures extremely difficult in production.Apply this diff to preserve error information:
- } catch { - console.error('Error loading selected document:'); + } catch (error: unknown) { + console.error('Error loading selected document:', error); await init(selfClient, 'dsc', true); } finally {If your analytics or logging infrastructure is available, consider also sending the error there for better observability.
app/src/screens/home/HomeScreen.tsx (1)
56-56: Removeas nevercasts to maintain type safety.The
as nevercasts on lines 56 and 60 circumvent TypeScript's type checking, which contradicts this PR's type-tightening objectives. While these are in error-handling paths, they still defeat the purpose of typed navigation.Ensure
RootStackParamListproperly defines theLaunchroute, then remove the casts:- navigation.navigate('Launch' as never); + navigation.navigate('Launch');Apply the same fix at line 60.
Also applies to: 60-60
app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx (2)
279-322: Critical: Duplicate timeout setup with immediate cancellation.Lines 279-294 set up a 30-second timeout, but then lines 299-301 immediately clear it and set up an identical timeout again. The first timeout never has a chance to fire, wasting setup overhead. This appears to be duplicated code that should have been removed.
Apply this diff to remove the duplicate timeout:
scanCancelledRef.current = false; const scanStartTime = Date.now(); - if (scanTimeoutRef.current) { - clearTimeout(scanTimeoutRef.current); - scanTimeoutRef.current = null; - } - scanTimeoutRef.current = setTimeout(() => { - scanCancelledRef.current = true; - trackEvent(PassportEvents.NFC_SCAN_FAILED, { - error: 'timeout', - }); - logNFCEvent('warn', 'scan_timeout', { - ...baseContext, - stage: 'timeout', - }); - openErrorModal('Scan timed out. Please try again.'); - setIsNfcSheetOpen(false); - logNFCEvent('info', 'sheet_close', { - ...baseContext, - stage: 'ui', - }); - }, 30000); - - // Mark NFC scanning as active to prevent analytics flush interference - setNfcScanningActive(true); - if (scanTimeoutRef.current) { clearTimeout(scanTimeoutRef.current); scanTimeoutRef.current = null; } + // Mark NFC scanning as active to prevent analytics flush interference + setNfcScanningActive(true); + scanTimeoutRef.current = setTimeout(() => {
369-383: Critical: Early return leaves NFC sheet open and timeout active.When
parseScanResponsethrows an exception (line 370), the catch at line 371 logs the error and returns at line 382 without executing thefinallyblock. This leaves:
- The NFC sheet open (
isNfcSheetOpenstill true)- The timeout still running (
scanTimeoutRef.currentnot cleared)- NFC scanning state active (
setNfcScanningActive(false)not called)This causes the UI to freeze in the scanning state until the timeout fires 30 seconds later.
Apply this diff to handle the error properly:
let passportData: PassportData | null = null; try { passportData = parseScanResponse(scanResponse); } catch (e: unknown) { console.error('Parsing NFC Response Unsuccessful'); const errMsg = sanitizeErrorMessage( e instanceof Error ? e.message : String(e), ); trackEvent(PassportEvents.NFC_RESPONSE_PARSE_FAILED, { error: errMsg, }); trackNfcEvent(PassportEvents.NFC_RESPONSE_PARSE_FAILED, { error: errMsg, }); + openErrorModal('Failed to parse passport data. Please try again.'); - return; + throw e; // Re-throw to trigger outer catch and finally blocks }app/src/providers/selfClientProvider.tsx (1)
244-246: Use navigateIfReady for all navigation calls
Replace the direct navigationRef.navigate invocations (with@ts-expect-errorandas nevercasts) in the DOCUMENT_COUNTRY_SELECTED and DOCUMENT_TYPE_SELECTED listeners ofselfClientProvider.tsx(around lines 244–264) withnavigateIfReady('RouteName', params)to remove type suppressions.
♻️ Duplicate comments (6)
app/src/utils/notifications/notificationService.shared.ts (1)
18-18: Good type safety improvement, but interface may be unused.Changing the index signature from
anytounknownstrengthens type safety and aligns with the PR's objective to eliminateanytypes.However, a past review comment indicates the entire
RemoteMessageinterface is unused since the codebase now referencesFirebaseMessagingTypes.RemoteMessagedirectly (see lines 123, 129 in notificationService.ts). Consider removing the entire interface to eliminate redundancy.app/src/utils/proving/validateDocument.ts (1)
186-208: Critical: documentCategory fallback still uses pre-normalized documentType.When legacy records lack both
documentTypeanddocumentCategory, the logic on Line 190-192 computesinferredCategoryusingexistingDocumentType(which isundefined), defaulting to'id_card'. Line 196-198 then uses thisinferredCategoryas the primary value, so the fallback based onnormalizedDocumentTypeis never reached. The result is a mismatch:documentType: 'passport',documentCategory: 'id_card', breakingfetchAllTreesAndCircuitsandgetCommitmentTreecalls for those documents.Apply the fix from the previous review to derive
normalizedCategoryfromnormalizedDocumentType:export function migratePassportData(passportData: PassportData): PassportData { const migratedData: MigratedPassportData = { ...passportData }; const existingDocumentType = migratedData.documentType; const inferredMock = migratedData.mock ?? existingDocumentType?.startsWith('mock'); - const inferredCategory = - migratedData.documentCategory ?? - (existingDocumentType?.includes('passport') ? 'passport' : 'id_card'); const normalizedDocumentType = existingDocumentType ?? 'passport'; const normalizedMock = inferredMock ?? false; const normalizedCategory = - inferredCategory ?? + migratedData.documentCategory ?? (normalizedDocumentType.includes('passport') ? 'passport' : 'id_card'); const normalizedData: PassportData = { ...migratedData, documentType: normalizedDocumentType, mock: normalizedMock, documentCategory: normalizedCategory, }; return normalizedData; }app/src/components/homeScreen/SvgXmlWrapper.web.tsx (1)
5-5: Past security concern still applies.The change to use the named import
createDOMPurifyis correct for the modern DOMPurify API. However, the previous review comment about ensuring DOMPurify ≥3.2.4 for critical CVE fixes (CVE-2024-45801, CVE-2024-47875, CVE-2025-26791) remains unresolved.app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx (1)
23-23: Remove unnecessary empty object pattern.The empty object pattern
{}serves no purpose and was flagged by static analysis. This should be addressed to maintain clean code standards.Apply this diff:
-const AccountVerifiedSuccessScreen: React.FC = ({}) => { +const AccountVerifiedSuccessScreen: React.FC = () => {app/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsx (1)
136-137: Critical: Remove theas nevertype assertion.The
as nevercast completely defeats TypeScript's type checking for this navigation call, directly contradicting the PR's goal of tightening types. This creates a runtime risk if params don't match the route's expected type.Define the
DocumentNFCScanroute properly inRootStackParamListwith its params type, then remove the cast:- // Type assertion needed because static navigation doesn't infer optional params - navigation.navigate('DocumentNFCScan', params as never); + navigation.navigate('DocumentNFCScan', params);If params are truly optional, define in
RootStackParamList:DocumentNFCScan: NFCParams | undefined;app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx (1)
121-123: Critical: Removeas nevercasts to maintain type safety.The
as nevercasts circumvent TypeScript's type checking, which directly contradicts this PR's type-tightening objectives. These casts create runtime risks if params don't match the expected route type.Define the
AadhaarUploadErrorroute params properly inRootStackParamList:// In app/src/navigation/index.tsx AadhaarUploadError: { errorType?: 'general' | 'expired' };Then remove the casts:
- navigation.navigate('AadhaarUploadError', { - errorType: 'general', - } as never); + navigation.navigate('AadhaarUploadError', { + errorType: 'general', + });Apply the same fix at lines 128-130.
Also applies to: 128-130
🧹 Nitpick comments (12)
app/src/stores/settingStore.ts (1)
93-95: Consider reverting to destructuring for better type safety.The current approach uses type assertions (
as Partial<SettingsState>) to bypass TypeScript's checking, which contradicts the PR's goal of tightening types. Destructuring would leverage TypeScript's type system more effectively and eliminate the need for type assertions.Apply this diff to use a more type-safe destructuring approach:
partialize: state => { - const persistedState = { ...state }; - delete (persistedState as Partial<SettingsState>).hideNetworkModal; - delete (persistedState as Partial<SettingsState>).setHideNetworkModal; - return persistedState; + const { hideNetworkModal, setHideNetworkModal, ...persistedState } = state; + return persistedState; },This approach:
- Eliminates type assertions
- Makes it explicit which fields are excluded
- Automatically maintains type safety if
NonPersistedSettingsStateevolvespackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (1)
45-45: TODO: Implement error handling for scan failures.The TODO comment indicates missing error handling after scan failures. Currently, errors are logged and tracked but not surfaced to users. Consider implementing user-facing error feedback (e.g., toast notification, error state callback) to improve UX.
Do you want me to generate an implementation for error handling that includes user feedback mechanisms, or open a new issue to track this task?
app/src/utils/locale.web.ts (1)
20-24: Type augmentation correctly handles Intl.Locale.textInfo.The local type augmentation is a clean solution for accessing
textInfo.direction, which exists at runtime in modern browsers but isn't included in TypeScript's lib definitions. The optional properties appropriately align with the defensive optional chaining on line 32.Consider adding a brief comment explaining why this augmentation is necessary for future maintainers:
+ // Augment Intl.Locale to include textInfo (exists at runtime in modern browsers but not in TS lib) type LocaleWithTextInfo = Intl.Locale & { textInfo?: { direction?: string; }; };app/src/utils/keychainSecurity.ts (3)
48-51: Consider logging error details for debugging.Removing the unused error parameter is appropriate for lint compliance, but you've lost visibility into why biometrics might be unavailable. If there's an unexpected failure (not just "sensor not present"), debugging becomes harder.
Consider logging the error details while keeping the parameter unused from a linting perspective:
- } catch { - console.log('Biometrics not available'); + } catch (error) { + console.log('Biometrics not available:', error); return false; }
67-70: Log error details for better diagnostics.Same pattern here—while removing the unused parameter is correct for linting, you're discarding potentially useful diagnostic information. Knowing why the passcode check failed (e.g., permission denied vs. API unavailable) helps troubleshooting.
- } catch { - console.log('Device passcode not available'); + } catch (error) { + console.log('Device passcode not available:', error); return false; }
190-193: Preserve error context for security diagnostics.While the lint fix is valid, security-level detection failures warrant visibility into the underlying cause. When troubleshooting hardware security issues, the specific error can indicate whether it's a platform limitation, API unavailability, or something more serious.
- } catch { - console.log('Could not determine security level, defaulting to ANY'); + } catch (error) { + console.log('Could not determine security level, defaulting to ANY:', error); return Keychain.SECURITY_LEVEL.ANY; }app/src/screens/documents/selection/ConfirmBelongingScreen.tsx (1)
82-89: Consider logging the error for better diagnostics.While the fallback to default metadata is reasonable, the bare
catchsilently swallows any errors during document loading. This could hide important issues like data corruption or API failures.Apply this diff to add error logging:
- } catch { + } catch (error: unknown) { + console.error('Error loading document metadata, using defaults:', error); // setting defaults on error setDocumentMetadata({This preserves the type safety goals while maintaining debuggability.
app/src/screens/dev/DevFeatureFlagsScreen.tsx (1)
190-192: Fix works, but consider using ref-only instead of state + ref.The sync effect correctly keeps the ref updated for cleanup. However, since
debounceTimersis never used in the render (only in callbacks), maintaining it as state causes unnecessary re-renders when timers are added/removed.Consider refactoring to use only a ref:
- const [debounceTimers, setDebounceTimers] = useState< - Record<string, NodeJS.Timeout> - >({}); - const debounceTimersRef = useRef<Record<string, NodeJS.Timeout>>({}); + const debounceTimersRef = useRef<Record<string, NodeJS.Timeout>>({});Then update
debouncedSaveto work directly with the ref:const debouncedSave = useCallback( (flagKey: string, type: 'string' | 'number') => { // Clear existing timer for this flag if it exists - if (debounceTimers[flagKey]) { - clearTimeout(debounceTimers[flagKey]); + if (debounceTimersRef.current[flagKey]) { + clearTimeout(debounceTimersRef.current[flagKey]); } // Set a new timer const timer = setTimeout(() => { handleSaveTextFlag(flagKey, type); - setDebounceTimers(prev => { - const newTimers = { ...prev }; - delete newTimers[flagKey]; - return newTimers; - }); + delete debounceTimersRef.current[flagKey]; }, 500); // 500ms debounce delay - setDebounceTimers(prev => ({ - ...prev, - [flagKey]: timer, - })); + debounceTimersRef.current[flagKey] = timer; }, - [debounceTimers, handleSaveTextFlag], + [handleSaveTextFlag], );And remove the sync effect:
- useEffect(() => { - debounceTimersRef.current = debounceTimers; - }, [debounceTimers]);This eliminates unnecessary re-renders and simplifies the code while maintaining the same functionality.
app/src/utils/proving/validateDocument.ts (1)
179-182: Simplify the type definition.The current approach uses
Omitto remove fields only to re-add them as optional, which is unnecessarily complex. Consider usingPartial<Pick<...>>or simply an inline type for clarity.Apply this diff for a more straightforward definition:
-type MigratedPassportData = Omit<PassportData, 'documentCategory' | 'mock'> & { - documentCategory?: PassportData['documentCategory']; - mock?: PassportData['mock']; -}; +type MigratedPassportData = PassportData & { + documentCategory?: DocumentCategory; + mock?: boolean; +};app/src/screens/dev/DevLoadingScreen.tsx (1)
61-76: Consider moving these constants outside the component.While
useMemowith empty dependencies works correctly, these values are truly constant and could be defined outside the component for clarity and zero overhead:+const TERMINAL_STATES: ProvingStateType[] = [ + 'completed', + 'error', + 'failure', + 'passport_not_supported', + 'account_recovery_choice', + 'passport_data_not_found', +]; + +const SAFE_TO_CLOSE_STATES: ProvingStateType[] = [ + 'proving', + 'post_proving', + 'completed', +]; + const DevLoadingScreen: React.FC = () => { // ... - const terminalStates = useMemo<ProvingStateType[]>( - () => [ - 'completed', - 'error', - 'failure', - 'passport_not_supported', - 'account_recovery_choice', - 'passport_data_not_found', - ], - [], - ); - - const safeToCloseStates = useMemo<ProvingStateType[]>( - () => ['proving', 'post_proving', 'completed'], - [], - ); + const terminalStates = TERMINAL_STATES; + const safeToCloseStates = SAFE_TO_CLOSE_STATES;This achieves the same referential stability with clearer intent and no runtime overhead.
app/src/providers/selfClientProvider.tsx (1)
234-237: Redundant type assertion indicates type mismatch.The code declares
paramswith a type annotation and then immediately casts it to the same type withas RouteParams<'AadhaarUploadError'>. This pattern typically indicates the inferred type doesn't match the expected type. Remove the redundant cast and verify the route param definition matches the payload structure.- const params: RouteParams<'AadhaarUploadError'> = { - errorType, - } as RouteParams<'AadhaarUploadError'>; + const params: RouteParams<'AadhaarUploadError'> = { + errorType, + }; navigateIfReady('AadhaarUploadError', params);app/src/utils/logger.ts (1)
16-22: Approve logger configuration; cast is safe but consider widening options typeThe cast on line 22 bridges the type gap between
LokiTransportOptions(empty record) and the expectedobject; at runtimelokiTransportignores the options parameter, so no breakages. Consider redefiningLokiTransportOptionsasobjectto eliminate the cast.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
app/src/components/DelayedLottieView.tsx(2 hunks)app/src/components/buttons/AbstractButton.tsx(2 hunks)app/src/components/buttons/PrimaryButtonLongHold.tsx(0 hunks)app/src/components/buttons/PrimaryButtonLongHold.web.tsx(0 hunks)app/src/components/homeScreen/SvgXmlWrapper.native.tsx(1 hunks)app/src/components/homeScreen/SvgXmlWrapper.web.tsx(2 hunks)app/src/components/native/PassportCamera.tsx(1 hunks)app/src/components/native/QRCodeScanner.tsx(1 hunks)app/src/hooks/useAppUpdates.ts(1 hunks)app/src/hooks/useAppUpdates.web.ts(1 hunks)app/src/hooks/useModal.ts(2 hunks)app/src/navigation/app.tsx(2 hunks)app/src/navigation/index.tsx(2 hunks)app/src/providers/selfClientProvider.tsx(5 hunks)app/src/screens/account/recovery/AccountRecoveryChoiceScreen.tsx(3 hunks)app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx(3 hunks)app/src/screens/account/settings/CloudBackupScreen.tsx(2 hunks)app/src/screens/app/LoadingScreen.tsx(1 hunks)app/src/screens/dev/CreateMockScreen.tsx(3 hunks)app/src/screens/dev/CreateMockScreenDeepLink.tsx(2 hunks)app/src/screens/dev/DevFeatureFlagsScreen.tsx(3 hunks)app/src/screens/dev/DevLoadingScreen.tsx(1 hunks)app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx(4 hunks)app/src/screens/documents/aadhaar/AadhaarUploadedSuccessScreen.tsx(2 hunks)app/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsx(4 hunks)app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx(3 hunks)app/src/screens/documents/selection/ConfirmBelongingScreen.tsx(1 hunks)app/src/screens/home/HomeScreen.tsx(3 hunks)app/src/screens/home/ProofHistoryList.tsx(2 hunks)app/src/screens/home/ProofHistoryScreen.tsx(2 hunks)app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx(2 hunks)app/src/screens/onboarding/DisclaimerScreen.tsx(2 hunks)app/src/screens/verification/ProveScreen.tsx(3 hunks)app/src/screens/verification/QRCodeViewFinderScreen.tsx(2 hunks)app/src/stores/settingStore.ts(1 hunks)app/src/types/elliptic.d.ts(1 hunks)app/src/utils/analytics.ts(2 hunks)app/src/utils/keychainSecurity.ts(3 hunks)app/src/utils/locale.web.ts(1 hunks)app/src/utils/logger.ts(4 hunks)app/src/utils/logger/consoleInterceptor.ts(1 hunks)app/src/utils/logger/lokiTransport.ts(4 hunks)app/src/utils/logger/nativeLoggerBridge.ts(2 hunks)app/src/utils/nfcScanner.ts(4 hunks)app/src/utils/notifications/notificationService.shared.ts(1 hunks)app/src/utils/notifications/notificationService.ts(2 hunks)app/src/utils/passportReader.ts(1 hunks)app/src/utils/proving/validateDocument.ts(2 hunks)app/tsconfig.json(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)
💤 Files with no reviewable changes (2)
- app/src/components/buttons/PrimaryButtonLongHold.web.tsx
- app/src/components/buttons/PrimaryButtonLongHold.tsx
🧰 Additional context used
📓 Path-based instructions (2)
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
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/documents/selection/ConfirmBelongingScreen.tsxapp/src/screens/dev/DevFeatureFlagsScreen.tsxapp/src/components/buttons/AbstractButton.tsxapp/src/stores/settingStore.tsapp/src/screens/dev/CreateMockScreen.tsxapp/src/utils/analytics.tsapp/src/hooks/useAppUpdates.tsapp/src/screens/verification/QRCodeViewFinderScreen.tsxapp/src/types/elliptic.d.tsapp/src/utils/locale.web.tsapp/src/screens/app/LoadingScreen.tsxapp/src/components/homeScreen/SvgXmlWrapper.web.tsxapp/src/screens/documents/aadhaar/AadhaarUploadScreen.tsxapp/src/utils/logger/consoleInterceptor.tsapp/src/screens/verification/ProveScreen.tsxapp/src/screens/home/ProofHistoryScreen.tsxapp/src/navigation/index.tsxapp/src/screens/account/recovery/RecoverWithPhraseScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/utils/keychainSecurity.tsapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/components/DelayedLottieView.tsxapp/src/utils/logger/nativeLoggerBridge.tsapp/src/screens/account/settings/CloudBackupScreen.tsxapp/src/utils/proving/validateDocument.tsapp/src/utils/logger.tsapp/src/screens/home/ProofHistoryList.tsxapp/src/utils/notifications/notificationService.shared.tsapp/src/utils/passportReader.tsapp/src/utils/logger/lokiTransport.tsapp/src/screens/account/recovery/AccountRecoveryChoiceScreen.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/hooks/useModal.tsapp/src/providers/selfClientProvider.tsxapp/src/components/native/QRCodeScanner.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/components/native/PassportCamera.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/navigation/app.tsxapp/src/utils/notifications/notificationService.tsapp/src/screens/documents/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/utils/nfcScanner.tsapp/src/components/homeScreen/SvgXmlWrapper.native.tsxapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/home/HomeScreen.tsx
🧠 Learnings (12)
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/**/*.{js,ts,tsx,jsx} : Use `react-navigation/native` with `createStaticNavigation` for type-safe navigation in React Native.
Applied to files:
app/src/screens/dev/CreateMockScreen.tsxapp/src/hooks/useAppUpdates.tsapp/src/screens/verification/QRCodeViewFinderScreen.tsxapp/src/screens/home/ProofHistoryScreen.tsxapp/src/navigation/index.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/account/settings/CloudBackupScreen.tsxapp/src/screens/home/ProofHistoryList.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/hooks/useModal.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/screens/documents/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/home/HomeScreen.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
app/src/screens/verification/QRCodeViewFinderScreen.tsxapp/src/screens/verification/ProveScreen.tsxapp/src/navigation/index.tsxapp/src/screens/account/recovery/RecoverWithPhraseScreen.tsxapp/src/hooks/useAppUpdates.web.tsapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/screens/account/settings/CloudBackupScreen.tsxapp/src/screens/account/recovery/AccountRecoveryChoiceScreen.tsxapp/src/providers/selfClientProvider.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/aadhaar/AadhaarUploadedSuccessScreen.tsxapp/src/screens/home/HomeScreen.tsx
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Follow ESLint TypeScript-specific rules
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured
Applied to files:
app/tsconfig.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
app/src/utils/proving/validateDocument.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.
Applied to files:
app/src/screens/home/HomeScreen.tsx
🪛 Biome (2.1.2)
app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx
[error] 23-23: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ 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). (5)
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (58)
app/src/components/DelayedLottieView.tsx (1)
7-7: LGTM! Clean refactor to modern React import pattern.The switch from
React.forwardRefto the directly importedforwardRefhelper is correct and follows current React best practices. Type safety is preserved with the generic parameters, and the component behavior remains unchanged.Also applies to: 23-23
app/src/components/buttons/AbstractButton.tsx (1)
6-10: LGTM: Clean type addition for onLayout support.The
LayoutChangeEventimport andonLayoutprop are correctly typed. The implementation properly passes the prop through to the underlyingButtoncomponent via the spread operator (line 73), maintaining a clean separation between type definition and runtime behavior.Also applies to: 23-23
app/src/utils/notifications/notificationService.ts (3)
6-8: LGTM! Proper use of Firebase's native types.Adding
FirebaseMessagingTypesto the import and using the framework's native types is the correct approach. This eliminates reliance on a custom local interface and ensures compatibility with the Firebase SDK.
10-10: Good practice using type-only import.The
typekeyword import forDeviceTokenRegistrationis good practice—it signals this is a type-only dependency and enables better tree-shaking.
123-129: LGTM! Consistent use of Firebase types in message handlers.Using
FirebaseMessagingTypes.RemoteMessagefor both background and foreground handlers ensures type consistency with the Firebase SDK and improves type safety throughout the notification flow.packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (2)
14-15: LGTM: Typed exports improve API surface.The type-only and value exports for
MRZScannerViewalign with the PR's goal to tighten types and eliminateany. This pattern allows consumers to import the props type independently, which is a best practice for TypeScript libraries.
62-63: Platform-neutral date formatting is correct.The iOS-specific date formatting using
Platform.OScheck is appropriate here. React Native'sPlatformAPI provides a good abstraction for platform-specific behavior.app/src/utils/locale.web.ts (1)
26-26: LGTM: Type-safe cast eliminates need for type error suppression.The explicit cast to
LocaleWithTextInfoprovides a typed path totextInfo.direction(line 32) without usinganyor suppressing type errors, successfully achieving the PR's goal of tightening types across the workspace.app/src/screens/dev/DevFeatureFlagsScreen.tsx (3)
5-5: LGTM!Correctly imports
useReffor the new ref-based timer storage pattern.
47-47: LGTM!The ref declaration is properly typed and will store the current debounce timers for cleanup access.
197-197: LGTM!Cleanup now correctly accesses current timers via ref at unmount time, fixing the original bug where the empty dependency array captured stale state.
app/src/utils/proving/validateDocument.ts (1)
49-49: LGTM: Type safety improvement.Changing from
anytounknownenforces proper type checking at usage sites while maintaining flexibility for additional context fields.app/src/components/homeScreen/SvgXmlWrapper.web.tsx (1)
23-23: LGTM! Initialization matches the named import pattern.The change to
createDOMPurify(window)correctly aligns with the named import on line 5. The sanitization logic remains unchanged.app/src/components/homeScreen/SvgXmlWrapper.native.tsx (3)
5-6: Type import addition supports the elimination ofany.Adding
ComponentPropsenables deriving props fromRNSvgXmlinstead of usingany, which aligns with the PR's objective to tighten types.
9-9: Excellent type safety improvement.Deriving
PropsfromComponentProps<typeof RNSvgXml>ensures type consistency with the underlying library and eliminates manual maintenance. This is a best practice for wrapper components.
11-11: Approve removal of ref forwarding
No instances ofreffound on<SvgXml>usages in the codebase—removing theforwardRefwrapper is safe.app/src/components/native/QRCodeScanner.tsx (1)
56-64: LGTM! Error handling is now more explicit.Renaming
errortonativeErrorwhen destructuring avoids shadowing and makes the error construction clearer. The pattern is consistent with similar changes in PassportCamera.tsx.app/src/navigation/app.tsx (1)
14-14: LGTM! Modal screen params are now properly typed.Adding
ModalNavigationParamstype ensures type-safe navigation to the Modal screen, consistent with the broader navigation typing improvements across the codebase.Also applies to: 44-44
app/src/screens/home/ProofHistoryList.tsx (1)
67-68: LGTM! Navigation is now properly typed.The strongly-typed navigation hook ensures type-safe navigation throughout the component, aligning with the codebase-wide navigation typing improvements.
app/src/hooks/useAppUpdates.web.ts (1)
16-17: LGTM! Navigation typing is now consistent.The typed navigation hook ensures type-safe navigation calls, consistent with the codebase-wide improvements.
app/src/components/native/PassportCamera.tsx (1)
72-80: LGTM! Error handling matches the improved pattern.The error construction is now more explicit and avoids variable shadowing, consistent with the pattern in QRCodeScanner.tsx.
app/src/screens/verification/ProveScreen.tsx (1)
49-50: LGTM! Typed navigation with direct destructuring.The strongly-typed navigation ensures type safety while the direct
navigatedestructuring improves code readability.app/src/screens/account/settings/CloudBackupScreen.tsx (1)
185-186: LGTM! Navigation typing is consistent.The typed navigation hook in
BottomButtonensures type-safe navigation, completing the consistent typing pattern across the codebase.app/tsconfig.json (1)
18-25: LGTM! Source path resolution enhances development workflow.Adding source path mappings alongside distribution paths enables more flexible module resolution during development while maintaining compatibility with built outputs. This aligns well with the PR's goal of tightening TypeScript configuration across the workspace.
app/src/navigation/index.tsx (2)
52-53: LGTM! Strongly-typed screen props improve type safety.The
RootStackScreenPropstype alias provides a convenient way for screens to receive properly typed navigation and route props. This enhances IntelliSense and catches navigation-related type errors at compile time.
60-63: Good clarification on interface merging.The added comments clearly explain the purpose of the global namespace augmentation and the use of interface merging to avoid duplicate identifier errors. This helps future maintainers understand the pattern.
app/src/screens/verification/QRCodeViewFinderScreen.tsx (1)
37-38: LGTM! Typed navigation improves type safety.Replacing the untyped
useNavigation()withuseNavigation<NativeStackNavigationProp<RootStackParamList>>()provides compile-time validation of route names and parameters in navigation calls throughout this component (e.g., lines 61, 91, 112).app/src/screens/dev/CreateMockScreen.tsx (1)
164-165: LGTM! Consistent navigation typing.The strongly-typed navigation hook ensures that the
navigation.navigatecall at line 202 is validated against theRootStackParamList, catching invalid route names or parameters at compile time.app/src/hooks/useAppUpdates.ts (1)
18-19: LGTM! Type-safe navigation in custom hook.The typed navigation hook ensures that the
Modalroute parameters at line 46 are validated against theRootStackParamList, preventing runtime navigation errors.app/src/hooks/useModal.ts (1)
19-20: LGTM! Strongly-typed modal navigation.The typed navigation hook provides compile-time validation for the
Modalroute navigation at line 28 and enables type-safe access to navigation state at line 33.app/src/screens/home/ProofHistoryScreen.tsx (1)
62-63: LGTM! Type-safe navigation for history details.The strongly-typed navigation hook ensures that the
ProofHistoryDetailroute navigation at line 225 receives correctly typed parameters, catching data structure mismatches at compile time.app/src/screens/dev/CreateMockScreenDeepLink.tsx (1)
31-32: LGTM! Consistent navigation typing across the codebase.The strongly-typed navigation hook completes the consistent pattern of navigation type safety improvements across all files in this PR. The
ConfirmBelongingroute navigation at line 62 is now validated at compile time.app/src/screens/onboarding/DisclaimerScreen.tsx (1)
9-9: LGTM! Typed navigation properly implemented.The addition of
NativeStackNavigationProp<RootStackParamList>provides type-safe navigation without changing runtime behavior. This aligns well with the PR's goal to tighten types across the workspace.Also applies to: 19-19, 25-26
app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx (1)
8-8: LGTM! Typed navigation properly implemented.The typed navigation changes enhance type safety without altering functionality.
Also applies to: 18-18, 24-25
app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx (1)
11-11: LGTM! Typed navigation properly implemented.The navigation typing enhancements improve type safety throughout the recovery flow.
Also applies to: 20-20, 36-37
app/src/screens/account/recovery/AccountRecoveryChoiceScreen.tsx (1)
8-8: LGTM! Typed navigation properly implemented.The typed navigation pattern is consistently applied, improving type safety across the recovery flow.
Also applies to: 23-23, 42-43
app/src/screens/documents/aadhaar/AadhaarUploadedSuccessScreen.tsx (1)
8-8: LGTM! Typed navigation properly implemented.The navigation typing improves type safety while maintaining existing functionality.
Also applies to: 17-17, 23-24
app/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsx (1)
9-9: LGTM! Typed navigation setup is correct.The navigation type imports and initialization properly establish type-safe navigation.
Also applies to: 20-20, 95-96
app/src/screens/home/HomeScreen.tsx (1)
14-14: LGTM! Typed navigation properly implemented.The typed navigation setup correctly establishes type-safe navigation throughout the home screen.
Also applies to: 23-23, 32-33
app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx (1)
6-6: LGTM! Excellent type-safe image source handling.The use of
ImageSourcePropTypeto properly type the image source is a great example of type safety without resorting to unsafe casts. This is exactly what the PR aims to achieve.Also applies to: 35-35, 156-156
app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx (2)
31-31: LGTM! Strong typing improves navigation safety.The addition of
NativeStackNavigationProp<RootStackParamList>correctly types the navigation hook, eliminating potential runtime errors from incorrect route names or parameters. This aligns with the PR objective to tighten types across the workspace.Based on learnings
Also applies to: 52-52, 97-98
394-394: ConfirmBelonging route correctly defined ConfirmBelonging is declared in RootStackParamList via AppNavigation and accepts an empty params object.app/src/utils/logger/consoleInterceptor.ts (2)
14-19: LGTM! Type safety improvement.The
LoggerMethodsinterface provides a clear contract for the logger, and usingunknown[]for arguments is the right choice for type-safe variadic parameters.
21-45: LGTM! Console interception properly typed.All console method wrappers correctly use
unknown[]and maintain the dual-logging behavior (app logger + original console).app/src/utils/logger/lokiTransport.ts (3)
168-171: LGTM! Transport options properly typed.
LokiTransportOptionsasRecord<string, never>correctly indicates no configuration options are currently supported, and the transport function signature now uses proper generics.
146-155: Good addition: timer cleanup in flush.Clearing
batchTimerinflushLokiTransportprevents potential memory leaks and ensures clean shutdown behavior.
194-207: LGTM! Log object properly typed.The explicit type definition for
logObjecteliminatesanyusage and clearly documents the expected structure with optionaldatafield.app/src/utils/logger.ts (1)
38-39: LGTM! Public API surface expanded with useful types.Exporting
RootLogger,LoggerExtension, andlogLevelsenables type-safe logger usage throughout the codebase and allows consumers to create properly-typed logger extensions.Also applies to: 70-85
app/src/utils/logger/nativeLoggerBridge.ts (3)
7-7: LGTM! Type imports and safer data handling.Using type-only imports prevents circular dependencies, and changing
datafromanytounknownenforces explicit type checking at usage sites.Also applies to: 16-16
21-31: LGTM! Logger dependencies properly typed.The
injectedLoggersstate andsetupNativeLoggerBridgeparameters now use the publicLoggerExtensionandRootLoggertypes, eliminatinganyusage and enabling compile-time type checking.
76-87: LGTM! Logger routing with type safety.The
loggervariable is now properly typed asLoggerExtension, ensuring all logger method calls are type-checked.app/src/utils/analytics.ts (1)
280-284: LGTM: Guarding native callsGood defensive checks before calling trackEvent; avoids crashes when the module is absent.
app/src/utils/nfcScanner.ts (2)
13-17: LGTM: Typed import for AndroidScanResponseImporting the shared AndroidScanResponse type improves cross-file consistency.
228-229: LGTM: Explicit radix in parseIntUsing parseInt(key, 10) avoids octal pitfalls and ensures consistent mapping.
app/src/utils/passportReader.ts (4)
21-67: Strongly-typed module surfaces look solidDefining AndroidScanResponse and explicit Android/iOS module interfaces removes
anyusage and clarifies the public API. Good improvement for safety and maintainability.
110-153: LGTM: iOS scan normalizationNormalizing the iOS native result to an object (parsing when string) avoids downstream double-parsing. This pairs well with consumers expecting an object.
18-19: Confirmqualitysemantics and boundsAndroid defaulting quality to 1 may impact performance/time. Ensure acceptable range and document expected values (e.g., 0–1 vs 1–100) to prevent overflow or ineffective settings.
Also applies to: 94-104
75-86: Verify NativeModules names match native code
- Android:
RNPassportReadermatchesgetName()in RNPassportReaderModule.kt- iOS: unable to locate a
PassportReaderexport in native sources; please confirm the bridged module name
transphorm
changed the title
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 (7)
packages/mobile-sdk-alpha/src/context.tsx (1)
37-37: Removeanytype from event listener signature.The
anytype for the event payload parameter directly contradicts the PR's objective of enforcing@typescript-eslint/no-explicit-any. Event payloads should be properly typed based on the event type.Consider creating a typed event map that associates each event type with its payload type:
- listeners: Map<SdkEvents, Set<(p: any) => void>>; + listeners: Map<SdkEvents, Set<(payload: unknown) => void>>;Or, for better type safety, define an event payload map in
./types/events:// In ./types/events export type SdkEventPayloadMap = { [K in SdkEvents]: /* specific payload type for each event */; }; // Then use it here: listeners: { [K in SdkEvents]?: Set<(payload: SdkEventPayloadMap[K]) => void>; };packages/mobile-sdk-demo/tests/screens/index.test.ts (3)
36-36: Replaceas anywith proper types.The PR objectives explicitly aim to eliminate
anytypes and enforce@typescript-eslint/no-explicit-any. Casting toanycontradicts this goal.Consider defining proper return types for
getPropsmethods or using type guards to safely access the props:-const generateProps = screenMap.generate.getProps?.(context) as any; +const generateProps = screenMap.generate.getProps?.(context);If the return type is truly dynamic, define a union type or use proper type assertions with runtime validation.
43-43: Replaceas anywith proper types.Same issue as line 36—this
as anyassertion undermines the PR's type-safety objectives.Apply this diff:
-const registerProps = screenMap.register.getProps?.(context) as any; +const registerProps = screenMap.register.getProps?.(context);
47-47: Replaceas anywith proper types.Same issue as lines 36 and 43—this
as anyassertion conflicts with the PR's goal to eliminateanytypes.Apply this diff:
-const documentsProps = screenMap.documents.getProps?.(context) as any; +const documentsProps = screenMap.documents.getProps?.(context);app/src/utils/nfcScanner.ts (1)
127-187: Type uncertainty and iOS/Android inconsistency in response handling.The extensive
String()coercions raise concerns:
Line 129:
JSON.parse(String(parsed?.dataGroupHashes ?? '{}'))— IfdataGroupHashesis an object (not a string),String()produces"[object Object]", causingJSON.parseto throw. Android (line 205) doesJSON.parse(dataGroupHashes)directly, suggesting it's already a string.Lines 149-152:
JSON.parse(String(documentSigningCertificate)).PEMimplies the certificate is a JSON string containing aPEMfield. But Android (lines 209-212) treats it as a plain string for direct concatenation. This platform inconsistency suggests incomplete normalization or unclear types.Lines 154, 161, 168: Additional
String()coercions for base64 buffers. If types are correct, these are redundant; if not, they mask bugs.The comment "already parsed as an object" conflicts with the need for
JSON.parse. Either the type definitions are inaccurate or the normalization layer isn't fully unifying the response shapes.Consider:
- Tightening
AndroidScanResponseand the iOS response type to match actual runtime shapes- Removing redundant
String()calls if properties are truly strings- Adding explicit error handling for
JSON.parseand.PEMaccess to surface shape mismatches earlypackages/mobile-sdk-alpha/src/client.ts (1)
48-60: Residualanytypes contradict PR objective.The PR aims to "Enforce @typescript-eslint/no-explicit-any", yet multiple
anytypes remain in the event listener system:
Map<SDKEvent, Set<(p: any) => void>>(lines 48, 51, 76, 85)cb as anycasts (lines 55, 89, 91)- Callback return type
any(line 49)While the generic parameter
E extends SDKEventprovides type safety at the call site, the internal implementations still useany.Consider replacing
anywithunknownor properly typed alternatives:-export const createListenersMap = (): { - map: Map<SDKEvent, Set<(p: any) => void>>; - addListener: <E extends SDKEvent>(event: E, cb: (payload: SDKEventMap[E]) => any) => void; -} => { - const map = new Map<SDKEvent, Set<(p: any) => void>>(); +export const createListenersMap = (): { + map: Map<SDKEvent, Set<(p: unknown) => void>>; + addListener: <E extends SDKEvent>(event: E, cb: (payload: SDKEventMap[E]) => void) => void; +} => { + const map = new Map<SDKEvent, Set<(p: unknown) => void>>(); const addListener = <E extends SDKEvent>(event: E, cb: (payload: SDKEventMap[E]) => void) => { const set = map.get(event) ?? new Set(); - set.add(cb as any); + set.add(cb as (p: unknown) => void); map.set(event, set); };Apply similar changes to lines 76, 85, 89, 91.
Also applies to: 76-76, 85-92
packages/mobile-sdk-alpha/src/types/public.ts (1)
190-191: WebSocket callbacks useanyinstead ofunknown.The
onMessageandonErrorcallbacks useany, which contradicts the PR's objective to eliminateanytypes. Since WebSocket data and errors require runtime validation anyway,unknownenforces safer type checking.Apply this diff:
export interface WsConn { send: (data: string | ArrayBufferView | ArrayBuffer) => void; close: () => void; - onMessage: (cb: (data: any) => void) => void; - onError: (cb: (e: any) => void) => void; + onMessage: (cb: (data: unknown) => void) => void; + onError: (cb: (e: unknown) => void) => void; onClose: (cb: () => void) => void; }
♻️ Duplicate comments (1)
app/src/utils/nfcScanner.ts (1)
96-123: iOS scanning now correctly unified with cross-platform path.The refactor to use the normalized
scanDocumentexport for iOS successfully addresses the previous review concern about divergent platform behavior and potential JSON-shape mismatches.
🧹 Nitpick comments (7)
app/src/screens/dev/DevLoadingScreen.tsx (1)
61-76: Consider moving constant arrays outside the component.While
useMemocorrectly creates stable references for theuseEffectdependency array (line 104), these arrays contain constant values that never change. Moving them outside the component asconstdeclarations would eliminate theuseMemooverhead entirely.Apply this refactor to move the arrays outside the component:
+const TERMINAL_STATES: ProvingStateType[] = [ + 'completed', + 'error', + 'failure', + 'passport_not_supported', + 'account_recovery_choice', + 'passport_data_not_found', +]; + +const SAFE_TO_CLOSE_STATES: ProvingStateType[] = [ + 'proving', + 'post_proving', + 'completed', +]; + const DevLoadingScreen: React.FC = () => { const [currentState, setCurrentState] = useState<ProvingStateType>('idle'); const [documentType, setDocumentType] = useState<provingMachineCircuitType>('dsc'); // ... - const terminalStates = useMemo<ProvingStateType[]>( - () => [ - 'completed', - 'error', - 'failure', - 'passport_not_supported', - 'account_recovery_choice', - 'passport_data_not_found', - ], - [], - ); - - const safeToCloseStates = useMemo<ProvingStateType[]>( - () => ['proving', 'post_proving', 'completed'], - [], - ); useEffect(() => { // ... - setCanCloseApp(safeToCloseStates.includes(currentState)); - setShouldLoopAnimation(!terminalStates.includes(currentState)); - }, [currentState, documentType, safeToCloseStates, terminalStates]); + setCanCloseApp(SAFE_TO_CLOSE_STATES.includes(currentState)); + setShouldLoopAnimation(!TERMINAL_STATES.includes(currentState)); + }, [currentState, documentType]);app/src/screens/app/LoadingScreen.tsx (1)
101-103: Consider adding safe error logging for debugging.The parameterless catch is consistent since the error wasn't being used. However, the console.error on line 102 logs a static message without any error details, which may hinder debugging in production.
Consider logging the error type or a sanitized message while adhering to the PII guidelines:
- } catch { - console.error('Error loading selected document:'); + } catch (error) { + console.error('Error loading selected document:', error instanceof Error ? error.message : 'Unknown error');app/scripts/alias-imports.cjs (3)
33-38: Add empty spec validation for consistency.The export declarations section validates for empty spec at line 85 (
if (!spec) continue;), but the import section lacks this check. WhilegetModuleSpecifierValue()typically returns a string, explicit validation improves robustness.Apply this diff:
const spec = declaration.getModuleSpecifierValue(); + if (!spec) continue; // Skip existing alias imports if (spec.startsWith('@/') || spec.startsWith('@tests/')) {
22-121: Extract common transformation logic to reduce duplication.The import (lines 23-69) and export (lines 74-120) declaration handling share nearly identical logic: module specifier validation, alias checks, relative path resolution, and containment verification. This duplication increases maintenance burden and risk of divergence.
Consider extracting the common logic into a helper function:
function transformDeclaration(declaration, dir, srcDir, testsDir) { const moduleSpecifier = declaration.getModuleSpecifier(); if ( !moduleSpecifier || moduleSpecifier.getKind() !== SyntaxKind.StringLiteral ) { return false; } const spec = declaration.getModuleSpecifierValue(); if (!spec) return false; // Skip existing alias imports/exports if (spec.startsWith('@/') || spec.startsWith('@tests/')) { return false; } // Handle relative imports/exports if (!spec.startsWith('./') && !spec.startsWith('../')) return false; const abs = path.resolve(dir, spec); let baseDir = null; let baseAlias = null; const relFromSrc = path.relative(srcDir, abs); if (!relFromSrc.startsWith('..') && !path.isAbsolute(relFromSrc)) { baseDir = srcDir; baseAlias = '@'; } else { const relFromTests = path.relative(testsDir, abs); if ( !relFromTests.startsWith('..') && !path.isAbsolute(relFromTests) ) { baseDir = testsDir; baseAlias = '@tests'; } else { return false; } } const newSpec = determineAliasStrategy(dir, abs, baseDir, baseAlias); declaration.setModuleSpecifier(newSpec); return true; }Then use it in both loops:
for (const declaration of sourceFile.getImportDeclarations()) { try { transformDeclaration(declaration, dir, srcDir, testsDir); } catch (error) { console.warn(`Skipping import declaration: ${error.message}`); continue; } } for (const declaration of sourceFile.getExportDeclarations()) { try { transformDeclaration(declaration, dir, srcDir, testsDir); } catch (error) { console.warn(`Skipping export declaration: ${error.message}`); continue; } }
124-175: Apply consistent error handling to require() calls.Unlike import/export declarations (which have try-catch wrappers at lines 23-69 and 74-120), the require() calls section lacks error handling. If malformed AST nodes or edge cases cause errors here, the script will fail rather than skip problematic calls.
Wrap the require() call processing in a try-catch for consistency:
for (const call of requireCalls) { + try { const expression = call.getExpression(); const exprText = expression.getText(); // ... rest of logic ... const newSpec = determineAliasStrategy(dir, abs, baseDir, baseAlias); arg.setLiteralValue(newSpec); + } catch (error) { + console.warn(`Skipping require/import call: ${error.message}`); + continue; + } }packages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx (1)
34-34: Unused variable can be removed.The variable
_parsedPassportDatais assigned but never used. Consider removing this line or completing the intended implementation if this was left as a TODO.Apply this diff to remove the unused variable:
- const _parsedPassportData = initPassportDataParsing((scanResult as NFCScanResult).passportData, skiPem); + initPassportDataParsing((scanResult as NFCScanResult).passportData, skiPem);packages/mobile-sdk-alpha/src/client.ts (1)
155-155: Preferunknownoveranyfor log details.The
detailsparameter usesRecord<string, any>, which contradicts the PR's goal to eliminateany.Apply this diff:
- logProofEvent: (level: LogLevel, message: string, context: ProofContext, details?: Record<string, any>) => { + logProofEvent: (level: LogLevel, message: string, context: ProofContext, details?: Record<string, unknown>) => { emit(SdkEvents.PROOF_EVENT, { context, event: message, details, level }); },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
.github/workflows/circuits-build.yml(1 hunks)app/.eslintrc.cjs(1 hunks)app/scripts/alias-imports.cjs(1 hunks)app/src/components/homeScreen/idCard.tsx(1 hunks)app/src/components/native/PassportCamera.tsx(2 hunks)app/src/components/native/PassportCamera.web.tsx(1 hunks)app/src/mocks/react-native-community-blur.ts(1 hunks)app/src/mocks/react-native-svg.ts(1 hunks)app/src/navigation/index.tsx(2 hunks)app/src/providers/authProvider.tsx(1 hunks)app/src/providers/selfClientProvider.tsx(5 hunks)app/src/screens/app/LoadingScreen.tsx(2 hunks)app/src/screens/dev/DevLoadingScreen.tsx(2 hunks)app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx(4 hunks)app/src/screens/documents/management/IdDetailsScreen.tsx(1 hunks)app/src/screens/home/HomeScreen.tsx(2 hunks)app/src/screens/verification/ProofRequestStatusScreen.tsx(1 hunks)app/src/types/elliptic.d.ts(1 hunks)app/src/utils/analytics.ts(3 hunks)app/src/utils/deeplinks.ts(1 hunks)app/src/utils/keychainSecurity.ts(4 hunks)app/src/utils/nfcScanner.ts(5 hunks)app/src/utils/notifications/notificationService.ts(2 hunks)app/src/utils/proving/loadingScreenStateText.ts(1 hunks)app/tests/src/hooks/useAppUpdates.test.tsx(1 hunks)app/tests/utils/deeplinks.test.ts(1 hunks)packages/mobile-sdk-alpha/.eslintrc.cjs(1 hunks)packages/mobile-sdk-alpha/src/client.ts(2 hunks)packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx(1 hunks)packages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx(1 hunks)packages/mobile-sdk-alpha/src/context.tsx(1 hunks)packages/mobile-sdk-alpha/src/documents/utils.ts(1 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/stores/mrzStore.tsx(1 hunks)packages/mobile-sdk-alpha/src/types/events.ts(1 hunks)packages/mobile-sdk-alpha/src/types/public.ts(1 hunks)packages/mobile-sdk-alpha/tests/client.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/documents/utils.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.ts(1 hunks)packages/mobile-sdk-alpha/tests/proving/provingMachine.test.ts(1 hunks)packages/mobile-sdk-demo/.eslintrc.cjs(1 hunks)packages/mobile-sdk-demo/App.tsx(1 hunks)packages/mobile-sdk-demo/src/components/DocumentScanResultCard.tsx(1 hunks)packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx(1 hunks)packages/mobile-sdk-demo/src/components/ScreenLayout.tsx(1 hunks)packages/mobile-sdk-demo/src/hooks/useMRZScanner.ts(1 hunks)packages/mobile-sdk-demo/src/screens/HomeScreen.tsx(1 hunks)packages/mobile-sdk-demo/src/utils/camera.ts(1 hunks)packages/mobile-sdk-demo/tests/screens/HomeScreen.test.tsx(1 hunks)packages/mobile-sdk-demo/tests/screens/index.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- app/tests/utils/deeplinks.test.ts
- .github/workflows/circuits-build.yml
- app/src/components/homeScreen/idCard.tsx
- app/src/utils/deeplinks.ts
- packages/mobile-sdk-demo/src/components/DocumentScanResultCard.tsx
- app/src/screens/verification/ProofRequestStatusScreen.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src/utils/notifications/notificationService.ts
- app/src/utils/analytics.ts
- app/src/providers/selfClientProvider.tsx
- packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
- app/src/screens/documents/aadhaar/AadhaarUploadScreen.tsx
- app/src/utils/keychainSecurity.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsxpackages/mobile-sdk-demo/src/utils/camera.tspackages/mobile-sdk-demo/src/hooks/useMRZScanner.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tsapp/tests/src/hooks/useAppUpdates.test.tsxapp/src/components/native/PassportCamera.tsxpackages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.tsapp/src/utils/proving/loadingScreenStateText.tspackages/mobile-sdk-demo/App.tsxpackages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tsapp/src/providers/authProvider.tsxapp/src/utils/nfcScanner.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/src/context.tsxapp/src/navigation/index.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/app/LoadingScreen.tsxpackages/mobile-sdk-demo/tests/screens/HomeScreen.test.tsxpackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tsapp/src/mocks/react-native-svg.tsapp/src/screens/documents/management/IdDetailsScreen.tsxpackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-demo/src/components/ScreenLayout.tsxapp/src/mocks/react-native-community-blur.tsapp/src/components/native/PassportCamera.web.tsxpackages/mobile-sdk-alpha/src/client.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.tspackages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-demo/src/screens/HomeScreen.tsxpackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsxapp/src/types/elliptic.d.tsapp/src/screens/home/HomeScreen.tsxpackages/mobile-sdk-demo/tests/screens/index.test.ts
packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}: Do NOT mock @selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('@selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Use actual imports from @selfxyz/mobile-sdk-alpha in tests
Write integration tests that exercise the real validation logic (not mocks)
Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Ensure parseNFCResponse() works with representative, synthetic NFC data
Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors
Files:
packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tspackages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.tspackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tspackages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.tspackages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-alpha/src/client.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.tspackages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.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:
packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tsapp/tests/src/hooks/useAppUpdates.test.tsxpackages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.tspackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-demo/tests/screens/HomeScreen.test.tsxpackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.tspackages/mobile-sdk-demo/tests/screens/index.test.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tspackages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.tspackages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-alpha/src/client.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.tspackages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/tests/src/hooks/useAppUpdates.test.tsxapp/src/components/native/PassportCamera.tsxapp/src/utils/proving/loadingScreenStateText.tsapp/src/providers/authProvider.tsxapp/src/utils/nfcScanner.tsapp/src/navigation/index.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/mocks/react-native-svg.tsapp/src/screens/documents/management/IdDetailsScreen.tsxapp/src/mocks/react-native-community-blur.tsapp/src/components/native/PassportCamera.web.tsxapp/src/types/elliptic.d.tsapp/src/screens/home/HomeScreen.tsx
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/components/native/PassportCamera.tsxapp/src/utils/proving/loadingScreenStateText.tsapp/src/providers/authProvider.tsxapp/src/utils/nfcScanner.tsapp/src/navigation/index.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/mocks/react-native-svg.tsapp/src/screens/documents/management/IdDetailsScreen.tsxapp/src/mocks/react-native-community-blur.tsapp/src/components/native/PassportCamera.web.tsxapp/src/types/elliptic.d.tsapp/src/screens/home/HomeScreen.tsx
app/**/*.{ios,android,web}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Explain platform-specific code paths in the PR description (files with .ios, .android, or .web extensions)
Files:
app/src/components/native/PassportCamera.web.tsx
🧠 Learnings (17)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-demo/src/utils/camera.tspackages/mobile-sdk-demo/src/hooks/useMRZScanner.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tspackages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
packages/mobile-sdk-demo/src/utils/camera.tspackages/mobile-sdk-demo/App.tsxpackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-alpha/.eslintrc.cjspackages/mobile-sdk-alpha/src/client.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.tspackages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.tsapp/src/utils/proving/loadingScreenStateText.tspackages/mobile-sdk-demo/App.tsxpackages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.tspackages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-demo/.eslintrc.cjspackages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-demo/tests/screens/HomeScreen.test.tsxpackages/mobile-sdk-alpha/tests/proving/provingMachine.test.tspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-demo/src/components/ScreenLayout.tsxpackages/mobile-sdk-alpha/.eslintrc.cjsapp/src/components/native/PassportCamera.web.tsxpackages/mobile-sdk-alpha/src/client.tspackages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.tspackages/mobile-sdk-alpha/src/components/MRZScannerView.tsxpackages/mobile-sdk-demo/src/screens/HomeScreen.tsxpackages/mobile-sdk-demo/tests/screens/index.test.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Use `renderHook` for testing custom React hooks.
Applied to files:
app/tests/src/hooks/useAppUpdates.test.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
app/src/components/native/PassportCamera.tsxpackages/mobile-sdk-alpha/src/context.tsxapp/src/navigation/index.tsxapp/src/screens/home/HomeScreen.tsx
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
packages/mobile-sdk-demo/App.tsxpackages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-demo/.eslintrc.cjspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-alpha/.eslintrc.cjsapp/src/components/native/PassportCamera.web.tsxpackages/mobile-sdk-alpha/src/client.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
packages/mobile-sdk-demo/App.tsxpackages/mobile-sdk-demo/.eslintrc.cjspackages/mobile-sdk-alpha/tests/client.test.tspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/src/documents/utils.tspackages/mobile-sdk-alpha/.eslintrc.cjspackages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
app/src/utils/nfcScanner.tspackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Applied to files:
packages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-alpha/tests/client.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase
Applied to files:
packages/mobile-sdk-alpha/tests/documents/utils.test.tspackages/mobile-sdk-demo/.eslintrc.cjspackages/mobile-sdk-alpha/src/types/public.tspackages/mobile-sdk-alpha/.eslintrc.cjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Follow ESLint TypeScript-specific rules
Applied to files:
packages/mobile-sdk-demo/.eslintrc.cjspackages/mobile-sdk-alpha/.eslintrc.cjs
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/**/*.{js,ts,tsx,jsx} : Use `react-navigation/native` with `createStaticNavigation` for type-safe navigation in React Native.
Applied to files:
app/src/navigation/index.tsxapp/src/screens/home/HomeScreen.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors
Applied to files:
packages/mobile-sdk-alpha/tests/client.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured
Applied to files:
packages/mobile-sdk-alpha/.eslintrc.cjs
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)
Applied to files:
packages/mobile-sdk-alpha/.eslintrc.cjs
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.
Applied to files:
app/src/screens/home/HomeScreen.tsx
🧬 Code graph analysis (4)
app/scripts/alias-imports.cjs (1)
app/scripts/analyze-tree-shaking.cjs (1)
srcDir(264-264)
app/src/utils/nfcScanner.ts (1)
common/src/constants/constants.ts (1)
document_type(2-2)
app/src/screens/dev/DevLoadingScreen.tsx (1)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
ProvingStateType(341-364)
app/src/screens/home/HomeScreen.tsx (1)
app/src/navigation/index.tsx (1)
RootStackParamList(54-72)
⏰ 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). (5)
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (44)
packages/mobile-sdk-demo/src/components/ScreenLayout.tsx (1)
6-7: LGTM! Type-only import separation improves tree-shaking.Splitting
ViewStyleinto a type-only import follows TypeScript best practices and supports better build optimization. The runtime imports remain correct.app/src/mocks/react-native-svg.ts (1)
5-6: LGTM! Type-only import optimization.Splitting the React import into type-only (
Reactnamespace) and value imports (createElement,forwardRef) is correct and aligns with TypeScript best practices. AllReact.*references in this file are type annotations, so this change improves bundle size through better tree-shaking while maintaining full functionality.packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
6-7: LGTM! Type-only import separation follows best practices.Splitting
ScrollViewPropsinto a type-only import improves tree-shaking and makes the distinction between types and runtime values explicit.app/tests/src/hooks/useAppUpdates.test.tsx (1)
5-5: LGTM: Type-only import correctly applied.The conversion to
import typeis appropriate sinceReactNodeis only used in type position (line 41). This aligns with the PR's goal of tightening TypeScript types and has no runtime impact.packages/mobile-sdk-alpha/src/context.tsx (1)
5-9: LGTM! Type-only imports correctly separated.The split between type-only imports (PropsWithChildren, SdkEvents) and runtime imports (createContext, useContext, useMemo) is correct and improves bundle size by allowing tree-shaking of type-only references.
packages/mobile-sdk-alpha/tests/proving/internal/statusHandlers.test.ts (1)
12-13: LGTM! Clean type-only import separation.Splitting the import into type-only (
import type) and value imports aligns with TypeScript best practices and the PR's goal of tightening types. The test coverage remains solid—exercising real validation logic without mocks, as required by the SDK testing guidelines.app/src/screens/dev/DevLoadingScreen.tsx (1)
5-5: LGTM! Type-only imports are correctly applied.The conversion to type-only imports for
LottieView,provingMachineCircuitType, andProvingStateTypeis correct and follows TypeScript best practices. These symbols are only used as types in the code.Also applies to: 10-13
packages/mobile-sdk-alpha/tests/proving/provingMachine.test.ts (1)
10-11: LGTM! Import separation follows TypeScript best practices.The change correctly separates type-only imports from runtime imports.
SelfClientis used only in type positions throughout the test (type assertions), whileSdkEventsis accessed at runtime. This improves type safety and aligns with the PR objective of tightening types.app/src/screens/app/LoadingScreen.tsx (1)
5-5: Type-only imports are correctly applied.The conversion to type-only imports for
LottieViewandProvingStateTypeis appropriate since both are used exclusively in type positions. This improves build output and aligns with TypeScript best practices.Also applies to: 13-13
packages/mobile-sdk-demo/tests/screens/index.test.ts (1)
3-4: LGTM!The type-only import split is correct.
ScreenContextis used solely for type annotation, soimport typeappropriately prevents it from being included in the runtime bundle.packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (1)
11-12: LGTM!The import split correctly separates type-only imports from value imports. This aligns with TypeScript best practices and the PR's objective to tighten types across the mobile app.
packages/mobile-sdk-demo/tests/screens/HomeScreen.test.tsx (1)
7-8: LGTM!The import split is correct and consistent with the pattern used in the component file. This enhances type safety without affecting test runtime behavior.
app/src/utils/nfcScanner.ts (2)
215-215: LGTM: Explicit radix prevents octal interpretation.Specifying radix 10 in
parseInt(key, 10)is a best practice that avoids unexpected behavior with strings like"08".
53-60: Verify error messages don't leak sensitive data.The error logging stringifies caught exceptions. If native module errors embed document numbers, DOB, or other PII in their messages, this would violate the coding guideline to never log sensitive data.
Review the native module error messages to confirm they don't include:
- Passport/document numbers
- Dates of birth/expiry
- Names or other PII fields
If native errors can contain such data, redact before logging (e.g., mask document numbers as
***-***-1234).As per coding guidelines.
app/src/screens/home/HomeScreen.tsx (2)
14-14: LGTM on typed navigation setup.The type-only imports and typed navigation prop correctly strengthen type safety. The changes align with the PR objective to eliminate
anytypes and tighten typing across the mobile app.Also applies to: 16-18, 24-24, 33-34
57-57: Remove unnecessaryas nevercasts in navigation.navigate
LaunchandIdDetailsare declared inRootStackParamList, so these casts bypass type safety and can be removed.app/src/utils/proving/loadingScreenStateText.ts (1)
5-8: LGTM! Type-only import is correct.The conversion to
import typeis appropriate sinceprovingMachineCircuitTypeandProvingStateTypeare only used as type annotations in function signatures, not as runtime values.app/src/screens/documents/management/IdDetailsScreen.tsx (1)
11-11: LGTM! Type-only import is correct.Converting to
import typeis appropriate sinceDocumentCatalogandIDDocumentare only used for type annotations and not as runtime values.packages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx (1)
12-12: LGTM! Type-only import is correct.The conversion to
import typeis appropriate sinceNFCScanResultis only used in a type assertion on line 34, not as a runtime value.packages/mobile-sdk-demo/App.tsx (1)
11-12: LGTM! Import split correctly separates types from values.The separation of type-only imports (
ScreenContext,ScreenRoute) from value imports (screenMap) is correct and aligns with the PR's objective to enforce consistent type imports.app/.eslintrc.cjs (1)
176-184: LGTM! ESLint rule enforces consistent type imports.Adding
@typescript-eslint/consistent-type-importswithprefer: 'type-imports'correctly enforces the type import pattern demonstrated throughout this PR. ThedisallowTypeAnnotations: falsesetting appropriately allows type annotations in inline contexts like function signatures.app/src/mocks/react-native-community-blur.ts (1)
5-6: LGTM! Import split correctly separates React type from runtime utilities.The separation is correct:
Reactis only used for type annotations (React.FC,React.HTMLAttributes), whilecreateElementandforwardRefare used as runtime values.app/src/components/native/PassportCamera.tsx (2)
9-10: LGTM! Import split correctly separates type from runtime value.The separation is appropriate:
SelfClientis only used as a type in thePassportCameraPropsinterface (line 53), whileuseSelfClientis used as a runtime hook.
73-80: Good improvement to error handling.Using an alias (
nativeError) when destructuring avoids naming collision with the Error constructor and makes the code clearer. Settinge.name = nativeErroris more explicit than the previouse.name = error.app/src/providers/authProvider.tsx (2)
15-16: LGTM! Import split correctly separates types from runtime value.The separation is appropriate:
GetOptionsandSetOptionsare only used as type annotations, whileKeychainis used as a runtime value for keychain operations.
23-27: LGTM! Type-only import is correct.Converting
GetSecureOptionsto a type-only import is appropriate since it's only used as a type annotation in function signatures (lines 41, 72, etc.), not as a runtime value.packages/mobile-sdk-alpha/.eslintrc.cjs (1)
73-80: LGTM!The
@typescript-eslint/consistent-type-importsrule is correctly configured to enforce type-only imports across the mobile SDK, aligning with the broader type-safety initiative in this PR. ThedisallowTypeAnnotations: falseoption appropriately permits inline type annotations.packages/mobile-sdk-demo/.eslintrc.cjs (1)
37-44: LGTM!Consistent with the mobile-sdk-alpha ESLint configuration. Enforcing type-only imports in the demo package maintains consistency across the workspace.
packages/mobile-sdk-alpha/tests/documents/utils.test.ts (1)
10-11: LGTM!The conversion to type-only imports for
DocumentsAdapterandSelfClientis correct. These types are used exclusively for type annotations increateMockSelfClientWithDocumentsAdapter(line 13) and not at runtime.app/src/navigation/index.tsx (2)
74-75: LGTM!The
RootStackScreenProps<T>helper type simplifies screen component typing and provides a consistent pattern for accessing navigation props throughout the app.
51-72: Route parameter definitions align with navigation usage
Allnavigatecalls supply the required params (optional properties omitted where allowed). No changes needed.packages/mobile-sdk-alpha/tests/proving/provingMachine.generatePayload.test.ts (1)
5-5: LGTM!The conversion to a type-only import for
SelfClientis correct. It's used exclusively as a type annotation for the mock client (lines 105, 113) without runtime value access.packages/mobile-sdk-alpha/src/types/events.ts (1)
5-5: LGTM!The conversion to a type-only import for
DocumentCategoryis correct. It's used exclusively as a type annotation in theSDKEventMapinterface (line 151) without runtime value access.packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts (1)
12-12: LGTM!The conversion to a type-only import for
MRZInfois correct. It's used exclusively for typing mock objects throughout the test (lines 71, 115, 156, 189, 222) without runtime value access.packages/mobile-sdk-alpha/tests/client.test.ts (1)
9-9: LGTM: Correct type-only import conversion.The change to
import typeforAuthAdapteris appropriate since it's only used as a type annotation (line 214). This improves tree-shaking and follows TypeScript best practices.packages/mobile-sdk-demo/src/hooks/useMRZScanner.ts (1)
11-12: LGTM: Proper separation of type and value imports.Correctly splits
NormalizedMRZResult(used only in type positions) into a type-only import while keepingnormalizeMRZPayloadas a value import since it's called at runtime (line 131).packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)
7-7: LGTM: Correct type-only import.
MRZInfois only used in type position (line 9'sPick<MRZInfo, ...>), making the type-only import appropriate and beneficial for bundle size.packages/mobile-sdk-demo/src/utils/camera.ts (1)
5-6: LGTM: Appropriate type-only imports.Correctly separates
MRZInfoandMRZValidation(used only in type positions) fromextractMRZInfo(called at runtime on lines 97 and 103). This pattern optimizes the bundle size.packages/mobile-sdk-alpha/src/documents/utils.ts (1)
22-22: LGTM: Correct type-only import for SelfClient.
SelfClientis used exclusively as a type annotation in function parameters throughout this file (lines 24, 49, 94, 119, 132, etc.), making the type-only import correct and beneficial.app/src/components/native/PassportCamera.web.tsx (1)
7-8: LGTM: Proper import separation.Correctly separates
SelfClient(used only in type position on line 16) fromuseSelfClient(called at runtime on line 24). This follows the established pattern across the codebase.packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (1)
10-10: LGTM: Correct type-only import.
MRZInfois used solely as a type in theMRZScannerViewPropsinterface (line 50), making the type-only import appropriate. This aligns with the broader type-import migration across the SDK.packages/mobile-sdk-alpha/tests/proving/provingMachine.startFetchingData.test.ts (1)
5-5: LGTM: Appropriate type-only import in test.
SelfClientis used exclusively as a type annotation for the mock client (lines 40 and 62), making the type-only import correct. The test properly uses actual SDK utilities rather than mocking the SDK itself, aligning with coding guidelines.packages/mobile-sdk-alpha/src/client.ts (1)
9-9: LGTM! Type-only import conversions are correct.The separation of type-only imports (
ProofContext,SDKEvent,SDKEventMap) from the runtime value import (SdkEvents) is precisely aligned with TypeScript best practices and the PR objective.Also applies to: 14-15, 25-25
packages/mobile-sdk-alpha/src/types/public.ts (1)
5-5: LGTM! Type-only imports correctly applied.All imports are appropriately converted to type-only since they're exclusively used in type positions within the
SelfClientinterface definition. This eliminates unnecessary runtime dependencies and aligns with the PR's goal to tighten TypeScript types.Also applies to: 10-14
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/providers/selfClientProvider.tsx (1)
39-54: Remove the navigation casts and let TypeScript validate the call.Replacing the old suppression with
(navigationRef.navigate as …)androute as neverstill bypasses the navigator’s overloads, so we’ve lost the type-safety this helper is supposed to guarantee. Please branch onparamsand callnavigationRef.navigate(route)ornavigationRef.navigate(route, params)directly so the compiler can enforce the correct tuple.Apply this diff:
function navigateIfReady<RouteName extends keyof RootStackParamList>( route: RouteName, ...args: undefined extends RootStackParamList[RouteName] ? [params?: RootStackParamList[RouteName]] : [params: RootStackParamList[RouteName]] ): void { if (navigationRef.isReady()) { const params = args[0]; - if (params !== undefined) { - (navigationRef.navigate as (r: RouteName, p: typeof params) => void)( - route, - params, - ); - } else { - navigationRef.navigate(route as never); - } + if (params === undefined) { + navigationRef.navigate(route); + } else { + navigationRef.navigate(route, params); + } } }
🧹 Nitpick comments (2)
app/scripts/alias-imports.cjs (2)
23-79: Error handling improvement addresses past concern.The added try-catch with logging resolves the previous issue about silent error swallowing. The warning messages now provide file context and error details, which aids debugging.
One optional enhancement: Consider whether the try-catch around
console.debug(lines 72-74) is necessary. Ifdeclaration.getText()can genuinely throw in edge cases, keep it; otherwise, simplify:- try { - console.debug('Import declaration text:', declaration.getText()); - } catch {} + console.debug('Import declaration text:', declaration.getText());
84-140: Symmetric export handling mirrors import logic.The export declaration processing applies the same validation and error handling patterns as imports, maintaining consistency. The same optional simplification regarding the try-catch around
console.debug(lines 133-135) applies here as well.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
app/package.json(1 hunks)app/scripts/alias-imports.cjs(1 hunks)app/src/navigation/index.tsx(2 hunks)app/src/providers/selfClientProvider.tsx(5 hunks)app/src/types/elliptic.d.ts(0 hunks)packages/mobile-sdk-alpha/src/types/public.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/types/elliptic.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/navigation/index.tsx
- packages/mobile-sdk-alpha/src/types/public.ts
🧰 Additional context used
📓 Path-based instructions (5)
app/package.json
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests
Files:
app/package.json
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts
Files:
app/package.json
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/providers/selfClientProvider.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/providers/selfClientProvider.tsx
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/selfClientProvider.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
app/src/providers/selfClientProvider.tsx
🧬 Code graph analysis (1)
app/src/providers/selfClientProvider.tsx (1)
app/src/navigation/index.tsx (2)
RootStackParamList(54-72)navigationRef(78-78)
⏰ 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). (4)
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (1)
app/scripts/alias-imports.cjs (1)
24-31: LGTM! Solid validation logic.The module specifier validation correctly guards against processing invalid declarations before attempting transformations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/utils/nfcScanner.ts (1)
102-160: Unify iOS path to use the normalizedscanDocumentto avoid divergent behavior and shape mismatchesThis dual-path implementation (preferring
scanPassportwith a fallback toscanDocument) creates a divergent iOS codepath that risks inconsistencies in response shapes, double JSON parsing, or test vs production drift.As previously noted, prefer using the normalized
scanDocumentexport consistently for both test and production scenarios to ensure uniform response handling across platforms.
🧹 Nitpick comments (2)
app/src/utils/nfcScanner.ts (2)
165-168: Type assertion doesn't protect againstnullresponseThe type assertion
response as Record<string, unknown>doesn't preventnullfrom passing through. While the?? {}fallback helps, the assertion is misleading about type safety.Apply this diff to improve null safety:
- const parsed: Record<string, unknown> = - typeof response === 'string' - ? (JSON.parse(response) as Record<string, unknown>) - : ((response as Record<string, unknown>) ?? {}); + const parsed: Record<string, unknown> = + typeof response === 'string' + ? (JSON.parse(response) as Record<string, unknown>) + : (response && typeof response === 'object' ? (response as Record<string, unknown>) : {});
233-233: Remove redundantString()coercionSince
mrzis already validated as a string on line 177, theString()coercion here is unnecessary.Apply this diff:
- const document_type = String(mrz).length === 88 ? 'passport' : 'id_card'; + const document_type = mrz.length === 88 ? 'passport' : 'id_card';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/utils/nfcScanner.ts(6 hunks)packages/mobile-sdk-alpha/src/types/public.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/mobile-sdk-alpha/src/types/public.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/utils/nfcScanner.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/utils/nfcScanner.ts
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/utils/nfcScanner.ts
🧠 Learnings (1)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
app/src/utils/nfcScanner.ts
🧬 Code graph analysis (1)
app/src/utils/nfcScanner.ts (2)
app/src/mocks/react-native-passport-reader.ts (1)
PassportReader(8-28)common/src/constants/constants.ts (1)
document_type(2-2)
⏰ 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). (5)
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
🔇 Additional comments (1)
app/src/utils/nfcScanner.ts (1)
277-277: LGTM!Good use of explicit radix parameter in
parseIntto prevent octal interpretation issues.
transphorm
changed the title
transphorm
deleted the
codex/eliminate-any-types-and-lint-issues-in-app
branch
Summary
@typescript-eslint/no-explicit-anyand align TypeScript path aliases for the mobile app workspaceanyTesting
https://chatgpt.com/codex/tasks/task_b_68e0bf51e4e4832d968e1a93299361ee
Summary by CodeRabbit
Bug Fixes
Performance
Developer Experience