feat: scaffold mrz, nfc, and qr feature directories #893
Conversation
WalkthroughAdds new public modules for NFC, MRZ, and QR with scan stubs and explicit exports. Reorganizes top-level and browser barrels to re-export from ./nfc, ./mrz, ./qr, and adds mergeConfig. Updates documentation to reflect modular capability directories and explicit named exports. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant SDK as mobile-sdk-alpha (nfc/mrz/qr)
participant Processing as processing/*
App->>SDK: scanNFC(opts)
SDK->>Processing: parseNFCResponse(...)
Processing-->>SDK: ParsedNFCResponse
SDK-->>App: ScanResult (stub)
App->>SDK: scanMRZ(opts)
SDK->>Processing: extractMRZInfo / formatDateToYYMMDD
Processing-->>SDK: MRZ data
SDK-->>App: ScanResult (stub)
App->>SDK: scanQRProof(opts)
SDK-->>App: ScanResult (stub)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/mobile-sdk-alpha/docs/ARCHITECTURE_PROMPTS.md (1)
25-26: Add a brief example to cement the guidanceA short example makes the “explicit named exports” directive actionable and consistent across contributors.
You can append an example block after the list:
// src/index.ts export { scanMRZ } from './mrz'; export { scanNFC } from './nfc'; export { scanQRProof } from './qr'; // type-only re-exports to preserve tree-shaking export type { MRZScanOptions } from './mrz'; export type { NFCScanOptions } from './nfc'; export type { QRProofOptions } from './qr'; // Consumer usage import { scanMRZ, scanNFC, scanQRProof } from '@selfxyz/mobile-sdk-alpha';packages/mobile-sdk-alpha/docs/MIGRATION_PROMPTS.md (1)
10-10: Strengthen export guidance for tree-shakingExplicitly discourage
export *and suggestexport typefor types to prevent bundlers from pulling values.Apply this diff:
-- Group new capabilities in their own directories (e.g., `processing/`, `validation/`, `mrz/`, `nfc/`, `qr/`) and re-export them from `src/index.ts` using explicit named exports. +- Group new capabilities in their own directories (e.g., `processing/`, `validation/`, `mrz/`, `nfc/`, `qr/`) and re-export them from `src/index.ts` using explicit named exports (avoid `export *`). Use `export type { ... }` for type-only re-exports.packages/mobile-sdk-alpha/src/qr/index.ts (1)
7-9: Prefer a clearer, consistent not-implemented error and add TSDocImprove debuggability with a specific message and TSDoc. Longer term, unify on a typed error (e.g., FeatureUnavailableError) across MRZ/NFC/QR.
Apply this diff to clarify the error:
-export async function scanQRProof(_opts: QRProofOptions): Promise<ScanResult> { - throw new Error('Not implemented'); -} +export async function scanQRProof(_opts: QRProofOptions): Promise<ScanResult> { + throw new Error('scanQRProof is not implemented yet in @selfxyz/mobile-sdk-alpha'); +}Optionally add TSDoc (outside the selected range) for API clarity:
/** * Scans and parses a QR proof payload. * @throws Error with message "scanQRProof is not implemented yet in @selfxyz/mobile-sdk-alpha" * until the capability is delivered. * @public */ export async function scanQRProof(_opts: QRProofOptions): Promise<ScanResult> { /* ... */ }Also consider introducing a shared typed error in
src/errors/(e.g.,FeatureUnavailableError) and reuse it acrossmrz,nfc, andqrstubs for consistency and easier client-side handling.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/mobile-sdk-alpha/docs/ARCHITECTURE.md(1 hunks)packages/mobile-sdk-alpha/docs/ARCHITECTURE_PROMPTS.md(1 hunks)packages/mobile-sdk-alpha/docs/MIGRATION_CHECKLIST.md(1 hunks)packages/mobile-sdk-alpha/docs/MIGRATION_PROMPTS.md(1 hunks)packages/mobile-sdk-alpha/src/browser.ts(1 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)packages/mobile-sdk-alpha/src/mrz/index.ts(1 hunks)packages/mobile-sdk-alpha/src/nfc/index.ts(1 hunks)packages/mobile-sdk-alpha/src/qr/index.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/mobile-sdk-alpha/docs/ARCHITECTURE.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...plicit named exports (avoid export *). 2. Bridge layer for native events – wrap ...
(QB_NEW_EN)
🪛 Biome (2.1.2)
packages/mobile-sdk-alpha/src/qr/index.ts
[error] 3-5: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
packages/mobile-sdk-alpha/src/nfc/index.ts
[error] 5-7: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
packages/mobile-sdk-alpha/src/mrz/index.ts
[error] 3-5: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (5)
packages/mobile-sdk-alpha/docs/MIGRATION_CHECKLIST.md (1)
5-6: No change needed — entry barrels already use explicit named exports and type-only re-exportsVerified packages/mobile-sdk-alpha/src/index.ts and packages/mobile-sdk-alpha/src/browser.ts:
- Both files use
export type { ... } from './types/public'for types.- MRZ/NFC/QR are re-exported with explicit named exports (e.g.
export { type ..., scanMRZ } from './mrz',export { type ..., scanNFC } from './nfc',export { type ..., scanQRProof } from './qr').mergeConfigis explicitly exported (export { mergeConfig } from './config/merge').- No
export * frompatterns were found.Conclusion: the proposed diff is unnecessary — the barrels already follow the recommended tree-shaking-friendly pattern. Leave as-is.
packages/mobile-sdk-alpha/docs/ARCHITECTURE.md (1)
11-11: LGTM: Clear guidance toward modular, tree-shakable exportsCalling out explicit named exports and avoiding
export *here aligns with best practices and the PR’s reorganization.packages/mobile-sdk-alpha/src/qr/index.ts (1)
1-1: Good: type-only import for ScanResultUsing
import typeensures no runtime footprint and keeps the bundle lean.packages/mobile-sdk-alpha/src/index.ts (1)
35-37: LGTM: modularized re-exports with type-only specifiersThe re-exports from ./nfc, ./mrz, ./qr, and ./config/merge cleanly expose public surfaces and the
typespecifiers avoid runtime emissions. This improves tree-shaking and clarifies the intended import paths for consumers.Also applies to: 41-41
packages/mobile-sdk-alpha/src/browser.ts (1)
37-39: LGTM: browser barrel mirrors the modular export surfaceThe browser-focused barrel re-exports align with the new modular structure and use type-only specifiers for better treeshaking. Keeping browser and universal entrypoints in sync mitigates drift.
Also applies to: 44-44
|
fyi @shazarre @aaronmgdr first pr that updates our sdk structure based on the architecture checklist |
Summary
Testing
yarn workspaces foreach -A -p -v --topological-dev --since=HEAD run nice --if-present(fails: Invalid option schema)yarn workspace @selfxyz/mobile-sdk-alpha niceyarn workspace @selfxyz/mobile-sdk-alpha types --listFilesyarn workspace @selfxyz/mobile-sdk-alpha testyarn workspace @selfxyz/mobile-sdk-alpha buildyarn workspace @selfxyz/mobile-sdk-alpha validate:exportsyarn workspace @selfxyz/mobile-sdk-alpha validate:pkgyarn lintyarn buildyarn workspace @selfxyz/contracts build(fails: Invalid account for network)yarn typeshttps://chatgpt.com/codex/tasks/task_b_689d171ebc54832d8caa061115a41d47
Summary by CodeRabbit