MOVING COUNTRIES: #1229
MOVING COUNTRIES: #1229
Conversation
WalkthroughReplaces external country ISO utilities with internal mappings; introduces useCountries hook and exports; adds event-driven document selection flow; updates NFC scanner to read MRZ from store; consolidates color tokens via SDK export; removes SelfMobileSdk entry and related tests; adjusts build/exports and adds react-native-localize with test mocks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CountryPicker
participant SDKClient as SDK Client
participant AppNav as App Navigation
Note over CountryPicker,SDKClient: Country selection via hook + events
User->>CountryPicker: Select country
CountryPicker->>SDKClient: emit DOCUMENT_COUNTRY_SELECTED {countryCode, documentTypes}
SDKClient-->>AppNav: DOCUMENT_COUNTRY_SELECTED listener
AppNav->>AppNav: navigate(IDPicker, payload)
User->>IDPicker: Select document type
IDPicker->>SDKClient: emit DOCUMENT_TYPE_SELECTED {type, name, countryCode, countryName}
SDKClient-->>AppNav: DOCUMENT_TYPE_SELECTED listener
alt type in [p,i]
AppNav->>AppNav: navigate(DocumentOnboarding)
else type == a
AppNav->>AppNav: navigate(AadhaarUpload)
else
AppNav->>AppNav: navigate(ComingSoon)
end
sequenceDiagram
autonumber
participant NFC as NFCScannerScreen
participant MRZStore
participant NFCModule as NFC Reader
participant Handler as onSuccess/onFailure
Note over NFC,MRZStore: MRZ fetched from store (not via props)
NFC->>MRZStore: getMRZ()
MRZStore-->>NFC: {documentNumber, dateOfBirth, dateOfExpiry}
NFC->>NFCModule: startScan(mrzData)
alt scan success
NFCModule-->>NFC: ScanResultNFC
NFC->>Handler: onSuccess(result)
else error
NFCModule-->>NFC: Error
NFC->>Handler: onFailure(error)
end
sequenceDiagram
autonumber
participant Hook as useCountries
participant Net as https api.staging.self.xyz/id-picker
participant Localize as react-native-localize
Hook->>Localize: getLocales()/getCountry()
Localize-->>Hook: 2-letter region code
Hook->>Hook: alpha2ToAlpha3 lookup (internal)
Hook->>Net: fetch country data
Net-->>Hook: { data: CountryData }
Hook->>Hook: set state {countryData, countryList, userCountryCode, showSuggestion}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/screens/document/IDPickerScreen.tsx (1)
86-97: Fix incorrect variable naming in event payload.Line 89 assigns the result of
getDocumentName(docType)(which returns document names like "Passport", "ID card", "Aadhaar") to a variable namedcountryName, then passes this ascountryNamein the event payload on line 95. This mislabels the data—the event payload should distinguish between document name and country name.Apply this diff:
const onSelectDocumentType = (docType: string) => { buttonTap(); - const countryName = getDocumentName(docType); + const documentDisplayName = getDocumentName(docType); selfClient.emit(SdkEvents.DOCUMENT_TYPE_SELECTED, { documentType: docType, documentName: getDocumentNameForEvent(docType), countryCode: countryCode, - countryName: countryName, + countryName: commonNames[countryCode as keyof typeof commonNames] || countryCode, }); };You'll need to import
commonNames:+import { commonNames } from '@selfxyz/common/constants/countries';packages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx (1)
22-43: AddmrzDatato callback dependencies.The
onNFCScancallback capturesmrzData(line 28-30) but doesn't list it in the dependencies array (line 42). While the inline arrow function on line 48 mitigates stale closures in this specific case, the callback should declare all captured values for correctness.Apply this diff:
[client, onSuccess, onFailure], + [client, mrzData, onSuccess, onFailure], );
🧹 Nitpick comments (5)
packages/mobile-sdk-alpha/src/documents/useCountries.tsx (2)
46-53: API response structure not validated.The code checks
result.status === 'success'but doesn't validate the structure ofresult.data. If the API returns malformed data (e.g.,datais not an object or is missing), it could cause runtime errors when accessingcountryDatakeys.Add basic validation:
if (result.status === 'success') { + if (typeof result.data === 'object' && result.data !== null) { setCountryData(result.data); if (__DEV__) { console.log('Set country data:', result.data); } + } else { + console.error('API returned invalid data structure:', result); + } } else { console.error('API returned non-success status:', result.status); }
39-43: Consider adding fetch timeout.The fetch relies solely on AbortController for cleanup on unmount, but doesn't have a timeout. If the API is slow or unresponsive, users could remain in the loading state indefinitely.
Add a timeout to improve UX:
const fetchCountryData = async () => { try { + const timeoutId = setTimeout(() => controller.abort(), 10000); // 10s timeout const response = await fetch('https://api.staging.self.xyz/id-picker', { signal: controller.signal, }); + clearTimeout(timeoutId); const result = await response.json();app/src/components/flag/RoundFlag.tsx (1)
9-9: Consider usinggetCountryISO2for API consistency.While
alpha3ToAlpha2works correctly, other parts of the codebase (CreateMockScreen.tsx:25, generateCountryOptions.ts:6) usegetCountryISO2for the same purpose. Using the unified API would improve consistency.Apply this diff if you prefer consistency:
-import { alpha3ToAlpha2 } from '@selfxyz/common/constants/countries'; +import { getCountryISO2 } from '@selfxyz/common/constants/countries';- const iso2 = alpha3ToAlpha2(normalizedCountryCode); + const iso2 = getCountryISO2(normalizedCountryCode);Also applies to: 45-45
common/src/constants/countries.ts (1)
7-9: Consider explicit return type for consistency.The
alpha3ToAlpha2function lacks an explicit return type whilealpha2ToAlpha3(lines 3-5) explicitly returnsCountry3LetterCode | undefined. Adding an explicit return type would improve consistency and self-documentation.Apply this diff:
-export function alpha3ToAlpha2(key: string) { +export function alpha3ToAlpha2(key: string): string | undefined { return ALPHA3_TO_ALPHA2[key.toUpperCase()]; }packages/mobile-sdk-alpha/package.json (1)
141-141: Consider constraining the react-native-localize peer dependency.The peer dependency uses a wildcard
"*", which accepts any version. While this maximizes compatibility, it may lead to runtime issues if incompatible versions are installed. Consider using a more specific range like"^3.5.0"to match your devDependency version.Apply this diff:
- "react-native-localize": "*", + "react-native-localize": "^3.5.0",
📜 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 (25)
app/package.json(0 hunks)app/src/components/flag/RoundFlag.tsx(2 hunks)app/src/providers/selfClientProvider.tsx(1 hunks)app/src/screens/dev/CreateMockScreen.tsx(1 hunks)app/src/screens/dev/CreateMockScreenDeepLink.tsx(1 hunks)app/src/screens/dev/DevSettingsScreen.tsx(1 hunks)app/src/screens/document/CountryPickerScreen.tsx(4 hunks)app/src/screens/document/IDPickerScreen.tsx(1 hunks)app/src/utils/colors.ts(1 hunks)common/package.json(0 hunks)common/src/constants/countries.ts(2 hunks)common/src/scripts/generateCountryOptions.ts(1 hunks)packages/mobile-sdk-alpha/package.json(3 hunks)packages/mobile-sdk-alpha/src/browser.ts(1 hunks)packages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx(1 hunks)packages/mobile-sdk-alpha/src/constants/colors.ts(1 hunks)packages/mobile-sdk-alpha/src/documents/useCountries.tsx(1 hunks)packages/mobile-sdk-alpha/src/entry/index.tsx(0 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)packages/mobile-sdk-alpha/src/stores/mrzStore.tsx(4 hunks)packages/mobile-sdk-alpha/src/types/events.ts(1 hunks)packages/mobile-sdk-alpha/tests/SelfMobileSdk.test.tsx(0 hunks)packages/mobile-sdk-alpha/tests/entry.test.tsx(0 hunks)packages/mobile-sdk-alpha/tests/setup.ts(1 hunks)packages/mobile-sdk-alpha/tsup.config.ts(1 hunks)
💤 Files with no reviewable changes (5)
- app/package.json
- packages/mobile-sdk-alpha/tests/entry.test.tsx
- common/package.json
- packages/mobile-sdk-alpha/tests/SelfMobileSdk.test.tsx
- packages/mobile-sdk-alpha/src/entry/index.tsx
🧰 Additional context used
📓 Path-based instructions (11)
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/types/events.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsxpackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/tests/setup.tspackages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/src/documents/useCountries.tsxpackages/mobile-sdk-alpha/src/constants/colors.ts
**/*.{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-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/index.tsapp/src/screens/dev/DevSettingsScreen.tsxcommon/src/constants/countries.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxapp/src/utils/colors.tsapp/src/providers/selfClientProvider.tsxcommon/src/scripts/generateCountryOptions.tspackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsxapp/src/screens/document/IDPickerScreen.tsxpackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/tests/setup.tsapp/src/screens/dev/CreateMockScreen.tsxpackages/mobile-sdk-alpha/src/browser.tsapp/src/screens/dev/CreateMockScreenDeepLink.tsxpackages/mobile-sdk-alpha/src/documents/useCountries.tsxapp/src/screens/document/CountryPickerScreen.tsxpackages/mobile-sdk-alpha/src/constants/colors.tsapp/src/components/flag/RoundFlag.tsx
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/types/events.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/stores/mrzStore.tsxpackages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsxpackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/tests/setup.tspackages/mobile-sdk-alpha/src/browser.tspackages/mobile-sdk-alpha/src/documents/useCountries.tsxpackages/mobile-sdk-alpha/src/constants/colors.ts
packages/mobile-sdk-alpha/src/index.ts
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Files:
packages/mobile-sdk-alpha/src/index.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/dev/DevSettingsScreen.tsxapp/src/utils/colors.tsapp/src/providers/selfClientProvider.tsxapp/src/screens/document/IDPickerScreen.tsxapp/src/screens/dev/CreateMockScreen.tsxapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/document/CountryPickerScreen.tsxapp/src/components/flag/RoundFlag.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/screens/dev/DevSettingsScreen.tsxapp/src/utils/colors.tsapp/src/providers/selfClientProvider.tsxapp/src/screens/document/IDPickerScreen.tsxapp/src/screens/dev/CreateMockScreen.tsxapp/src/screens/dev/CreateMockScreenDeepLink.tsxapp/src/screens/document/CountryPickerScreen.tsxapp/src/components/flag/RoundFlag.tsx
common/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
common/src/**/*.{ts,tsx,js,jsx}: Review shared utilities for:
- Reusability and modular design
- Type safety and error handling
- Side-effect management
- Documentation and naming clarity
Files:
common/src/constants/countries.tscommon/src/scripts/generateCountryOptions.ts
packages/mobile-sdk-alpha/package.json
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
packages/mobile-sdk-alpha/package.json: Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)
Files:
packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/**/package.json
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/package.json: Ensure package exports are properly configured
Verify package conditions are valid (e.g., exports conditions)
Files:
packages/mobile-sdk-alpha/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:
packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/tests/setup.ts
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise
Files:
packages/mobile-sdk-alpha/tests/setup.ts
🧠 Learnings (13)
📚 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-alpha/src/index.tspackages/mobile-sdk-alpha/package.jsonapp/src/utils/colors.tspackages/mobile-sdk-alpha/tsup.config.tspackages/mobile-sdk-alpha/src/browser.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/src/index.tspackages/mobile-sdk-alpha/tests/setup.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/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Applied to files:
packages/mobile-sdk-alpha/src/index.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: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
packages/mobile-sdk-alpha/src/index.tsapp/src/screens/document/IDPickerScreen.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/stores/mrzStore.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/**/package.json : Ensure package exports are properly configured
Applied to files:
packages/mobile-sdk-alpha/package.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/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/package.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 : Verify package conditions are valid (e.g., exports conditions)
Applied to files:
packages/mobile-sdk-alpha/package.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:
packages/mobile-sdk-alpha/package.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:
packages/mobile-sdk-alpha/tests/setup.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 app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks
Applied to files:
packages/mobile-sdk-alpha/tests/setup.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 jest.setup.js : Comprehensive mocks for all native modules must be set up in `jest.setup.js`.
Applied to files:
packages/mobile-sdk-alpha/tests/setup.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/tests/setup.ts : Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise
Applied to files:
packages/mobile-sdk-alpha/tests/setup.ts
🧬 Code graph analysis (6)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)
packages/mobile-sdk-alpha/src/types/public.ts (1)
MRZInfo(32-39)
app/src/providers/selfClientProvider.tsx (3)
packages/mobile-sdk-alpha/src/browser.ts (1)
SdkEvents(41-41)packages/mobile-sdk-alpha/src/index.ts (1)
SdkEvents(73-73)app/src/navigation/index.tsx (1)
navigationRef(51-51)
packages/mobile-sdk-alpha/src/components/screens/NFCScannerScreen.tsx (3)
packages/mobile-sdk-alpha/src/types/ui.ts (1)
ScreenProps(31-34)packages/mobile-sdk-alpha/src/context.tsx (1)
useSelfClient(64-68)packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)
useMRZStore(45-74)
packages/mobile-sdk-alpha/src/documents/useCountries.tsx (2)
common/src/constants/countries.ts (1)
alpha2ToAlpha3(3-5)packages/mobile-sdk-alpha/src/index.ts (1)
useCountries(110-110)
app/src/screens/document/CountryPickerScreen.tsx (1)
packages/mobile-sdk-alpha/src/documents/useCountries.tsx (1)
useCountries(32-81)
app/src/components/flag/RoundFlag.tsx (1)
common/src/constants/countries.ts (1)
alpha3ToAlpha2(7-9)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-ios
🔇 Additional comments (14)
app/src/screens/dev/DevSettingsScreen.tsx (1)
215-215: LGTM - Alphabetization improves developer UX.Sorting the screen list enhances the dropdown's usability for quick navigation during development.
packages/mobile-sdk-alpha/src/browser.ts (1)
72-72: LGTM - Export follows SDK patterns.The new
useCountriesexport aligns with the SDK's public API surface and follows established re-export conventions.packages/mobile-sdk-alpha/tests/setup.ts (1)
172-194: LGTM - Comprehensive mock for locale detection.The mock implementation provides all necessary react-native-localize functions with sensible test defaults, supporting the new useCountries hook's test requirements.
packages/mobile-sdk-alpha/src/index.ts (1)
110-110: LGTM - Export follows SDK conventions.Re-exporting
useCountriesvia the main index aligns with the SDK's established patterns for public API surfaces.packages/mobile-sdk-alpha/tsup.config.ts (1)
41-41: LGTM - Build entry for colors module.Adding the colors entry point expands the SDK's public API surface appropriately, following the established pattern for other constant exports.
app/src/screens/dev/CreateMockScreenDeepLink.tsx (1)
13-13: LGTM - Dependency internalized.Replacing the external
country-iso-3-to-2library with the internal utility aligns with the PR objective to consolidate country code handling in@selfxyz/common.packages/mobile-sdk-alpha/src/types/events.ts (1)
71-71: LGTM - Documentation clarification.The updated comment provides clearer guidance on the recommended navigation target for unsupported documents.
app/src/providers/selfClientProvider.tsx (1)
216-245: LGTM! Event-driven navigation pattern implemented correctly.The new listeners for
DOCUMENT_COUNTRY_SELECTEDandDOCUMENT_TYPE_SELECTEDproperly guard navigation withnavigationRef.isReady()checks, consistent with existing patterns in this file. The switch-based routing by document type is clear and maintainable.app/src/screens/document/CountryPickerScreen.tsx (1)
61-62: LGTM! Clean migration to useCountries hook.The migration from manual country data management to the
useCountrieshook is well-executed. The event-driven flow (emittingDOCUMENT_COUNTRY_SELECTEDfor supported countries andPROVING_PASSPORT_NOT_SUPPORTEDotherwise) aligns with the new navigation pattern established inselfClientProvider.tsx.Also applies to: 82-92
packages/mobile-sdk-alpha/package.json (1)
54-59: LGTM! Colors export properly configured.The new
./constants/colorsexport follows the established pattern with correct condition mappings for types, react-native, import, and require. This aligns with the tree-shaking configuration and ESM-friendly exports.Based on coding guidelines.
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)
62-69: LGTM! Clean implementation of the MRZ subset accessor.The
getMRZmethod correctly maps the internalpassportNumberfield to the publicdocumentNumberfield, maintaining consistency with theMRZInfointerface. The use ofget()ensures fresh state access.app/src/utils/colors.ts (1)
5-5: LGTM! Consolidates color tokens via SDK re-export.This aligns with the broader refactoring to centralize color definitions in the mobile SDK package.
packages/mobile-sdk-alpha/src/constants/colors.ts (2)
53-55: Verify duplicate color values for teal scale.Both
teal300andteal500are set to#5EEAD4. In typical color scales, 300 and 500 shades should differ in lightness/saturation. Confirm this is intentional or correct the values.
33-41: Verify duplicate color values for slate scale.Both
slate50andslate100are set to#F8FAFC. Typically, 50 is lighter than 100 in standard color scales. Confirm this duplication is intentional or update one of the values.
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Error detecting user country:', error); |
There was a problem hiding this comment.
Error object logged in production may expose sensitive details.
Line 28 logs the entire error object to console in production builds. Error objects can contain stack traces, internal paths, or other implementation details that shouldn't be exposed.
Apply this diff to restrict detailed logging to development:
} catch (error) {
- console.error('Error detecting user country:', error);
+ if (__DEV__) {
+ console.error('Error detecting user country:', error);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.error('Error detecting user country:', error); | |
| try { | |
| // … existing logic … | |
| } catch (error) { | |
| if (__DEV__) { | |
| console.error('Error detecting user country:', error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/documents/useCountries.tsx around line 28, the
code currently logs the full error object to console which can expose sensitive
details in production; replace that direct error dump with an
environment-guarded log so detailed error info is only printed in non-production
(e.g. if process.env.NODE_ENV !== 'production' log the full error or stack),
while in production log a minimal, non-sensitive message (e.g. "Error detecting
user country") and optionally send the full error to your internal
telemetry/monitoring instead.
| const controller = new AbortController(); | ||
| const fetchCountryData = async () => { | ||
| try { | ||
| const response = await fetch('https://api.staging.self.xyz/id-picker', { |
There was a problem hiding this comment.
Hardcoded staging URL will break in production.
The fetch uses https://api.staging.self.xyz/id-picker, which will fail in production environments. This endpoint should be configurable via the SDK config or environment-specific.
Consider making the API endpoint configurable:
- const response = await fetch('https://api.staging.self.xyz/id-picker', {
+ const response = await fetch(config.countryPickerEndpoint || 'https://api.self.xyz/id-picker', {Or pass it as a parameter to the hook if different consumers need different endpoints.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/documents/useCountries.tsx around line 41, the
hook currently hardcodes the staging URL
('https://api.staging.self.xyz/id-picker') which will fail in production; change
the hook to read the endpoint from the SDK configuration or an environment
variable (e.g., SDK.config.idPickerUrl or process.env.ID_PICKER_URL) and fall
back to a sensible default, or add an optional parameter to the hook so callers
can pass a custom URL; validate the value before calling fetch and update any
initialization/tests to supply the configured endpoint.
There was a problem hiding this comment.
@aaronmgdr Understood, thank you for clarifying. I'll note that this endpoint configuration is intentional for your setup.
moves country picker hooks to msdk
removes
country-iso-3-to-2andi18n-iso-countriesfrom app because why have 2 different dependencies for what are just object hashmaps?sets
NFCScannerScreenfrom msdk to use the mrz storemoves colors to the msdk -- gonna need these there if we are moving over entire screens
sorts list of screens on dev screen by alpha for sanity
removes old conceptual SelfMobileSDK component
Summary by CodeRabbit
New Features
Improvements
Chores