Revert "Mixpanel tweaks" #974
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 7536875.
Contributor
WalkthroughRefactors mocks and NFC integration. Updates Jest NetInfo mocks. Replaces the mock PassportReader object with top-level functions. Switches analytics to use NativeModules.PassportReader and makes several APIs synchronous. Adjusts iOS NFC scanning to call NativeModules.PassportReader.scanDocument with expanded arguments; Android path unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant App as App (JS)
participant ANA as Analytics (JS)
participant NM as NativeModules.PassportReader
Note over ANA,App: Analytics config and event tracking are now synchronous (fire-and-forget)
U->>App: Initiate NFC scan
App->>ANA: configureNfcAnalytics()
ANA->>NM: configure(...)
App->>App: Gather MRZ/CAN and flags
alt iOS
App->>NM: scanDocument(passportNumber, dob, doe, can, useCan, skipPACE, skipCA, extendedMode, usePacePolling)
NM-->>App: result/error (async native)
else Android
App->>NM: scanDocument({ documentNumber, dateOfBirth, dateOfExpiry, canNumber, useCan })
NM-->>App: result/error (async native)
end
App->>ANA: trackNfcEvent(eventName, props)
ANA->>NM: trackEvent(eventName, props)
sequenceDiagram
autonumber
participant Tests as Jest Tests
participant Jest as jest.setup.js
participant NetInfo as @react-native-community/netinfo (mock)
Tests->>Jest: Initialize testing env
Jest->>NetInfo: Define mock (block 1: addEventListener=jest.fn(), fetch=jest.fn())
Jest->>NetInfo: Define mock (block 2 override: addEventListener=jest.fn(()=>jest.fn()), fetch=jest.fn(()=>Promise.resolve({isConnected:true})))
Note over NetInfo: Later mock definition overrides earlier
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (6)
app/jest.setup.js (4)
153-166: Duplicate and conflicting NetInfo mocks; first block lacks unsubscribe and will be shadowed.This block is overridden by the later NetInfo mock (Lines 229-233) and omits returning an unsubscribe function. It also defines useNetInfo here but the later override removes it, causing runtime/test breakage. Consolidate to a single, consistent mock.
Apply this diff to remove the first block:
-// Mock @react-native-community/netinfo -jest.mock('@react-native-community/netinfo', () => ({ - addEventListener: jest.fn(), - useNetInfo: jest.fn().mockReturnValue({ - type: 'wifi', - isConnected: true, - isInternetReachable: true, - details: { - isConnectionExpensive: false, - cellularGeneration: '4g', - }, - }), - fetch: jest.fn(), -}));
208-218: react-native-passport-reader mock exports only default; named exports (reset, scan) are missing.Android path imports { reset, scan } directly; tests will fail. Export both named and default.
Apply this diff:
-jest.mock('react-native-passport-reader', () => ({ - default: { - configure: jest.fn(), - scanPassport: jest.fn(), - readPassport: jest.fn(), - cancelPassportRead: jest.fn(), - trackEvent: jest.fn(), - flush: jest.fn(), - reset: jest.fn(), - }, -})); +jest.mock('react-native-passport-reader', () => { + const named = { + reset: jest.fn(), + // Align with app/src/utils/nfcScanner.ts which imports `scan` as `scanDocument` + scan: jest.fn().mockRejectedValue(new Error('NFC scanning is not supported in Jest')), + }; + return { + __esModule: true, + ...named, + default: { + configure: jest.fn(), + scanPassport: jest.fn(), + readPassport: jest.fn(), + cancelPassportRead: jest.fn(), + trackEvent: jest.fn(), + flush: jest.fn(), + reset: named.reset, + }, + }; +});
222-227: NativeModules.PassportReader is missing scanDocument used by iOS path.scanIOS calls NativeModules.PassportReader.scanDocument; without this mock, tests will crash.
Apply this diff:
NativeModules.PassportReader = { configure: jest.fn(), - scanPassport: jest.fn(), + scanPassport: jest.fn(), + // Used by iOS path in nfcScanner.ts + scanDocument: jest.fn(() => Promise.resolve('{}')), trackEvent: jest.fn(), flush: jest.fn(), };
64-92: Segment client methods should return Promises to match .catch usage.analytics.ts calls .catch on track/screen; current mock returns undefined, causing TypeErrors. Return resolved Promises.
Apply this diff:
jest.mock('@segment/analytics-react-native', () => { const mockClient = { - add: jest.fn(), - track: jest.fn(), - identify: jest.fn(), - screen: jest.fn(), - group: jest.fn(), - alias: jest.fn(), - reset: jest.fn(), + add: jest.fn().mockResolvedValue(undefined), + track: jest.fn().mockResolvedValue(undefined), + identify: jest.fn().mockResolvedValue(undefined), + screen: jest.fn().mockResolvedValue(undefined), + group: jest.fn().mockResolvedValue(undefined), + alias: jest.fn().mockResolvedValue(undefined), + reset: jest.fn().mockResolvedValue(undefined), + flush: jest.fn().mockResolvedValue(undefined), };app/src/utils/analytics.ts (1)
176-194: Event loss risk: queue is shifted before successful send; failures drop events.Drain-then-send without re-queuing on failure can lose events. Shift only after successful send; if flush fails, restore drained events.
Apply this diff:
-const flushMixpanelEvents = () => { +const flushMixpanelEvents = () => { if (!MIXPANEL_NFC_PROJECT_TOKEN) return; try { if (__DEV__) console.log('[Mixpanel] flush'); - // Send any queued events before flushing - while (eventQueue.length > 0) { - const evt = eventQueue.shift()!; - NativeModules.PassportReader?.trackEvent?.(evt.name, evt.properties); - } - NativeModules.PassportReader?.flush?.(); + // Send queued events; only remove after successful send + const pending = [...eventQueue]; + const sent: typeof eventQueue = []; + for (const evt of pending) { + NativeModules.PassportReader?.trackEvent?.(evt.name, evt.properties); + sent.push(evt); + } + // remove sent from queue + eventQueue.splice(0, sent.length); + NativeModules.PassportReader?.flush?.(); eventCount = 0; } catch (err) { if (__DEV__) console.warn('Mixpanel flush failed', err); - // re-queue on failure - if (typeof err !== 'undefined') { - // no-op, events are already queued if failure happened before flush - } + // On failure, ensure events remain queued (no action needed as we only splice on success) } };app/src/utils/nfcScanner.ts (1)
46-52: configureNfcAnalytics may throw synchronously; don’t let scan fail because analytics failed to configure.Wrap analytics configuration to isolate scanning from analytics failures.
Apply this diff:
export const scan = async (inputs: Inputs) => { - configureNfcAnalytics(); + try { + configureNfcAnalytics(); + } catch (e) { + if (__DEV__) console.warn('configureNfcAnalytics failed (non-fatal)', e); + } return Platform.OS === 'android' ? await scanAndroid(inputs) : await scanIOS(inputs); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
app/jest.setup.js(3 hunks)app/src/mocks/react-native-passport-reader.ts(0 hunks)app/src/utils/analytics.ts(4 hunks)app/src/utils/nfcScanner.ts(3 hunks)
💤 Files with no reviewable changes (1)
- app/src/mocks/react-native-passport-reader.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/technical-specification.mdc)
**/*.{ts,tsx}: Define IdentityCommitment with fields: commitment (Poseidon hash), nullifier (domain-separated), timestamp (UTC number), version (circuit version), documentType ('passport' | 'eu_id_card')
Define DSCKeyCommitment with fields: publicKeyHash (Poseidon hash), certificateChain (hashes), revocationStatus (boolean), issuer (country code)
Define VerificationConfig with fields: circuitVersion (semver), complianceRules array, timeWindow (seconds, 24h), clockDrift (±5 min), trustAnchors, revocationRoots, timeSource (NTP), nullifierScope (domain separation)
Files:
app/src/utils/nfcScanner.tsapp/src/utils/analytics.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.tsapp/src/utils/analytics.ts
app/jest.setup.js
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Provide comprehensive Jest setup in app/jest.setup.js with required mocks
Files:
app/jest.setup.js
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/ios/**/*.{swift} : iOS NFC: implement custom PassportReader as a Swift module
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/android/**/*.{kt,kts} : Android NFC: implement RNPassportReaderModule in Kotlin
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to jest.setup.js : Maintain comprehensive mocks in jest.setup.js for all native modules
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
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/ios/**/*.{swift} : iOS NFC: implement custom PassportReader as a Swift module
Applied to files:
app/src/utils/nfcScanner.tsapp/src/utils/analytics.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to native/android/**/*.{kt,kts} : Android NFC: implement RNPassportReaderModule in Kotlin
Applied to files:
app/src/utils/nfcScanner.tsapp/src/utils/analytics.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to jest.setup.js : Maintain comprehensive mocks in jest.setup.js for all native modules
Applied to files:
app/jest.setup.js
📚 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:
app/jest.setup.js
📚 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.config.cjs : Use Jest in the app with a React Native preset configured in app/jest.config.cjs
Applied to files:
app/jest.setup.js
📚 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/src/utils/analytics.ts
🧬 Code graph analysis (2)
app/src/utils/nfcScanner.ts (3)
app/src/utils/analytics.ts (1)
configureNfcAnalytics(197-213)app/android/react-native-passport-reader/index.android.js (1)
NativeModules(5-5)app/ios/PassportReader.swift (1)
scanPassport(78-239)
app/src/utils/analytics.ts (2)
app/tests/__setup__/@env.js (4)
MIXPANEL_NFC_PROJECT_TOKEN(18-18)MIXPANEL_NFC_PROJECT_TOKEN(18-18)ENABLE_DEBUG_LOGS(11-11)ENABLE_DEBUG_LOGS(11-11)app/android/react-native-passport-reader/index.android.js (1)
NativeModules(5-5)
⏰ 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)
- GitHub Check: build-android
- GitHub Check: test
- GitHub Check: build-ios
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
Comment on lines
+229
to
+233
| jest.mock('@react-native-community/netinfo', () => ({ | ||
| addEventListener: jest.fn(() => jest.fn()), | ||
| fetch: jest.fn(() => Promise.resolve({ isConnected: true })), | ||
| })); | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate NetInfo mock and restore useNetInfo; ensure proper return shape and unsubscribe.
Provide one definitive mock that supports both fetch and useNetInfo, and returns an unsubscribe fn from addEventListener.
Apply this diff to replace the block:
-jest.mock('@react-native-community/netinfo', () => ({
- addEventListener: jest.fn(() => jest.fn()),
- fetch: jest.fn(() => Promise.resolve({ isConnected: true })),
-}));
+jest.mock('@react-native-community/netinfo', () => {
+ const state = {
+ type: 'wifi',
+ isConnected: true,
+ isInternetReachable: true,
+ details: { isConnectionExpensive: false, cellularGeneration: '4g' },
+ };
+ return {
+ addEventListener: jest.fn(cb => {
+ if (typeof cb === 'function') cb(state);
+ return jest.fn(); // unsubscribe
+ }),
+ fetch: jest.fn(() => Promise.resolve(state)),
+ useNetInfo: jest.fn().mockReturnValue(state),
+ };
+});📝 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.
Suggested change
| jest.mock('@react-native-community/netinfo', () => ({ | |
| addEventListener: jest.fn(() => jest.fn()), | |
| fetch: jest.fn(() => Promise.resolve({ isConnected: true })), | |
| })); | |
| jest.mock('@react-native-community/netinfo', () => { | |
| const state = { | |
| type: 'wifi', | |
| isConnected: true, | |
| isInternetReachable: true, | |
| details: { isConnectionExpensive: false, cellularGeneration: '4g' }, | |
| }; | |
| return { | |
| addEventListener: jest.fn(cb => { | |
| if (typeof cb === 'function') cb(state); | |
| return jest.fn(); // unsubscribe | |
| }), | |
| fetch: jest.fn(() => Promise.resolve(state)), | |
| useNetInfo: jest.fn().mockReturnValue(state), | |
| }; | |
| }); |
Comment on lines
198
to
211
| if (!MIXPANEL_NFC_PROJECT_TOKEN || mixpanelConfigured) return; | ||
| const enableDebugLogs = JSON.parse(String(ENABLE_DEBUG_LOGS)); | ||
| if (PassportReader.configure) { | ||
| await Promise.resolve( | ||
| PassportReader.configure(MIXPANEL_NFC_PROJECT_TOKEN, enableDebugLogs, { | ||
| flushInterval: 20, | ||
| flushCount: 5, | ||
| flushOnBackground: true, | ||
| flushOnForeground: true, | ||
| flushOnNetworkChange: true, | ||
| }), | ||
| ); | ||
| } | ||
| NativeModules.PassportReader.configure( | ||
| MIXPANEL_NFC_PROJECT_TOKEN, | ||
| enableDebugLogs, | ||
| { | ||
| flushInterval: 20, | ||
| flushCount: 5, | ||
| flushOnBackground: true, | ||
| flushOnForeground: true, | ||
| flushOnNetworkChange: true, | ||
| }, | ||
| ); | ||
| setupFlushPolicies(); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard native call to avoid crashes on platforms/environments without PassportReader.
Unconditional NativeModules.PassportReader.configure can throw on web or when the module isn’t loaded.
Apply this diff:
export const configureNfcAnalytics = () => {
if (!MIXPANEL_NFC_PROJECT_TOKEN || mixpanelConfigured) return;
const enableDebugLogs = JSON.parse(String(ENABLE_DEBUG_LOGS));
- NativeModules.PassportReader.configure(
+ if (!NativeModules?.PassportReader?.configure) {
+ if (__DEV__) console.warn('[Mixpanel] PassportReader.configure not available');
+ return;
+ }
+ NativeModules.PassportReader.configure(
MIXPANEL_NFC_PROJECT_TOKEN,
enableDebugLogs,
{
flushInterval: 20,
flushCount: 5,
flushOnBackground: true,
flushOnForeground: true,
flushOnNetworkChange: true,
},
);📝 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.
Suggested change
| if (!MIXPANEL_NFC_PROJECT_TOKEN || mixpanelConfigured) return; | |
| const enableDebugLogs = JSON.parse(String(ENABLE_DEBUG_LOGS)); | |
| if (PassportReader.configure) { | |
| await Promise.resolve( | |
| PassportReader.configure(MIXPANEL_NFC_PROJECT_TOKEN, enableDebugLogs, { | |
| flushInterval: 20, | |
| flushCount: 5, | |
| flushOnBackground: true, | |
| flushOnForeground: true, | |
| flushOnNetworkChange: true, | |
| }), | |
| ); | |
| } | |
| NativeModules.PassportReader.configure( | |
| MIXPANEL_NFC_PROJECT_TOKEN, | |
| enableDebugLogs, | |
| { | |
| flushInterval: 20, | |
| flushCount: 5, | |
| flushOnBackground: true, | |
| flushOnForeground: true, | |
| flushOnNetworkChange: true, | |
| }, | |
| ); | |
| setupFlushPolicies(); | |
| export const configureNfcAnalytics = () => { | |
| if (!MIXPANEL_NFC_PROJECT_TOKEN || mixpanelConfigured) return; | |
| const enableDebugLogs = JSON.parse(String(ENABLE_DEBUG_LOGS)); | |
| if (!NativeModules?.PassportReader?.configure) { | |
| if (__DEV__) console.warn('[Mixpanel] PassportReader.configure not available'); | |
| return; | |
| } | |
| NativeModules.PassportReader.configure( | |
| MIXPANEL_NFC_PROJECT_TOKEN, | |
| enableDebugLogs, | |
| { | |
| flushInterval: 20, | |
| flushCount: 5, | |
| flushOnBackground: true, | |
| flushOnForeground: true, | |
| flushOnNetworkChange: true, | |
| }, | |
| ); | |
| setupFlushPolicies(); | |
| }; |
🤖 Prompt for AI Agents
In app/src/utils/analytics.ts around lines 198 to 211, the call
NativeModules.PassportReader.configure is unguarded and can throw on web or when
the native module isn't loaded; wrap the configure invocation with a guard that
checks NativeModules and NativeModules.PassportReader exist and that
PassportReader.configure is a function (and optionally wrap the call in
try/catch), and only call setupFlushPolicies() when the native configure
succeeded or when the guard passes; this prevents runtime crashes on unsupported
platforms.
Comment on lines
+228
to
249
| export const trackNfcEvent = ( | ||
| name: string, | ||
| properties?: Record<string, unknown>, | ||
| ) => { | ||
| if (!MIXPANEL_NFC_PROJECT_TOKEN) return; | ||
| if (!mixpanelConfigured) await configureNfcAnalytics(); | ||
| if (!mixpanelConfigured) configureNfcAnalytics(); | ||
|
|
||
| if (!isConnected) { | ||
| eventQueue.push({ name, properties }); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| if (PassportReader.trackEvent) { | ||
| await Promise.resolve(PassportReader.trackEvent(name, properties)); | ||
| } | ||
| NativeModules.PassportReader?.trackEvent?.(name, properties); | ||
| eventCount++; | ||
| if (eventCount >= 5) { | ||
| flushMixpanelEvents().catch(console.warn); | ||
| flushMixpanelEvents(); | ||
| } | ||
| } catch { | ||
| } catch (err) { | ||
| eventQueue.push({ name, properties }); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Avoid incrementing eventCount when trackEvent is unavailable; otherwise flush triggers with no events.
Optional chaining can skip the call silently, but eventCount still increments and flush resets, losing events.
Apply this diff:
try {
- NativeModules.PassportReader?.trackEvent?.(name, properties);
- eventCount++;
+ if (NativeModules.PassportReader?.trackEvent) {
+ NativeModules.PassportReader.trackEvent(name, properties);
+ eventCount++;
+ } else {
+ // Module not available; queue for later
+ eventQueue.push({ name, properties });
+ return;
+ }
if (eventCount >= 5) {
flushMixpanelEvents();
}
} catch (err) {
eventQueue.push({ name, properties });
}📝 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.
Suggested change
| export const trackNfcEvent = ( | |
| name: string, | |
| properties?: Record<string, unknown>, | |
| ) => { | |
| if (!MIXPANEL_NFC_PROJECT_TOKEN) return; | |
| if (!mixpanelConfigured) await configureNfcAnalytics(); | |
| if (!mixpanelConfigured) configureNfcAnalytics(); | |
| if (!isConnected) { | |
| eventQueue.push({ name, properties }); | |
| return; | |
| } | |
| try { | |
| if (PassportReader.trackEvent) { | |
| await Promise.resolve(PassportReader.trackEvent(name, properties)); | |
| } | |
| NativeModules.PassportReader?.trackEvent?.(name, properties); | |
| eventCount++; | |
| if (eventCount >= 5) { | |
| flushMixpanelEvents().catch(console.warn); | |
| flushMixpanelEvents(); | |
| } | |
| } catch { | |
| } catch (err) { | |
| eventQueue.push({ name, properties }); | |
| } | |
| }; | |
| export const trackNfcEvent = ( | |
| name: string, | |
| properties?: Record<string, unknown>, | |
| ) => { | |
| if (!MIXPANEL_NFC_PROJECT_TOKEN) return; | |
| if (!mixpanelConfigured) configureNfcAnalytics(); | |
| if (!isConnected) { | |
| eventQueue.push({ name, properties }); | |
| return; | |
| } | |
| try { | |
| if (NativeModules.PassportReader?.trackEvent) { | |
| NativeModules.PassportReader.trackEvent(name, properties); | |
| eventCount++; | |
| } else { | |
| // Module not available; queue for later | |
| eventQueue.push({ name, properties }); | |
| return; | |
| } | |
| if (eventCount >= 5) { | |
| flushMixpanelEvents(); | |
| } | |
| } catch (err) { | |
| eventQueue.push({ name, properties }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In app/src/utils/analytics.ts around lines 228 to 249, the code increments
eventCount even when NativeModules.PassportReader?.trackEvent is undefined due
to optional chaining, which can trigger flushes with no events; change the logic
so you first check that NativeModules.PassportReader?.trackEvent exists (or
store it in a local variable) and only call it and increment eventCount when it
was actually invoked, otherwise push the event to eventQueue (and do not
increment); preserve the try/catch for runtime errors and keep the existing
flush check (eventCount >= 5) only inside the branch where the event was
successfully sent.
Comment on lines
65
to
76
| const scanIOS = async (inputs: Inputs) => { | ||
| return await Promise.resolve( | ||
| PassportReader.scan({ | ||
| documentNumber: inputs.passportNumber, | ||
| dateOfBirth: inputs.dateOfBirth, | ||
| dateOfExpiry: inputs.dateOfExpiry, | ||
| canNumber: inputs.canNumber ?? '', | ||
| useCan: inputs.useCan ?? false, | ||
| }), | ||
| return await NativeModules.PassportReader.scanDocument( | ||
| inputs.passportNumber, | ||
| inputs.dateOfBirth, | ||
| inputs.dateOfExpiry, | ||
| inputs.canNumber ?? '', | ||
| inputs.useCan ?? false, | ||
| inputs.skipPACE ?? false, | ||
| inputs.skipCA ?? false, | ||
| inputs.extendedMode ?? false, | ||
| inputs.usePacePolling ?? false, | ||
| ); |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify iOS native API exposure and signature.
Confirm whether the iOS module exports scanDocument(...) with the 9-arg signature or only scanPassport(...). Adjust mocks/native code accordingly.
🏁 Script executed:
#!/bin/bash
# Verify exported selectors in iOS sources
rg -nP -C2 '(?i)@objc\([^)]*scan(Document|Passport)' app/ios || true
rg -nP -C2 '\bfunc\s+scan(Document|Passport)\b' app/ios || true
# Also check JS bridge shims (if any)
rg -nP -C2 'NativeModules\.PassportReader\.(scanDocument|scanPassport)' app -g '!**/node_modules/**'Length of output: 1972
Align JS call with iOS export: use scanPassport instead of scanDocument.
In app/src/utils/nfcScanner.ts (at the NativeModules.PassportReader.scanDocument call), replace .scanDocument(...) with .scanPassport(...) (matching the 9-arg signature exposed by the iOS module), or update the iOS bridge to export a scanDocument selector.
🤖 Prompt for AI Agents
In app/src/utils/nfcScanner.ts around lines 65 to 76, the JS call is invoking
NativeModules.PassportReader.scanDocument but the iOS bridge exports
scanPassport with a 9-argument signature; update the JS call to use
NativeModules.PassportReader.scanPassport(...) with the same arguments currently
passed (passportNumber, dateOfBirth, dateOfExpiry, canNumber ?? '', useCan ??
false, skipPACE ?? false, skipCA ?? false, extendedMode ?? false, usePacePolling
?? false). Alternatively, if you prefer to keep the JS name, modify the iOS
bridge to export a scanDocument selector that delegates to the existing
scanPassport implementation with the matching 9-arg signature.
iOS bridge method mismatch risk: scanDocument vs scanPassport; add fallback and explicit error.
The iOS native module may expose scanPassport(...) instead of scanDocument(...). Add a compatibility fallback to avoid runtime crashes.
Apply this diff:
-const scanIOS = async (inputs: Inputs) => {
- return await NativeModules.PassportReader.scanDocument(
- inputs.passportNumber,
- inputs.dateOfBirth,
- inputs.dateOfExpiry,
- inputs.canNumber ?? '',
- inputs.useCan ?? false,
- inputs.skipPACE ?? false,
- inputs.skipCA ?? false,
- inputs.extendedMode ?? false,
- inputs.usePacePolling ?? false,
- );
-};
+const scanIOS = async (inputs: Inputs) => {
+ const mod = NativeModules.PassportReader;
+ if (typeof mod?.scanDocument === 'function') {
+ return await mod.scanDocument(
+ inputs.passportNumber,
+ inputs.dateOfBirth,
+ inputs.dateOfExpiry,
+ inputs.canNumber ?? '',
+ inputs.useCan ?? false,
+ inputs.skipPACE ?? false,
+ inputs.skipCA ?? false,
+ inputs.extendedMode ?? false,
+ inputs.usePacePolling ?? false,
+ );
+ }
+ if (typeof mod?.scanPassport === 'function') {
+ // Legacy signature: 3-arg variant
+ return await mod.scanPassport(
+ inputs.passportNumber,
+ inputs.dateOfBirth,
+ inputs.dateOfExpiry,
+ );
+ }
+ throw new Error('PassportReader.scanDocument/scanPassport not available on iOS');
+};📝 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.
Suggested change
| const scanIOS = async (inputs: Inputs) => { | |
| return await Promise.resolve( | |
| PassportReader.scan({ | |
| documentNumber: inputs.passportNumber, | |
| dateOfBirth: inputs.dateOfBirth, | |
| dateOfExpiry: inputs.dateOfExpiry, | |
| canNumber: inputs.canNumber ?? '', | |
| useCan: inputs.useCan ?? false, | |
| }), | |
| return await NativeModules.PassportReader.scanDocument( | |
| inputs.passportNumber, | |
| inputs.dateOfBirth, | |
| inputs.dateOfExpiry, | |
| inputs.canNumber ?? '', | |
| inputs.useCan ?? false, | |
| inputs.skipPACE ?? false, | |
| inputs.skipCA ?? false, | |
| inputs.extendedMode ?? false, | |
| inputs.usePacePolling ?? false, | |
| ); | |
| const scanIOS = async (inputs: Inputs) => { | |
| const mod = NativeModules.PassportReader; | |
| if (typeof mod?.scanDocument === 'function') { | |
| return await mod.scanDocument( | |
| inputs.passportNumber, | |
| inputs.dateOfBirth, | |
| inputs.dateOfExpiry, | |
| inputs.canNumber ?? '', | |
| inputs.useCan ?? false, | |
| inputs.skipPACE ?? false, | |
| inputs.skipCA ?? false, | |
| inputs.extendedMode ?? false, | |
| inputs.usePacePolling ?? false, | |
| ); | |
| } | |
| if (typeof mod?.scanPassport === 'function') { | |
| // Legacy signature: 3-arg variant | |
| return await mod.scanPassport( | |
| inputs.passportNumber, | |
| inputs.dateOfBirth, | |
| inputs.dateOfExpiry, | |
| ); | |
| } | |
| throw new Error('PassportReader.scanDocument/scanPassport not available on iOS'); | |
| }; |
🤖 Prompt for AI Agents
In app/src/utils/nfcScanner.ts around lines 65 to 76, the code directly calls
NativeModules.PassportReader.scanDocument but the native iOS bridge may instead
expose scanPassport; update the implementation to check for the presence of
scanDocument and call it if available, otherwise check for and call scanPassport
with the same argument order, and if neither exists throw a clear, explicit
error describing the missing native method so the app fails fast and logs the
compatibility issue.
Member
Author
|
not worth merging. easier to fix |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduces an error in scanning. Reverting for now since so many of us are building on
devReverts #971
Summary by CodeRabbit