Add skipReselect Option #1561
Add skipReselect Option #1561
Conversation
📝 WalkthroughWalkthroughAdds try/catch error handling and DEV-only logging around feedback hide/show cleanup, and introduces an optional Changes
Sequence Diagram(s)(Skipped — changes are parameter propagation and localized error handling without new multi-component control flow that requires sequencing.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/integrations/nfc/nfcScanner.ts (1)
18-30: Critical: Add missingskipReselectproperty toInputsinterface.Line 94 references
inputs.skipReselect, but theInputsinterface doesn't declare this property. This causes the TypeScript compilation error reported in the pipeline.🔧 Proposed fix
interface Inputs { passportNumber: string; dateOfBirth: string; dateOfExpiry: string; canNumber?: string; useCan?: boolean; skipPACE?: boolean; skipCA?: boolean; extendedMode?: boolean; usePacePolling?: boolean; sessionId: string; userId?: string; + skipReselect?: boolean; }app/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsx (2)
25-32: Critical: NFCParams type missing skipReselect field.The
NFCParamstype doesn't includeskipReselect?: boolean, but the new NFC method at line 47 passes{ skipReselect: true }in its params. This breaks type safety—the code only compiles because of theas neverassertion at line 146.🔧 Proposed fix: Add skipReselect to NFCParams
type NFCParams = { skipPACE?: boolean; canNumber?: string; useCan?: boolean; skipCA?: boolean; extendedMode?: boolean; usePacePolling?: boolean; + skipReselect?: boolean; };
146-146: Remove type assertion after fixing NFCParams.The
as neverassertion bypasses TypeScript's type checking and masks the missingskipReselectfield inNFCParams. Once the type is corrected (see previous comment), this assertion should be removed or replaced with a proper type.♻️ Proposed fix after NFCParams is updated
- navigation.navigate('DocumentNFCScan', params as never); + navigation.navigate('DocumentNFCScan', params);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/hooks/useFeedbackAutoHide.tsapp/src/hooks/useFeedbackModal.tsapp/src/integrations/nfc/nfcScanner.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/types/reactNativePassportReader.d.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: 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/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use React Navigation withcreateStaticNavigationfor type-safe navigation in React Native applications.
Implement platform-specific handling withPlatform.OS === 'ios' ? 'iOS' : 'Android'checks before platform-specific code in React Native.
Initialize native modules withinitializeNativeModules()before any native operations in React Native.
Implement lazy loading for screens usingReact.lazy()in React Native applications.
Implement custom modal system withuseModalhook and callback registry in React Native.
Integrate haptic feedback usinguseHapticNavigationhook in React Native navigation.
Use platform-specific initial routes: web uses 'Home', mobile uses 'Splash' in React Navigation.
Use Zustand for global state management in React Native applications.
Use custom hooks for complex state (useModal,useHapticNavigation) instead of inline logic.
Use AsyncStorage for simple data, SQLite for complex data, and Keychain for sensitive data in React Native.
Use@/alias for src imports and@tests/alias for test imports in TypeScript/JavaScript files.
Use conditional rendering with Platform.OS for platform-specific code in React Native.
Use Tamagui for UI components in React Native applications.
Do not log sensitive data in production, including identity verification and passport information.
Use Keychain for secure storage of sensitive data in React Native.
Implement proper cleanup of sensitive data after use.
Implement certificate validation for passport data verification.
Always use try-catch for async operations in React Native and TypeScript code.
Implement graceful degradation when native modules fail in React Native.
Provide user-friendly error messages in UI and error handlers.
Lazy load screens and components to optimize bundle size in React Native.
Prevent memory leaks in native modules in React Native.
Files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
**/*.{tsx,jsx,ts,js}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper cleanup in useEffect and component unmount hooks in React.
Files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
**/{mobile,client,app,time,verification}/**/*.{ts,tsx,js,swift,kt}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
Use server-signed time tokens or chain block timestamps for trusted time in mobile clients, do not trust device wall-clock alone
Files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
**/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt}
📄 CodeRabbit inference engine (.cursor/rules/compliance-verification.mdc)
**/{mobile,client,app,proof,zk}/**/*.{ts,tsx,js,swift,kt}: Include trusted time anchor in proof generation and verify time anchor authenticity before proof generation in mobile implementations
Achieve proof generation in <60 seconds on mid-tier mobile devices
Files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Ensure
yarn typespasses (TypeScript validation) before creating a PR
Files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
app/**/*.{ts,tsx,js,jsx}: Ensure web build succeeds withyarn webbefore creating a PR
Do not include sensitive data in logs - avoid logging PII, credentials, and tokens
Usereact-native-dotenvfor environment configuration via@envimport
Confirm no sensitive data exposed before PR merge
Files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.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/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.tsapp/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Implement comprehensive error boundaries in React components.
Files:
app/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:04.954Z
Learning: Explain platform-specific code paths (iOS/Android/Web) in PR descriptions
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Implement certificate validation for passport data verification.
Applied to files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Test `isPassportDataValid()` with realistic, synthetic passport data (never use real user data)
Applied to files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.ts
📚 Learning: 2025-11-18T12:17:14.819Z
Learnt from: seshanthS
Repo: selfxyz/self PR: 1337
File: packages/mobile-sdk-alpha/src/processing/mrz.ts:189-194
Timestamp: 2025-11-18T12:17:14.819Z
Learning: In packages/mobile-sdk-alpha/src/processing/mrz.ts, the checkScannedInfo function and related TD1 extraction/validation logic are only reached on Android. iOS uses native Swift parsing (LiveMRZScannerView.swift) that bypasses this TypeScript layer.
Applied to files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/integrations/nfc/nfcScanner.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Ensure no breaking changes to public API or document them properly
Applied to files:
app/src/types/reactNativePassportReader.d.tsapp/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.ts
📚 Learning: 2025-10-23T12:08:55.529Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1236
File: packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx:356-378
Timestamp: 2025-10-23T12:08:55.529Z
Learning: In packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx, the NFC native events emitted via NativeEventEmitter are generic status strings (e.g., "PACE succeeded", "BAC failed", "Reading DG1 succeeded") and do not contain sensitive MRZ data or passport numbers, so they do not require sanitization before logging.
Applied to files:
app/src/integrations/nfc/passportReader.tsapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsxapp/src/integrations/nfc/nfcScanner.ts
📚 Learning: 2025-12-25T19:19:35.354Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-12-25T19:19:35.354Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.test.{ts,tsx} : Ensure `parseNFCResponse()` works with representative, synthetic NFC data
Applied to files:
app/src/integrations/nfc/passportReader.tsapp/src/integrations/nfc/nfcScanner.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use custom hooks for complex state (`useModal`, `useHapticNavigation`) instead of inline logic.
Applied to files:
app/src/hooks/useFeedbackModal.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Implement custom modal system with `useModal` hook and callback registry in React Native.
Applied to files:
app/src/hooks/useFeedbackModal.ts
📚 Learning: 2025-11-25T14:06:55.970Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T14:06:55.970Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Always use try-catch for async operations in React Native and TypeScript code.
Applied to files:
app/src/hooks/useFeedbackModal.tsapp/src/hooks/useFeedbackAutoHide.ts
🪛 GitHub Actions: Mobile CI
app/src/integrations/nfc/nfcScanner.ts
[error] 94-94: TypeScript error TS2339: Property 'skipReselect' does not exist on type 'Inputs'.
⏰ 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: test-common
- GitHub Check: type-check
- GitHub Check: analyze-android
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
🔇 Additional comments (7)
app/src/hooks/useFeedbackModal.ts (1)
30-49: LGTM - Solid defensive error handling.The try/catch blocks around Sentry feedback operations prevent crashes from native module failures, with sensible fallbacks and dev-only logging. This aligns with the guideline to always use try-catch for async/native operations in React Native.
Also applies to: 55-61, 73-79
app/src/hooks/useFeedbackAutoHide.ts (1)
20-26: LGTM - Consistent error handling in cleanup.The try/catch wrapper in the cleanup function prevents errors during screen unfocus from crashing the app. Consistent with the pattern in useFeedbackModal.ts.
app/src/types/reactNativePassportReader.d.ts (1)
14-14: LGTM - Type definition properly extended.The optional
skipReselectproperty is correctly added to theScanOptionsinterface, maintaining backward compatibility with existing code.app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx (1)
84-84: LGTM - skipReselect parameter properly integrated.The
skipReselectparameter is correctly added to the route params type (line 84), destructured from route params (line 336), and passed to the scan function (line 351). Implementation follows the existing pattern for optional NFC scan parameters.Also applies to: 330-337, 351-351
app/src/integrations/nfc/passportReader.ts (2)
19-19: LGTM: Android-specific NFC option added correctly.The optional
skipReselectparameter is properly added to theScanOptionstype and will be Android-only in practice (iOS scan path doesn't use it).
87-109: No action needed—sessionIdforwarding is correct and intentional.Both platform implementations properly require
sessionId: the Android module accepts it via theScanOptionsabstraction, and the iOS implementation explicitly includes it as a parameter in thescanPassportsignature (line 56). This is not a separate bundled fix—it's part of the unified type-safe abstraction across both platforms.app/src/screens/documents/scanning/DocumentNFCMethodSelectionScreen.tsx (1)
42-48: LGTM: Android-specific NFC option properly configured.The new
skipReselectmethod is correctly configured for Android-only with appropriate label and description. The params will be properly typed onceNFCParamsis updated.
Description
skipReselectoption to DocumentNFCMethodSelection screenTested
Manually tested the values passed using logcat.
How to QA
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.