-
Notifications
You must be signed in to change notification settings - Fork 181
setup for us to have svgs in the mobile sdk #1230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughApp components switched icon and asset imports from local files to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{js,ts,tsx,jsx,sol,nr}📄 CodeRabbit inference engine (.cursorrules)
Files:
app/**/*.{ts,tsx}📄 CodeRabbit inference engine (app/AGENTS.md)
Files:
app/src/**/*.{ts,tsx,js,jsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-26T14:49:11.190ZApplied to files:
⏰ 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)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
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/document/IDPickerScreen.tsx (1)
5-5: Fix import sorting to pass CI.The pipeline reports an import sorting violation. Run the autofix command to resolve this before merging.
#!/bin/bash # Run the lint fix to auto-sort imports yarn lint:fix
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/mobile-sdk-alpha/svgs/icons/aadhaar.svgis excluded by!**/*.svgpackages/mobile-sdk-alpha/svgs/icons/epassport.svgis excluded by!**/*.svgpackages/mobile-sdk-alpha/svgs/icons/epassport_rounded.svgis excluded by!**/*.svgpackages/mobile-sdk-alpha/svgs/icons/plus.svgis excluded by!**/*.svgpackages/mobile-sdk-alpha/svgs/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
app/src/components/homeScreen/idCard.tsx(1 hunks)app/src/screens/document/IDPickerScreen.tsx(1 hunks)packages/mobile-sdk-alpha/package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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
**/*.{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/components/homeScreen/idCard.tsxapp/src/screens/document/IDPickerScreen.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/components/homeScreen/idCard.tsxapp/src/screens/document/IDPickerScreen.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/homeScreen/idCard.tsxapp/src/screens/document/IDPickerScreen.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Update the app to consume the mobile-sdk-alpha package and replace local modules with SDK imports
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
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
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)
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.
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
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
📚 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/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
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
🪛 GitHub Actions: Workspace CI
app/src/screens/document/IDPickerScreen.tsx
[error] 5-5: Run autofix to sort these imports! simple-import-sort/imports
⏰ 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: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (4)
app/src/components/homeScreen/idCard.tsx (1)
16-17: LGTM! Icon imports migrated to SDK package.The import paths correctly reference the SVG exports added in the SDK package. The usage remains unchanged, which is the expected pattern for this migration.
app/src/screens/document/IDPickerScreen.tsx (2)
16-19: LGTM! SVG assets migrated to SDK package.All icon imports correctly reference the SDK's exported SVG paths, aligning with the migration strategy.
26-26: LGTM! Switched to production safe area context.Good change moving from the mock implementation to the actual
react-native-safe-area-contextlibrary. This provides proper safe area insets handling.Based on learnings
packages/mobile-sdk-alpha/package.json (1)
85-85: LGTM! SVGs directory added to published files.Including "svgs" in the files array ensures the SVG assets will be published to npm and available to consumers.
As per coding guidelines
| "./svgs/*.svg": { | ||
| "react-native": "./svgs/*.svg", | ||
| "import": "./svgs/*.svg", | ||
| "require": "./svgs/*.svg" | ||
| }, | ||
| "./svgs/icons/*.svg": { | ||
| "react-native": "./svgs/icons/*.svg", | ||
| "import": "./svgs/icons/*.svg", | ||
| "require": "./svgs/icons/*.svg" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify SVG directory structure matches export patterns.
The wildcard export patterns look correct, but ensure the svgs/ and svgs/icons/ directories exist with the expected SVG files before publishing.
Run this script to verify the SVG directory structure:
🏁 Script executed:
#!/bin/bash
# Verify svgs directory structure and list available SVG files
echo "=== Checking svgs directory structure ==="
fd -e svg . svgs/
echo -e "\n=== Verifying specific SVG files used in the app ==="
files=(
"svgs/icons/aadhaar.svg"
"svgs/icons/epassport.svg"
"svgs/icons/epassport_rounded.svg"
"svgs/icons/plus.svg"
"svgs/logo.svg"
)
for file in "${files[@]}"; do
if [ -f "$file" ]; then
echo "✓ $file exists"
else
echo "✗ $file MISSING"
fi
doneLength of output: 353
Fix missing svgs directory or remove exports. The svgs/ folder isn’t present, so the wildcard export entries in package.json will fail. Add the directory with the SVG assets or remove those export conditions.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/package.json around lines 66 to 75, the package
exports reference a non-existent svgs/ directory which will break consumers;
either add the missing svgs/ folder with the referenced SVG files (and keep the
export map entries) or remove the "./svgs/*.svg" and "./svgs/icons/*.svg" export
entries from package.json so exports only point to existing files; update git to
include the new directory and assets if adding, or commit the package.json edit
removing those export conditions and run a quick npm pack/local install to
verify the package resolves correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am begging you to consider build artifacts before making exaggerated claims
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Current directory ==="
pwd
echo -e "\n=== Checking for mobile-sdk-alpha package ==="
if [ -d "packages/mobile-sdk-alpha" ]; then
cd packages/mobile-sdk-alpha
echo "Changed to: $(pwd)"
else
echo "ERROR: packages/mobile-sdk-alpha directory not found"
exit 1
fi
echo -e "\n=== Listing top-level directories ==="
ls -la
echo -e "\n=== Checking for svgs directory ==="
if [ -d "svgs" ]; then
echo "✓ svgs directory exists"
echo -e "\n=== Contents of svgs/ ==="
find svgs -type f -name "*.svg" | head -20
echo -e "\n=== Total SVG files ==="
find svgs -type f -name "*.svg" | wc -l
else
echo "✗ svgs directory not found"
fi
echo -e "\n=== Checking build scripts for SVG handling ==="
if [ -f "package.json" ]; then
cat package.json | jq -r '.scripts | to_entries[] | select(.value | contains("svg") or contains("svgs")) | "\(.key): \(.value)"'
fi
echo -e "\n=== Checking for SVG files in the entire package ==="
fd -e svg . | head -20
Length of output: 2052
@aaronmgdr You're absolutely right, and I apologize. The svgs directory exists with all the required SVG files (aadhaar.svg, epassport.svg, epassport_rounded.svg, plus.svg, and logo.svg). My initial verification was flawed. The export configuration in package.json is correct and ready to go.
✏️ Learnings added
Learnt from: aaronmgdr
PR: selfxyz/self#1230
File: packages/mobile-sdk-alpha/package.json:66-75
Timestamp: 2025-10-07T14:25:57.485Z
Learning: Always verify directory existence from the correct package root before flagging missing directories as critical issues, especially when the package.json "files" array includes them.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
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
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
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
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)
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
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)
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/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
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 : Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
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)
if we are moving over components we will need these
Summary by CodeRabbit