feat: Privy Passkeys #1511
feat: Privy Passkeys #1511
Conversation
WalkthroughThis pull request introduces comprehensive changes across the application's onboarding and configuration processes. The modifications primarily focus on restructuring the onboarding flow, updating workflow configurations, and enhancing authentication mechanisms. Key changes include replacing user profile screens with a new contact card screen, updating navigation logic, modifying environment configurations, and introducing new components for a more streamlined user experience. Changes
Sequence DiagramsequenceDiagram
participant User
participant WelcomeScreen
participant ContactCardScreen
participant AuthService
User->>WelcomeScreen: Starts onboarding
WelcomeScreen->>AuthService: Authenticate with Passkey
AuthService-->>WelcomeScreen: Authentication successful
WelcomeScreen->>ContactCardScreen: Navigate
ContactCardScreen->>User: Prompt to create profile
User->>ContactCardScreen: Enter profile details
ContactCardScreen->>AuthService: Save profile
AuthService-->>ContactCardScreen: Profile saved
ContactCardScreen->>User: Onboarding complete
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (16)
components/TransactionPreview/TransactionPreview.tsx (1)
Line range hint
177-189: Improve error handling type safety.The error handling uses string concatenation and type 'any'. This could be improved for better type safety and error reporting.
Consider this improvement:
- } catch (e: any) { - if (`${e}`.includes("User rejected the request")) { + } catch (error: unknown) { + if (error instanceof Error && error.message.includes("User rejected the request")) { setTxState({ status: "pending" }); } else { - logger.error(e); - let txError = e?.message; - if (!txError) { - try { - txError = JSON.stringify(e); - } catch {} - } - if (!txError) txError = e; - setTxState({ status: "failure", error: `${txError}` }); + logger.error(error); + const errorMessage = error instanceof Error + ? error.message + : typeof error === 'object' + ? JSON.stringify(error) + : String(error); + setTxState({ status: "failure", error: errorMessage }); }features/onboarding/Privy/usePrivySmartWalletConnection.ts (1)
46-54: Handle embedded wallet creation errors appropriatelyThere's a TODO comment on line 52 indicating that error handling needs to be improved for
embeddedWallet.create(). Consider implementing better error handling to provide users with meaningful feedback.Do you want me to generate error handling code or open a GitHub issue to track this task?
utils/xmtpRN/signer.ts (1)
23-38: Consider making wallet type configurable.The implementation looks good, but hardcoding
walletType: () => "SCW"might be too restrictive. Consider making it configurable through a parameter, similar to theethersSignerToXmtpSignerfunction.-export const viemAccountToXmtpSigner = (account: ViemAccount): XmtpSigner => ({ +export const viemAccountToXmtpSigner = ( + account: ViemAccount, + isSCW?: boolean +): XmtpSigner => ({ getAddress: () => Promise.resolve(account.address), getChainId: () => account.chainId, getBlockNumber: () => undefined, - walletType: () => "SCW", + walletType: () => (isSCW ? "SCW" : "EOA"), signMessage: async (message: string) => {features/onboarding/passkey/useCreatePasskey.ts (2)
52-54: Consider using typed errors.Instead of using
anytype for the error, consider creating a custom error type or usingunknownfor better type safety.- } catch (e: any) { - setError(e?.message ?? "Error creating Passkey account"); + } catch (e: unknown) { + const errorMessage = e instanceof Error ? e.message : "Error creating Passkey account"; + setError(errorMessage);
50-51: Consider extracting status strings as constants.Status strings like "Account created - Waiting for smart wallet" should be extracted into constants to maintain consistency and ease updates.
features/onboarding/passkey/useLoginWithPasskey.ts (1)
36-57: Consider extracting common logic.The implementation is solid, but there's significant code duplication with
useCreatePasskey. Consider extracting common patterns into shared utilities:
- Error handling logic
- Loading state management
- User session handling
Example shared utility:
type PasskeyOperation = { operation: () => Promise<{ user: { id: string } | null }>; errorMessage: string; successMessage: string; }; const executePasskeyOperation = async ({ operation, errorMessage, successMessage, setLoading, setError, setStatusString, setPrivyAccountId, logout, privyUser }: PasskeyOperation & Dependencies) => { try { setLoading(true); if (privyUser) { await logout(); } const { user } = await operation(); if (!user) { throw new Error("No account operation completed"); } setStatusString(successMessage); setPrivyAccountId(user.id); } catch (e: unknown) { const error = e instanceof Error ? e.message : errorMessage; setError(error); captureErrorWithToast(e); } finally { setLoading(false); } };Also applies to: 67-70
features/onboarding/passkey/passkeyAuthStore.tsx (1)
4-9: Consider making essential props requiredThe store props are all marked as optional, which could lead to runtime issues if not properly handled. Consider making essential props like
loadingrequired to enforce proper initialization.type IPasskeyAuthStoreProps = { - loading?: boolean; + loading: boolean; error?: string; statusString?: string; account?: string; };navigation/OnboardingNavigator.tsx (1)
53-56: Consider adding screen options for consistencyThe new OnboardingPasskey screen is missing screen options that are present in other screens (e.g., headerShown). Consider adding consistent screen options.
<OnboardingNativeStack.Screen + options={{ + headerShown: false, + }} name="OnboardingPasskey" component={OnboardingPasskeyScreen} />utils/xmtpRN/signIn.ts (2)
74-121: Consider refactoring to reduce code duplication.The
createXmtpClientFromViemAccountfunction is almost identical tocreateXmtpClientFromSigner. Consider extracting the common logic into a shared helper function to improve maintainability.+async function createXmtpClientCommon( + options: any, + inboxId: string, + tempDirectory: string, + createClient: () => Promise<any>, + onInstallationRevoked: () => Promise<void> +) { + await copyDatabasesToTemporaryDirectory(tempDirectory, inboxId); + const client = await createClient(); + if (client.inboxId !== inboxId) { + throw new Error("Inbox ids don't match"); + } + const installationValid = await isClientInstallationValid(client); + if (!installationValid) { + await client.dropLocalDatabaseConnection(); + await deleteLibXmtpDatabaseForInboxId(inboxId); + onInstallationRevoked(); + return; + } + await client.dropLocalDatabaseConnection(); + await moveTemporaryDatabasesToDatabaseDirectory(tempDirectory, client.inboxId); +} export const createXmtpClientFromViemAccount = async ( account: ViemAccount, onInstallationRevoked: () => Promise<void>, preAuthenticateToInboxCallback?: () => Promise<void> ) => { const tempDirectory = await createTemporaryDirectory(); const dbEncryptionKey = await getDbEncryptionKey(); const options = { env, enableV3: true, dbDirectory: tempDirectory, dbEncryptionKey, }; const inboxId = await getInboxId(await account.address); - await copyDatabasesToTemporaryDirectory(tempDirectory, inboxId); - logger.debug("Instantiating client from signer"); - const client = await Client.create(viemAccountToXmtpSigner(account), { - ...options, - preAuthenticateToInboxCallback, - }); - if (client.inboxId !== inboxId) { - throw new Error("Inbox ids don't match"); - } - const installationValid = await isClientInstallationValid(client); - if (!installationValid) { - await client.dropLocalDatabaseConnection(); - await deleteLibXmtpDatabaseForInboxId(inboxId); - onInstallationRevoked(); - return; - } - await client.dropLocalDatabaseConnection(); - await moveTemporaryDatabasesToDatabaseDirectory(tempDirectory, client.inboxId); + await createXmtpClientCommon( + options, + inboxId, + tempDirectory, + () => Client.create(viemAccountToXmtpSigner(account), { + ...options, + preAuthenticateToInboxCallback, + }), + onInstallationRevoked + ); };
91-91: Update log message to match the account type.The log message indicates "Instantiating client from signer" but we're using a Viem account.
- logger.debug("Instantiating client from signer"); + logger.debug("Instantiating client from Viem account");screens/NewAccount/new-account-passkey-screen.tsx (3)
111-114: Use i18n for the login button text.The login button text is hardcoded. For consistency and internationalization support, use the translation system.
- text={"Login with passkey"} + text={translate("passkey.loginButton")}
98-103: Enhance account display format.The account address display could be improved by truncating the address and adding proper spacing.
- <Text preset="bodyBold"> - Account created: - <Text preset="body">{account}</Text> + <Text preset="bodyBold" style={{ marginBottom: 10 }}> + Account created:{" "} + <Text preset="body"> + {account.slice(0, 6)}...{account.slice(-4)} + </Text> </Text>
56-63: Improve error handling specificity.The error handling could be more specific to help users understand and resolve issues.
const handleError = useCallback( (error: Error) => { logger.error(error); - setError(error.message); + if (error.message.includes("not supported")) { + setError(translate("passkey.browser_not_supported")); + } else if (error.message.includes("cancelled")) { + setError(translate("passkey.operation_cancelled")); + } else { + setError(translate("passkey.generic_error")); + } captureErrorWithToast(error); }, [setError] );components/Onboarding/init-xmtp-client.ts (1)
63-64: Improve error message specificity.The error message could be more specific about the missing Viem account.
- throw new Error("No signer"); + throw new Error("No Viem account provided for XMTP client initialization");App.tsx (1)
154-154: Extract inline style to StyleSheet.Move the inline style
{ flex: 1 }to the StyleSheet for better maintainability.-<GestureHandlerRootView style={{ flex: 1 }}> +<GestureHandlerRootView style={styles.gestureHandler}> const useStyles = () => { return useMemo( () => StyleSheet.create({ safe: { flex: 1, }, + gestureHandler: { + flex: 1, + }, }), [] ); };🧰 Tools
🪛 GitHub Check: lint
[warning] 154-154:
Inline style: { flex: 1 }theme/useAppTheme.ts (1)
154-154: LGTM: Improved theme management.The changes simplify theme handling by:
- Removing navigation theme dependency
- Directly using system color scheme
- Streamlining theme context computation
Consider adding a comment explaining the rationale behind using controlledTheme over navTheme for better maintainability:
+ // Using system color scheme directly instead of navigation theme for simpler theme management const controlledTheme = useColorScheme();Also applies to: 180-180, 189-190, 212-212, 215-215
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
.github/workflows/build-app-release.yml(2 hunks).github/workflows/build-internal.yml(1 hunks)App.tsx(2 hunks)app.config.ts(1 hunks)components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx(1 hunks)components/Onboarding/init-xmtp-client.ts(2 hunks)components/TransactionPreview/TransactionPreview.tsx(2 hunks)config.ts(6 hunks)features/onboarding/Privy/usePrivySmartWalletConnection.ts(1 hunks)features/onboarding/passkey/passkey.constants.ts(1 hunks)features/onboarding/passkey/passkeyAuthStore.tsx(1 hunks)features/onboarding/passkey/useCreatePasskey.ts(1 hunks)features/onboarding/passkey/useLoginWithPasskey.ts(1 hunks)i18n/translations/en.ts(1 hunks)ios/Converse/Converse.entitlements(1 hunks)metro.config.js(2 hunks)navigation/OnboardingNavigator.tsx(3 hunks)package.json(6 hunks)screens/Main.tsx(0 hunks)screens/Navigation/Navigation.tsx(5 hunks)screens/NewAccount/NewAccountScreen.tsx(2 hunks)screens/NewAccount/new-account-passkey-screen.tsx(1 hunks)screens/Onboarding/OnboardingGetStartedScreen.tsx(2 hunks)screens/Onboarding/onboarding-passkey-screen.tsx(1 hunks)scripts/build/ios/preview.js(1 hunks)scripts/build/ios/production.js(1 hunks)theme/useAppTheme.ts(4 hunks)tsconfig.json(1 hunks)utils/events.ts(1 hunks)utils/navigation.ts(1 hunks)utils/xmtpRN/signIn.ts(2 hunks)utils/xmtpRN/signer.ts(2 hunks)
💤 Files with no reviewable changes (1)
- screens/Main.tsx
🧰 Additional context used
🪛 GitHub Check: lint
App.tsx
[warning] 154-154:
Inline style: { flex: 1 }
config.ts
[warning] 39-39:
Prefer named exports
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-with-eas
- GitHub Check: build-with-eas
- GitHub Check: build
- GitHub Check: build
- GitHub Check: performance-test
🔇 Additional comments (31)
utils/events.ts (1)
6-6: Verify type compatibility between viem and thirdweb TransactionReceipt.The migration from thirdweb to viem for TransactionReceipt type could impact existing event handlers. Let's verify the type compatibility.
✅ Verification successful
✓ TransactionReceipt type migration is consistent
The change from thirdweb to viem TransactionReceipt is consistently applied across all usage locations (utils/events.ts and components/TransactionPreview/TransactionPreview.tsx) with compatible type usage patterns maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for TransactionReceipt usage and potential type mismatches # Find all files using TransactionReceipt echo "Files importing TransactionReceipt:" rg -l "TransactionReceipt" # Check for potential type mismatches in event handlers echo "\nEvent handlers using transactionResult:" rg "transactionResult.*receipt.*" -A 2Length of output: 435
Script:
#!/bin/bash # Check implementation details and previous import source # Examine TransactionPreview component echo "TransactionPreview implementation:" ast-grep --pattern 'interface Props { $$$ }' components/TransactionPreview/TransactionPreview.tsx # Check previous version of events.ts echo "\nPrevious import in events.ts:" git show HEAD^:utils/events.ts | rg "TransactionReceipt" # Check type usage in both files echo "\nType usage in TransactionPreview:" rg "TransactionReceipt" components/TransactionPreview/TransactionPreview.tsx -A 2Length of output: 1041
features/onboarding/Privy/usePrivySmartWalletConnection.ts (1)
31-34: Confirm the necessity of logging out on mountThe
useEffecton lines 31-34 logs out the user when the component mounts. Is this intentional? This could cause the user to be logged out unexpectedly when this hook is used.features/onboarding/passkey/passkey.constants.ts (1)
3-4: LGTM!The constants
RPIDandRELYING_PARTYare correctly defined and exported.ios/Converse/Converse.entitlements (1)
11-11: LGTM!The added entry
<string>webcredentials:dev.converse.xyz</string>correctly adds the web credentials domain for the development environment.utils/xmtpRN/signer.ts (1)
17-21: LGTM! Well-structured type definition.The
ViemAccounttype correctly defines the required properties for Viem account integration.metro.config.js (2)
9-19: LGTM! Well-implemented package resolution.The function correctly handles Privy package resolution while maintaining compatibility with other packages.
Line range hint
39-48: Verify package exports compatibility.The configuration enables package exports for Privy modules. Ensure this doesn't affect other packages that might not be compatible with this feature.
✅ Verification successful
Package exports feature is safely scoped to Privy packages only
The implementation of
resolveRequestWithPackageExportsexplicitly enables package exports only for@privy-io/*packages, leaving all other packages unaffected. This scoped approach ensures compatibility and prevents any impact on other dependencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for package.json files with "exports" field rg -l '"exports"' '**/package.json'Length of output: 93
Script:
#!/bin/bash # Find package.json files and check for exports field echo "=== Checking for package.json files with exports field ===" fd package.json --type f --exec rg -l '"exports"' {} echo -e "\n=== Checking main package.json for dependencies ===" fd package.json --max-depth 1 --type f --exec cat {} echo -e "\n=== Checking for import/require statements ===" rg -g '!*.json' -g '!*.md' 'require\(|import .* from' -A 1Length of output: 68831
Script:
#!/bin/bash # Check Privy packages for exports field echo "=== Checking Privy packages for exports field ===" fd package.json -p node_modules/@privy-io --type f --exec rg -l '"exports"' {} echo -e "\n=== Looking for package exports related errors ===" rg -i "package.+exports?.+(error|issue|fail|invalid)" --type ts --type js --type log echo -e "\n=== Checking resolveRequestWithPackageExports implementation ===" rg -p "resolveRequestWithPackageExports" --type ts --type js -A 5Length of output: 1322
features/onboarding/passkey/useLoginWithPasskey.ts (1)
43-46: Verify error handling for missing user.The error handling for a missing user is good, but verify if there are specific error codes or messages from Privy that should be handled differently.
✅ Verification successful
Error handling implementation is appropriate
The current implementation properly handles both specific cases (missing user) and generic errors with user-friendly messages and error tracking. No additional Privy-specific error codes were found that would require different handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Privy error handling patterns in the codebase rg -A 3 "catch.*privy"Length of output: 24
Script:
#!/bin/bash # Search for Privy imports and error handling patterns echo "=== Privy imports ===" rg "from.*privy" -A 2 echo -e "\n=== Error handling in auth files ===" fd -e ts -e tsx . | grep -i "auth\|login" | xargs rg "catch|throw" -A 2 echo -e "\n=== Error constants and types ===" rg "type.*Error|interface.*Error|enum.*Error" -A 2Length of output: 15647
features/onboarding/passkey/passkeyAuthStore.tsx (1)
62-64: Add selector type validationThe
usePasskeyAuthStoreContexthook accepts any selector without type validation. Consider adding runtime validation for the selected value.scripts/build/ios/production.js (1)
93-97: LGTM: Associated domain configuration for passkeysThe addition of the webcredentials domain is correctly configured for the production environment, which is necessary for passkey authentication support.
Verify the domain configuration matches across environments:
✅ Verification successful
✅ Associated domains configuration is consistent
The webcredentials domain configuration correctly transitions from:
- dev.converse.xyz → preview.converse.xyz (in preview)
- dev.converse.xyz → converse.xyz (in production)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check consistency of webcredentials domain configuration rg -l "webcredentials:(dev|preview|)\.?converse\.xyz" scripts/build/ios/Length of output: 135
Script:
#!/bin/bash # Display the domain configuration context from both files echo "=== Production configuration ===" rg -B2 -A2 "webcredentials:" scripts/build/ios/production.js echo -e "\n=== Preview configuration ===" rg -B2 -A2 "webcredentials:" scripts/build/ios/preview.jsLength of output: 724
config.ts (2)
80-80: LGTM: Consistent domain configurationThe appDomain configuration is correctly set up across all environments (dev, preview, prod) and aligns with the iOS entitlements configuration.
Also applies to: 100-100, 136-136
39-40: Verify Privy configuration completenessThe Privy configuration is properly structured with clientId from environment variables. However, ensure all required Privy configuration parameters are included.
Check for any missing Privy configuration parameters:
Also applies to: 120-121
✅ Verification successful
Privy configuration is complete and correctly implemented
The current Privy configuration with
clientIdanddefaultChainparameters is sufficient for the implemented features, including SMS authentication, passkey support, smart wallets, and embedded wallets functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Privy-related configuration patterns in similar projects rg "PRIVY_" .env.example || echo "No .env.example file found"Length of output: 140
Script:
#!/bin/bash # Search for Privy-related files and patterns echo "=== Searching for Privy-related files ===" fd -i privy echo -e "\n=== Searching for Privy imports and configurations ===" rg -i "from ['\"](.*privy|@privy-io)" -A 3 echo -e "\n=== Searching for Privy configuration objects ===" ast-grep --pattern 'const $_ = { $$$ clientId: $_, $$$ }'Length of output: 14791
🧰 Tools
🪛 GitHub Check: lint
[warning] 39-39:
Prefer named exportsscreens/Onboarding/OnboardingGetStartedScreen.tsx (2)
9-22: LGTM: Import path standardizationThe switch to absolute imports improves code maintainability and follows better module resolution patterns.
62-66: Verify passkey implementation completenessThe passkey authentication option is properly integrated into the onboarding flow. However, ensure the OnboardingPasskey screen is properly implemented with appropriate error handling.
Check for the existence and implementation of the OnboardingPasskey screen:
✅ Verification successful
OnboardingPasskey screen implementation verified
The OnboardingPasskey screen is properly implemented with comprehensive error handling:
- Uses error capture and toast notifications
- Manages loading and status states
- Properly integrated with navigation
- Wrapped with necessary authentication providers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for OnboardingPasskey screen implementation ast-grep --pattern 'export function OnboardingPasskeyScreen() { $$$ }'Length of output: 74
Script:
#!/bin/bash # Find files containing OnboardingPasskey fd -t f "OnboardingPasskey" # Search for OnboardingPasskey references in code rg "OnboardingPasskey" -A 5 # Try alternative component definition patterns ast-grep --pattern 'const OnboardingPasskey = $$$' ast-grep --pattern 'export const OnboardingPasskey = $$$'Length of output: 3655
Script:
#!/bin/bash # Check Content component and error handling in passkey screen rg -A 15 "function Content" screens/Onboarding/onboarding-passkey-screen.tsx # Look for error handling patterns rg "catch|error|Error|try" screens/Onboarding/onboarding-passkey-screen.tsxLength of output: 1002
scripts/build/ios/preview.js (1)
120-124: LGTM: Preview environment domain configurationThe webcredentials domain is correctly configured for the preview environment, maintaining consistency with the production setup.
Verify domain configuration consistency:
✅ Verification successful
Domain configuration verified: Preview and production environments are properly aligned
The domain configurations maintain consistency across environments while correctly using environment-specific domains:
- Production: dev.converse.xyz → converse.xyz
- Preview: dev.converse.xyz → preview.converse.xyz
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Compare domain configurations across environments echo "Checking domain configuration consistency across environments..." rg -A 1 "webcredentials:" scripts/build/ios/{preview,production}.jsLength of output: 587
App.tsx (2)
142-147: LGTM: Privy configuration looks good.The PrivyProvider is properly configured with the required props for passkey support: clientId and supportedChains.
148-165: Provider structure is well-organized.The SmartWalletsProvider is correctly positioned within the provider tree, maintaining proper context hierarchy.
🧰 Tools
🪛 GitHub Check: lint
[warning] 154-154:
Inline style: { flex: 1 }app.config.ts (1)
228-228: LGTM: Required plugin for passkey support.The addition of "expo-secure-store" plugin is necessary for secure storage of passkey credentials.
components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx (1)
27-37: LGTM: Well-structured passkey item generator.The new function follows the established pattern and maintains consistency with other item generators.
screens/Navigation/Navigation.tsx (4)
49-50: LGTM: Clean imports for new passkey screens.The imports follow the established pattern and are correctly placed with other screen imports.
65-65: LGTM: Well-typed navigation parameters.The new screen parameters are properly typed as undefined, consistent with other similar screens.
Also applies to: 76-76
171-174: LGTM: Proper screen registration in SignedInNavigation.The NewAccountPasskey screen is correctly placed in the modal group, maintaining consistency with other account-related screens.
220-223: LGTM: Proper screen registration in SignedOutNavigation.The OnboardingPasskey screen is correctly placed in the auth/onboarding group with appropriate screen options inherited from the group configuration.
i18n/translations/en.ts (1)
107-111: LGTM: Clear and consistent translations for passkey features.The translations are well-organized in a dedicated section and provide clear, user-friendly messages for the passkey functionality.
tsconfig.json (1)
7-7: Verify compatibility with existing imports.The change to "Bundler" moduleResolution is good for modern bundlers, but verify that it doesn't break any existing imports, especially in:
- Dynamic imports
- Package exports
- Path aliases
Run this check to verify import compatibility:
✅ Verification successful
moduleResolution "Bundler" change is safe to apply
The change to "Bundler" moduleResolution is compatible with the project's:
- Path alias usage (@/, @components/, etc.)
- Package imports
- Expo toolchain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential import issues with new moduleResolution # Look for any import statements that might be affected # Search for dynamic imports echo "Checking dynamic imports..." rg "import\(.*\)" --type ts --type tsx # Search for package.json exports field usage echo "Checking package.json exports..." fd package.json --exec jq -r '.exports // empty' # Search for path alias usage echo "Checking path alias usage..." rg "from ['\"]@/" --type ts --type tsxLength of output: 397
Script:
#!/bin/bash # First check tsconfig configuration echo "Checking tsconfig.json configuration..." cat tsconfig.json # Then check for various import patterns echo -e "\nChecking import patterns..." # Using -t for file type and checking both .ts and .tsx rg -t ts "import .* from ['\"].*['\"]" # Check for dynamic imports echo -e "\nChecking dynamic imports..." rg -t ts "import\(['\"].*['\"]\)" # Check for path aliases (if any) echo -e "\nChecking path alias usage..." rg -t ts "from ['\"]@"Length of output: 71068
.github/workflows/build-internal.yml (1)
67-67: LGTM: Proper environment variable configuration.The EXPO_PUBLIC_PRIVY_CLIENT_ID is correctly added:
- Uses EXPO_PUBLIC_ prefix for Expo compatibility
- Properly sourced from repository secrets
- Added in the correct location in env file creation
.github/workflows/build-app-release.yml (2)
Line range hint
9-13: LGTM! Removal of scheduled triggersThe removal of scheduled triggers aligns with the current release schedule, making the workflow more maintainable by relying on manual triggers.
49-49: LGTM! Addition of Privy environment variableThe EXPO_PUBLIC_PRIVY_CLIENT_ID environment variable has been correctly added to both Android and iOS build configurations, which is necessary for the Privy passkey integration.
Also applies to: 93-93
package.json (3)
151-151: LGTM! New dependencies align with PR objectivesThe addition of new dependencies supports the PR objectives:
- react-native-passkeys (v0.3.1) for passkey authentication
- permissionless (v0.2.25) for smart contract wallet support
- react-native-qrcode-styled (v0.3.3) for QR code functionality
Also applies to: 174-174, 177-177
56-57: Verify migration steps for Privy SDK major version updateThe update from @privy-io/expo v0.16.2 to v0.36.2 is a significant version jump that may include breaking changes.
Run this script to check for migration guides and breaking changes:
111-111: Verify compatibility of updated Expo packagesMultiple Expo packages have been updated to newer patch versions. While these are minor updates, we should verify compatibility.
Run this script to check for any reported issues:
Also applies to: 117-117, 119-119, 130-130, 135-135, 141-141
✅ Verification successful
Expo package updates are compatible ✅
The updated packages are patch version changes (x.y.Z) which only include backwards-compatible bug fixes. No compatibility issues or bugs were found for these specific versions in the Expo repository.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any known issues with the updated Expo packages gh api graphql -f query=' { search(query: "repo:expo/expo is:issue state:open label:bug", type: ISSUE, first: 10) { nodes { ... on Issue { title url labels(first: 5) { nodes { name } } } } } }'Length of output: 959
Script:
#!/bin/bash # Get all expo-related dependencies and their versions echo "=== Expo Dependencies ===" cat package.json | jq -r '.dependencies | with_entries(select(.key | startswith("expo"))) | to_entries | .[] | "\(.key): \(.value)"' echo -e "\n=== Checking for version-specific issues ===" # Search for issues mentioning specific versions gh api graphql -f query=' { search(query: "repo:expo/expo is:issue state:open label:bug \"6.0.2\"", type: ISSUE, first: 5) { nodes { ... on Issue { title url body } } } }'Length of output: 1382
| if (!creatingEmbeddedWallet.current) { | ||
| creatingEmbeddedWallet.current = true; | ||
| onStatusChange("Creating embedded wallet"); | ||
| logger.debug("[Privy connection] Creating embedded wallet"); | ||
| embeddedWallet | ||
| .create() | ||
| // TODO: Handle better error | ||
| .catch(ensureErrorHandler(onConnectionError)); | ||
| } |
There was a problem hiding this comment.
Reset creatingEmbeddedWallet.current flag on error
In the embedded wallet creation logic, if embeddedWallet.create() fails, creatingEmbeddedWallet.current remains true, preventing further attempts to create the wallet. Consider resetting creatingEmbeddedWallet.current to false in the error handler to allow retries.
Apply this diff to reset the flag on error:
embeddedWallet
.create()
// TODO: Handle better error
.catch((error) => {
+ creatingEmbeddedWallet.current = false;
ensureErrorHandler(onConnectionError)(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.
| if (!creatingEmbeddedWallet.current) { | |
| creatingEmbeddedWallet.current = true; | |
| onStatusChange("Creating embedded wallet"); | |
| logger.debug("[Privy connection] Creating embedded wallet"); | |
| embeddedWallet | |
| .create() | |
| // TODO: Handle better error | |
| .catch(ensureErrorHandler(onConnectionError)); | |
| } | |
| if (!creatingEmbeddedWallet.current) { | |
| creatingEmbeddedWallet.current = true; | |
| onStatusChange("Creating embedded wallet"); | |
| logger.debug("[Privy connection] Creating embedded wallet"); | |
| embeddedWallet | |
| .create() | |
| // TODO: Handle better error | |
| .catch((error) => { | |
| creatingEmbeddedWallet.current = false; | |
| ensureErrorHandler(onConnectionError)(error); | |
| }); | |
| } |
| const Content = memo(function Content() { | ||
| const router = useRouter(); | ||
| // Passkey Store Hooks | ||
| const loading = usePasskeyAuthStoreContext((state) => state.loading); | ||
|
|
||
| const statusString = usePasskeyAuthStoreContext( | ||
| (state) => state.statusString | ||
| ); | ||
|
|
||
| const account = usePasskeyAuthStoreContext((state) => state.account); | ||
|
|
||
| const setError = usePasskeyAuthStoreContext((state) => state.setError); | ||
|
|
||
| const setStatusString = usePasskeyAuthStoreContext( | ||
| (state) => state.setStatusString | ||
| ); | ||
| const { createPasskey: handleCreateAccountWithPasskey } = useCreatePasskey(); | ||
|
|
||
| const { loginWithPasskey: handleLoginWithPasskey } = useLoginWithPasskey(); | ||
|
|
||
| const handleError = useCallback( | ||
| (error: Error) => { | ||
| logger.error(error); | ||
| setError(error.message); | ||
| captureErrorWithToast(error); | ||
| }, | ||
| [setError] | ||
| ); | ||
|
|
||
| usePrivySmartWalletConnection({ | ||
| onConnectionDone: () => { | ||
| if (isMissingConverseProfile()) { | ||
| router.navigate("OnboardingUserProfile"); | ||
| } else if (needToShowNotificationsPermissions()) { | ||
| router.navigate("OnboardingNotifications"); | ||
| } else { | ||
| setAuthStatus("signedIn"); | ||
| } | ||
| }, | ||
| onConnectionError: handleError, | ||
| onStatusChange: setStatusString, | ||
| }); | ||
|
|
||
| const showDebug = useCallback(() => { | ||
| converseEventEmitter.emit("showDebugMenu"); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Create a shared hook for passkey functionality.
The passkey-related logic is duplicated between screens. Consider extracting it into a custom hook.
Create a new shared hook:
// hooks/usePasskeyAuth.ts
export const usePasskeyAuth = (onSuccess: () => void) => {
const router = useRouter();
const loading = usePasskeyAuthStoreContext((state) => state.loading);
const statusString = usePasskeyAuthStoreContext((state) => state.statusString);
const account = usePasskeyAuthStoreContext((state) => state.account);
const setError = usePasskeyAuthStoreContext((state) => state.setError);
const setStatusString = usePasskeyAuthStoreContext((state) => state.setStatusString);
const handleError = useCallback(
(error: Error) => {
logger.error(error);
setError(error.message);
captureErrorWithToast(error);
},
[setError]
);
usePrivySmartWalletConnection({
onConnectionDone: onSuccess,
onConnectionError: handleError,
onStatusChange: setStatusString,
});
const showDebug = useCallback(() => {
converseEventEmitter.emit("showDebugMenu");
}, []);
return {
loading,
statusString,
account,
handleError,
showDebug,
};
};
alexrisch
force-pushed
the
ar/privy-passkey
branch
2 times, most recently
from
5c1e7ed to
003d806
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app.json (1)
Line range hint
1-12: Consider documenting version update strategy.Given that both iOS and Android versions have significant increments, it would be helpful to document the versioning strategy in the repository (e.g., in CONTRIBUTING.md) to help maintainers understand when and why version numbers should be incremented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (2)
app.json (2)
9-9: Verify Android version code increment.The Android versionCode has been incremented by 11 versions (from 257 to 268). While this is valid, please ensure this significant jump in version codes is intentional and aligns with your versioning strategy.
Run this script to check the version history and ensure proper version alignment:
6-6: Verify iOS build number increment.The iOS build number has been incremented by 9 versions (from 58 to 67). While this is valid, please ensure this significant jump in build numbers is intentional and aligns with your versioning strategy.
Run this script to check the version history and ensure no builds are skipped:
✅ Verification successful
Build number increment is part of automated CI/CD process
The increment from 58 to 67 is valid as it's managed by the automated build system (EAS) and dedicated increment scripts. The project uses proper version management with automated PRs for version updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check version history in git to ensure no builds are skipped # and verify the version alignment with release tags # Check git history for build number changes git log -p app.json | grep -B 5 -A 5 "buildNumber" # List tags to verify version alignment git tag --sort=-v:refname | head -n 5Length of output: 39
Script:
#!/bin/bash # Search for buildNumber patterns and version configurations rg -A 2 -B 2 '"buildNumber"' --type json # Look for version-related files fd -e json -e yaml -e yml | xargs rg -l 'version|build' # Check for CI configuration fd -e yml -e yaml . .github/Length of output: 1029
Script:
#!/bin/bash # Check EAS configuration cat eas.json # Examine build workflows cat .github/workflows/build-app-release.yml cat .github/workflows/build-internal.ymlLength of output: 13177
|
before we super review this @alexrisch is this at the level of a) its a poc - shit is going to suck but it works and demos privy |
alexrisch
force-pushed
the
ar/privy-passkey
branch
from
923ada6 to
3d3c926
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
features/onboarding/passkey/passkeyAuthStore.tsx (1)
50-52: 🛠️ Refactor suggestionImprove error handling in setLoading
The
setLoadingimplementation clears errors when loading is true but not when it's false. This asymmetric error handling could lead to stale error states.features/onboarding/Privy/usePrivySmartWalletConnection.ts (2)
46-54:⚠️ Potential issueReset
creatingEmbeddedWallet.currentflag on errorThe embedded wallet creation logic should reset the flag on error to allow retries.
68-101:⚠️ Potential issueReset
initializingXmtp.currentflag on errorThe XMTP initialization logic should reset the flag on error to allow retries.
config.ts (1)
39-40:⚠️ Potential issueVerify chain configuration consistency.
The code imports mainnet
basechain but uses Base Sepolia's chain ID (0x14a34) in the default config. This mismatch needs to be resolved.Apply this diff to fix the chain ID:
- transactionChainId: "0x14a34", // Base Sepolia + transactionChainId: "0x2105", // Base Mainnet🧰 Tools
🪛 GitHub Check: lint
[warning] 39-39:
Prefer named exportscomponents/Onboarding/init-xmtp-client.ts (1)
55-91: 🛠️ Refactor suggestionReduce code duplication in XMTP client initialization.
The initialization logic is duplicated between
initXmtpClientandinitXmtpClientFromViemAccount.This issue was previously identified in a past review. The suggested refactor to extract common initialization logic remains valid.
🧹 Nitpick comments (7)
features/onboarding/passkey/useCreatePasskey.ts (2)
37-58: Add retry mechanism for passkey creationThe
handleCreateAccountWithPasskeyfunction doesn't handle temporary failures that might occur during passkey creation. Consider implementing a retry mechanism with exponential backoff for transient errors.const handleCreateAccountWithPasskey = useCallback(async () => { + const maxRetries = 3; + const baseDelay = 1000; + let retryCount = 0; try { setLoading(true); if (privyUser) { await logout(); } - const { user } = await signupWithPasskey({ - relyingParty: RELYING_PARTY, - }); + while (retryCount < maxRetries) { + try { + const { user } = await signupWithPasskey({ + relyingParty: RELYING_PARTY, + }); + if (!user) { + throw new Error("No account created from Passkey"); + } + setStatusString("Account created - Waiting for smart wallet"); + setPrivyAccountId(user.id); + return; + } catch (e) { + retryCount++; + if (retryCount === maxRetries) throw e; + await new Promise(resolve => setTimeout(resolve, baseDelay * Math.pow(2, retryCount))); + } + } + } catch (e: any) { if (!user) { throw new Error("No account created from Passkey"); }
52-54: Improve error handling specificityThe catch block uses a generic error message. Consider handling specific error types differently to provide more helpful error messages to users.
- } catch (e: any) { - setError(e?.message ?? "Error creating Passkey account"); - captureErrorWithToast(e); + } catch (e: any) { + let errorMessage = "Error creating Passkey account"; + if (e instanceof PrivyError) { + errorMessage = "Failed to create Privy account: " + e.message; + } else if (e instanceof PasskeyError) { + errorMessage = "Failed to create Passkey: " + e.message; + } + setError(errorMessage); + captureErrorWithToast(e);features/onboarding/passkey/passkeyAuthStore.tsx (1)
26-38: Add display name to PasskeyAuthStoreProviderThe provider component is memoized but missing a display name, which can make debugging more difficult.
export const PasskeyAuthStoreProvider = memo( ({ children, ...props }: IPasskeyAuthStoreProviderProps) => { const storeRef = useRef<IPasskeyAuthStore>(); if (!storeRef.current) { storeRef.current = createPasskeyAuthStore(props); } return ( <PasskeyAuthStoreContext.Provider value={storeRef.current}> {children} </PasskeyAuthStoreContext.Provider> ); } -); +, { displayName: 'PasskeyAuthStoreProvider' });screens/NewAccount/new-account-passkey-screen.tsx (1)
69-79: Add loading state during navigationThe
onConnectionDonecallback navigates immediately without showing a loading state, which might lead to a jarring user experience.usePrivySmartWalletConnection({ onConnectionDone: () => { + setStatusString("Finalizing setup..."); if (isMissingConverseProfile()) { - router.navigate("NewAccountUserProfile"); + setTimeout(() => router.navigate("NewAccountUserProfile"), 500); } else { - router.popTo("Chats"); + setTimeout(() => router.popTo("Chats"), 500); } }, onConnectionError: handleError, onStatusChange: setStatusString, });screens/Onboarding/onboarding-passkey-screen.tsx (1)
26-34: Extract providers into a shared HOC.The provider wrapping pattern is duplicated across passkey-related screens. Consider creating a shared HOC to reduce duplication.
// components/Passkey/PasskeyAuthProviders.tsx export const withPasskeyAuth = (WrappedComponent: React.ComponentType) => { return function WithPasskeyAuthWrapper(props: any) { return ( <PrivyAuthStoreProvider> <PasskeyAuthStoreProvider> <WrappedComponent {...props} /> </PasskeyAuthStoreProvider> </PrivyAuthStoreProvider> ); }; };App.tsx (1)
170-170: Extract inline style to StyleSheet.Move the inline style to the StyleSheet definition for better performance and maintainability.
-<GestureHandlerRootView style={{ flex: 1 }}> +<GestureHandlerRootView style={styles.gestureHandler}> const useStyles = () => { return useMemo( () => StyleSheet.create({ safe: { flex: 1, }, + gestureHandler: { + flex: 1, + }, }), [] ); };🧰 Tools
🪛 GitHub Check: lint
[warning] 170-170:
Inline style: { flex: 1 }package.json (1)
Line range hint
1-275: Consider adding resolutions field for security.Given the large number of dependencies and the security-critical nature of wallet/passkey functionality, consider adding a resolutions field to enforce minimum versions of shared dependencies.
Add this section to package.json:
+ "resolutions": { + "@babel/traverse": "^7.25.3", + "axios": "^1.6.0", + "crypto-js": "^4.2.0" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
.github/workflows/build-app-release.yml(2 hunks).github/workflows/build-internal.yml(1 hunks)App.tsx(2 hunks)app.config.ts(1 hunks)components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx(1 hunks)components/Onboarding/init-xmtp-client.ts(2 hunks)components/TransactionPreview/TransactionPreview.tsx(2 hunks)config.ts(6 hunks)features/onboarding/Privy/usePrivySmartWalletConnection.ts(1 hunks)features/onboarding/passkey/passkey.constants.ts(1 hunks)features/onboarding/passkey/passkeyAuthStore.tsx(1 hunks)features/onboarding/passkey/useCreatePasskey.ts(1 hunks)features/onboarding/passkey/useLoginWithPasskey.ts(1 hunks)i18n/translations/en.ts(1 hunks)ios/Converse/Converse.entitlements(1 hunks)metro.config.js(2 hunks)navigation/OnboardingNavigator.tsx(3 hunks)package.json(6 hunks)screens/Main.tsx(0 hunks)screens/Navigation/Navigation.tsx(5 hunks)screens/NewAccount/NewAccountScreen.tsx(2 hunks)screens/NewAccount/new-account-passkey-screen.tsx(1 hunks)screens/Onboarding/OnboardingGetStartedScreen.tsx(2 hunks)screens/Onboarding/onboarding-passkey-screen.tsx(1 hunks)scripts/build/ios/preview.js(1 hunks)scripts/build/ios/production.js(1 hunks)tsconfig.json(1 hunks)utils/events.ts(1 hunks)utils/navigation.ts(1 hunks)utils/xmtpRN/signIn.ts(2 hunks)utils/xmtpRN/signer.ts(2 hunks)
💤 Files with no reviewable changes (1)
- screens/Main.tsx
🚧 Files skipped from review as they are similar to previous changes (16)
- scripts/build/ios/preview.js
- scripts/build/ios/production.js
- features/onboarding/passkey/passkey.constants.ts
- utils/navigation.ts
- tsconfig.json
- app.config.ts
- i18n/translations/en.ts
- .github/workflows/build-internal.yml
- components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx
- navigation/OnboardingNavigator.tsx
- metro.config.js
- screens/NewAccount/NewAccountScreen.tsx
- utils/events.ts
- components/TransactionPreview/TransactionPreview.tsx
- screens/Navigation/Navigation.tsx
- ios/Converse/Converse.entitlements
🧰 Additional context used
🪛 GitHub Check: lint
config.ts
[warning] 39-39:
Prefer named exports
App.tsx
[warning] 170-170:
Inline style: { flex: 1 }
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (15)
features/onboarding/passkey/useLoginWithPasskey.ts (2)
36-57: Add retry mechanism for passkey loginSimilar to passkey creation, the login process should handle temporary failures with a retry mechanism.
51-54: Improve error handling specificityThe error handling can be improved by providing more specific error messages based on the error type.
screens/Onboarding/onboarding-passkey-screen.tsx (2)
36-82: Extract passkey authentication logic into a custom hook.The passkey-related logic should be extracted into a reusable hook to improve maintainability and reusability.
83-112: Extract shared passkey components.The UI components are duplicated between passkey screens. Consider extracting them into shared components.
config.ts (1)
80-80: LGTM: Domain configuration is consistent.The
appDomainconfiguration is correctly set for each environment.Also applies to: 100-100, 136-136
screens/Onboarding/OnboardingGetStartedScreen.tsx (1)
62-66: LGTM: Passkey integration is properly implemented.The passkey option is correctly integrated into the onboarding flow.
App.tsx (1)
158-163: LGTM: Privy configuration is complete.The PrivyProvider is correctly configured with all required props.
.github/workflows/build-app-release.yml (1)
53-53: LGTM: Environment configuration is consistent.The Privy client ID is correctly added to both Android and iOS build configurations.
Also applies to: 101-101
package.json (3)
110-111: Verify Expo SDK compatibility with updated packages.Multiple Expo packages have been updated to patch versions:
- expo-apple-authentication: 7.1.3
- expo-application: 6.0.2
- expo-constants: 17.0.4
- expo-crypto: 14.0.2
- expo-linking: 7.0.4
- expo-secure-store: 14.0.1
- expo-web-browser: 14.0.2
These updates seem to be patch-level changes, but we should verify they're all compatible with the project's Expo SDK version (52.0.0).
Also applies to: 117-119, 130-130, 135-135, 141-141
✅ Verification successful
All updated Expo packages are compatible with SDK 52.0.0 ✅
The patch-level updates are safe to use as all packages specify flexible version requirements (
*) in their peer dependencies, indicating they're designed to work with any Expo SDK version.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Expo SDK compatibility # Get the expo SDK version from package.json EXPO_VERSION=$(jq -r '.dependencies.expo' package.json | tr -d '^~') # Check if all updated packages are compatible with this Expo version for pkg in "expo-apple-authentication" "expo-application" "expo-constants" "expo-crypto" "expo-linking" "expo-secure-store" "expo-web-browser"; do echo "Checking $pkg..." curl -s "https://registry.npmjs.org/$pkg" | jq -r ".versions[] | select(.peerDependencies.expo == \"$EXPO_VERSION\")" doneLength of output: 2404
Script:
#!/bin/bash # Description: Check specific Expo package versions compatibility declare -A versions=( ["expo-apple-authentication"]="7.1.3" ["expo-application"]="6.0.2" ["expo-constants"]="17.0.4" ["expo-crypto"]="14.0.2" ["expo-linking"]="7.0.4" ["expo-secure-store"]="14.0.1" ["expo-web-browser"]="14.0.2" ) for pkg in "${!versions[@]}"; do version="${versions[$pkg]}" echo "Checking $pkg@$version..." curl -s "https://registry.npmjs.org/$pkg/$version" | jq '{ name: .name, version: .version, peerDependencies: .peerDependencies }' doneLength of output: 3010
56-57: Verify compatibility with the major version upgrade of @privy-io/expo.The significant version jump from ^0.16.2 to 0.36.2 requires careful verification:
- Breaking changes might affect existing Privy integration
- New native extensions package might require additional setup
Run this script to check for breaking changes:
✅ Verification successful
The Privy SDK upgrade appears compatible with the current implementation
The codebase correctly implements all critical Privy SDK features including:
- Proper provider setup with required configuration
- Passkey authentication using latest patterns
- Smart wallet integration with correct provider nesting
- Native extensions package properly included
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check breaking changes in @privy-io/expo changelog # Get changelog or release notes gh api repos/privy-io/expo/releases | jq -r '.[].body' | grep -i "breaking" # Check if native extensions are properly configured rg -g "app.{json,config.js}" "expo-native-extensions|@privy-io/expo-native-extensions"Length of output: 254
Script:
#!/bin/bash # Search for Privy SDK usage and configuration echo "=== Privy SDK Usage ===" rg -l "@privy-io/expo" echo -e "\n=== App Configuration ===" fd "app.config" -e js -e json -x cat {} echo -e "\n=== Native Module Setup ===" rg -g "*.{ts,js,tsx,jsx}" "registerNativeModule|nativeExtensions|PrivyNativeExtensions"Length of output: 691
Script:
#!/bin/bash # Check SDK initialization and implementation echo "=== App.tsx Privy Setup ===" rg -A 10 "PrivyProvider|createPrivyClient" App.tsx echo -e "\n=== Authentication Implementation ===" cat features/onboarding/passkey/useLoginWithPasskey.ts cat features/onboarding/Privy/usePrivyConnection.ts echo -e "\n=== Plugin Configuration ===" rg -g "*.{json,js}" "plugins.*privy|expo-native-extensions" -A 5Length of output: 6729
151-151: Verify blockchain dependencies compatibility.New blockchain-related packages have been added:
- permissionless: 0.2.25
- viem: 2.22.7
- wagmi: 2.5.11
These packages are commonly used together but version compatibility should be verified.
Also applies to: 206-207
✅ Verification successful
Blockchain dependencies are compatible ✓
The package versions are compatible:
- wagmi 2.5.11 requires viem 2.x
- permissionless 0.2.25 requires viem ^2.0.0
- viem 2.22.7 satisfies both requirements
Integration patterns in the codebase look correct, with blockchain functionality properly organized in configuration, wallet connection, and transaction handling components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check blockchain package compatibility # Check peer dependencies for pkg in "permissionless" "viem" "wagmi"; do echo "Checking $pkg peer dependencies..." curl -s "https://registry.npmjs.org/$pkg" | jq '.versions | to_entries[] | select(.key | contains("2")) | .value.peerDependencies' done # Look for existing blockchain configurations rg -g "*.{ts,js}" "wagmi|viem|permissionless"Length of output: 66802
Script:
#!/bin/bash # Retry checking wagmi peer dependencies and usage patterns echo "Checking wagmi@2.5.11 peer dependencies..." curl -s "https://registry.npmjs.org/wagmi/2.5.11" | jq '.peerDependencies' echo -e "\nChecking for blockchain package usage patterns..." rg --type ts --type js -l 'wagmi|viem|permissionless' || trueLength of output: 641
utils/xmtpRN/signer.ts (3)
17-21: LGTM! Well-defined type interface.The
ViemAccounttype correctly defines the minimal required properties for XMTP signing functionality.
28-37: LGTM! Proper error handling and logging.The message signing implementation includes appropriate error handling and logging.
27-27: Verify wallet type determination.The wallet type is hardcoded as "SCW" (Smart Contract Wallet). This might not be accurate for all Viem accounts.
Consider adding a parameter to determine the wallet type:
- walletType: () => "SCW", + walletType: () => account.isSmartContract ? "SCW" : "EOA",Run this script to verify wallet type handling in the codebase:
components/Onboarding/init-xmtp-client.ts (1)
63-65: 🛠️ Refactor suggestionFix misleading error message.
The error message "No signer" is incorrect for a Viem account.
- throw new Error("No signer"); + throw new Error("No Viem account provided");Likely invalid or redundant comment.
| const viemAccount: ViemAccount = { | ||
| address: smartAccount.address, | ||
| chainId: | ||
| smartAccount.client.chain?.id ?? config.privy.defaultChain.id, | ||
| signMessage: async ({ message }) => { | ||
| return smartAccount.signMessage({ message }); | ||
| }, | ||
| }; | ||
| await initXmtpClientFromViemAccount({ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout for smart wallet operations
The smart wallet operations don't have a timeout, which could lead to hanging operations if the network is slow or unresponsive.
+ const TIMEOUT_MS = 30000;
+ const timeoutPromise = new Promise((_, reject) =>
+ setTimeout(() => reject(new Error('Smart wallet operation timed out')), TIMEOUT_MS)
+ );
+
const viemAccount: ViemAccount = {
address: smartAccount.address,
chainId:
smartAccount.client.chain?.id ?? config.privy.defaultChain.id,
signMessage: async ({ message }) => {
- return smartAccount.signMessage({ message });
+ return Promise.race([
+ smartAccount.signMessage({ message }),
+ timeoutPromise
+ ]);
},
};📝 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.
| const viemAccount: ViemAccount = { | |
| address: smartAccount.address, | |
| chainId: | |
| smartAccount.client.chain?.id ?? config.privy.defaultChain.id, | |
| signMessage: async ({ message }) => { | |
| return smartAccount.signMessage({ message }); | |
| }, | |
| }; | |
| await initXmtpClientFromViemAccount({ | |
| const TIMEOUT_MS = 30000; | |
| const timeoutPromise = new Promise((_, reject) => | |
| setTimeout(() => reject(new Error('Smart wallet operation timed out')), TIMEOUT_MS) | |
| ); | |
| const viemAccount: ViemAccount = { | |
| address: smartAccount.address, | |
| chainId: | |
| smartAccount.client.chain?.id ?? config.privy.defaultChain.id, | |
| signMessage: async ({ message }) => { | |
| return Promise.race([ | |
| smartAccount.signMessage({ message }), | |
| timeoutPromise | |
| ]); | |
| }, | |
| }; | |
| await initXmtpClientFromViemAccount({ |
utils/xmtpRN/signIn.ts
Outdated
| export const createXmtpClientFromViemAccount = async ( | ||
| account: ViemAccount, | ||
| onInstallationRevoked: () => Promise<void>, | ||
| preAuthenticateToInboxCallback?: () => Promise<void> | ||
| ) => { | ||
| const tempDirectory = await createTemporaryDirectory(); | ||
| const dbEncryptionKey = await getDbEncryptionKey(); | ||
|
|
||
| const options = { | ||
| env, | ||
| enableV3: true, | ||
| dbDirectory: tempDirectory, | ||
| dbEncryptionKey, | ||
| }; | ||
| const inboxId = await getInboxId(await account.address); | ||
| await copyDatabasesToTemporaryDirectory(tempDirectory, inboxId); | ||
|
|
||
| logger.debug("Instantiating client from signer"); | ||
|
|
||
| const client = await Client.create(viemAccountToXmtpSigner(account), { | ||
| ...options, | ||
| preAuthenticateToInboxCallback, | ||
| }); | ||
|
|
||
| if (client.inboxId !== inboxId) { | ||
| throw new Error("Inbox ids don't match"); | ||
| } | ||
|
|
||
| // In case we're logging with an existing libxmtp database, make sure | ||
| // the installation has not already been revoked | ||
| const installationValid = await isClientInstallationValid(client); | ||
| if (!installationValid) { | ||
| await client.dropLocalDatabaseConnection(); | ||
| await deleteLibXmtpDatabaseForInboxId(inboxId); | ||
| onInstallationRevoked(); | ||
| return; | ||
| } | ||
|
|
||
| logger.debug("Instantiated client from signer"); | ||
| // This Client is only be used to extract the key, we can disconnect | ||
| // it to prevent locks happening during Onboarding | ||
| await client.dropLocalDatabaseConnection(); | ||
| await moveTemporaryDatabasesToDatabaseDirectory( | ||
| tempDirectory, | ||
| client.inboxId | ||
| ); | ||
| logger.debug("Dropped client databases"); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication with the existing client creation function.
The new createXmtpClientFromViemAccount function duplicates most of the logic from createXmtpClientFromSigner. Consider extracting the common initialization logic into a shared function.
Apply this refactor to reduce duplication:
+async function createXmtpClientCommon(
+ options: {
+ env: string;
+ enableV3: boolean;
+ dbDirectory: string;
+ dbEncryptionKey: string;
+ },
+ inboxId: string,
+ createClient: () => Promise<Client>,
+ onInstallationRevoked: () => Promise<void>
+) {
+ await copyDatabasesToTemporaryDirectory(options.dbDirectory, inboxId);
+
+ logger.debug("Instantiating client");
+ const client = await createClient();
+
+ if (client.inboxId !== inboxId) {
+ throw new Error("Inbox ids don't match");
+ }
+
+ const installationValid = await isClientInstallationValid(client);
+ if (!installationValid) {
+ await client.dropLocalDatabaseConnection();
+ await deleteLibXmtpDatabaseForInboxId(inboxId);
+ onInstallationRevoked();
+ return;
+ }
+
+ logger.debug("Instantiated client");
+ await client.dropLocalDatabaseConnection();
+ await moveTemporaryDatabasesToDatabaseDirectory(
+ options.dbDirectory,
+ client.inboxId
+ );
+ logger.debug("Dropped client databases");
+}
export const createXmtpClientFromViemAccount = async (
account: ViemAccount,
onInstallationRevoked: () => Promise<void>,
preAuthenticateToInboxCallback?: () => Promise<void>
) => {
const tempDirectory = await createTemporaryDirectory();
const dbEncryptionKey = await getDbEncryptionKey();
const options = {
env,
enableV3: true,
dbDirectory: tempDirectory,
dbEncryptionKey,
};
const inboxId = await getInboxId(await account.address);
- await copyDatabasesToTemporaryDirectory(tempDirectory, inboxId);
-
- logger.debug("Instantiating client from signer");
-
- const client = await Client.create(viemAccountToXmtpSigner(account), {
- ...options,
- preAuthenticateToInboxCallback,
- });
-
- if (client.inboxId !== inboxId) {
- throw new Error("Inbox ids don't match");
- }
-
- const installationValid = await isClientInstallationValid(client);
- if (!installationValid) {
- await client.dropLocalDatabaseConnection();
- await deleteLibXmtpDatabaseForInboxId(inboxId);
- onInstallationRevoked();
- return;
- }
-
- logger.debug("Instantiated client from signer");
- await client.dropLocalDatabaseConnection();
- await moveTemporaryDatabasesToDatabaseDirectory(
- tempDirectory,
- client.inboxId
- );
- logger.debug("Dropped client databases");
+ await createXmtpClientCommon(
+ options,
+ inboxId,
+ () => Client.create(viemAccountToXmtpSigner(account), {
+ ...options,
+ preAuthenticateToInboxCallback,
+ }),
+ onInstallationRevoked
+ );
};📝 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.
| export const createXmtpClientFromViemAccount = async ( | |
| account: ViemAccount, | |
| onInstallationRevoked: () => Promise<void>, | |
| preAuthenticateToInboxCallback?: () => Promise<void> | |
| ) => { | |
| const tempDirectory = await createTemporaryDirectory(); | |
| const dbEncryptionKey = await getDbEncryptionKey(); | |
| const options = { | |
| env, | |
| enableV3: true, | |
| dbDirectory: tempDirectory, | |
| dbEncryptionKey, | |
| }; | |
| const inboxId = await getInboxId(await account.address); | |
| await copyDatabasesToTemporaryDirectory(tempDirectory, inboxId); | |
| logger.debug("Instantiating client from signer"); | |
| const client = await Client.create(viemAccountToXmtpSigner(account), { | |
| ...options, | |
| preAuthenticateToInboxCallback, | |
| }); | |
| if (client.inboxId !== inboxId) { | |
| throw new Error("Inbox ids don't match"); | |
| } | |
| // In case we're logging with an existing libxmtp database, make sure | |
| // the installation has not already been revoked | |
| const installationValid = await isClientInstallationValid(client); | |
| if (!installationValid) { | |
| await client.dropLocalDatabaseConnection(); | |
| await deleteLibXmtpDatabaseForInboxId(inboxId); | |
| onInstallationRevoked(); | |
| return; | |
| } | |
| logger.debug("Instantiated client from signer"); | |
| // This Client is only be used to extract the key, we can disconnect | |
| // it to prevent locks happening during Onboarding | |
| await client.dropLocalDatabaseConnection(); | |
| await moveTemporaryDatabasesToDatabaseDirectory( | |
| tempDirectory, | |
| client.inboxId | |
| ); | |
| logger.debug("Dropped client databases"); | |
| }; | |
| async function createXmtpClientCommon( | |
| options: { | |
| env: string; | |
| enableV3: boolean; | |
| dbDirectory: string; | |
| dbEncryptionKey: string; | |
| }, | |
| inboxId: string, | |
| createClient: () => Promise<Client>, | |
| onInstallationRevoked: () => Promise<void> | |
| ) { | |
| await copyDatabasesToTemporaryDirectory(options.dbDirectory, inboxId); | |
| logger.debug("Instantiating client"); | |
| const client = await createClient(); | |
| if (client.inboxId !== inboxId) { | |
| throw new Error("Inbox ids don't match"); | |
| } | |
| const installationValid = await isClientInstallationValid(client); | |
| if (!installationValid) { | |
| await client.dropLocalDatabaseConnection(); | |
| await deleteLibXmtpDatabaseForInboxId(inboxId); | |
| onInstallationRevoked(); | |
| return; | |
| } | |
| logger.debug("Instantiated client"); | |
| await client.dropLocalDatabaseConnection(); | |
| await moveTemporaryDatabasesToDatabaseDirectory( | |
| options.dbDirectory, | |
| client.inboxId | |
| ); | |
| logger.debug("Dropped client databases"); | |
| } | |
| export const createXmtpClientFromViemAccount = async ( | |
| account: ViemAccount, | |
| onInstallationRevoked: () => Promise<void>, | |
| preAuthenticateToInboxCallback?: () => Promise<void> | |
| ) => { | |
| const tempDirectory = await createTemporaryDirectory(); | |
| const dbEncryptionKey = await getDbEncryptionKey(); | |
| const options = { | |
| env, | |
| enableV3: true, | |
| dbDirectory: tempDirectory, | |
| dbEncryptionKey, | |
| }; | |
| const inboxId = await getInboxId(await account.address); | |
| await createXmtpClientCommon( | |
| options, | |
| inboxId, | |
| () => Client.create(viemAccountToXmtpSigner(account), { | |
| ...options, | |
| preAuthenticateToInboxCallback, | |
| }), | |
| onInstallationRevoked | |
| ); | |
| }; |
alexrisch
force-pushed
the
ar/privy-passkey
branch
from
5772d2c to
ee9dbbb
Compare
alexrisch
force-pushed
the
ar/privy-passkey
branch
from
26fd02d to
3140e3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 24
🔭 Outside diff range comments (3)
features/onboarding/screens/OnboardingPrivateKeyScreen.tsx (1)
Line range hint
31-49: Critical: Implement secure private key handling.The current implementation has several security vulnerabilities in handling private keys:
- Private keys are stored in plain text state
- No secure memory clearing after use
- No input masking
- Potential exposure through clipboard operations
Recommended security improvements:
export function OnboardingPrivateKeyScreen({...}) { - const [privateKey, setPrivateKey] = useState(""); + const [privateKey, setPrivateKey] = useState<string>(""); + + // Clear private key from memory when component unmounts + useEffect(() => { + return () => { + setPrivateKey(""); + }; + }, []); const handlePressConnect = useCallback(async () => { try { const trimmedPrivateKey = privateKey.trim(); if (!trimmedPrivateKey) return; await loginWithPrivateKey(trimmedPrivateKey); + // Clear private key from memory after use + setPrivateKey(""); if (isMissingConverseProfile()) { router.navigate("OnboardingContactCard"); } // ... } } export const PrivateKeyInput = ({ value, onChange }: IPrivateKeyInputProps) => { return ( <TextInput multiline textAlignVertical="top" + secureTextEntry={true} + contextMenuHidden={true} + autoCorrect={false} + autoCapitalize="none" style={styles.textInput} // ... /> ); };Additionally, consider:
- Using secure storage (e.g., Keychain) for temporary private key storage
- Implementing a secure input method that prevents screen recording
- Adding clipboard warnings and auto-clear functionality
Also applies to: 115-146
screens/NewAccount/NewAccountConnectWalletScreen.tsx (1)
Line range hint
18-27: Add missing router dependency to useCallback.The callback uses
router.navigatebut router is not included in the dependency array.}, - // eslint-disable-next-line react-hooks/exhaustive-deps - [] + [router] );features/onboarding/screens/OnboardingConnectWalletScreen.tsx (1)
Line range hint
26-34: Add missing router dependency to handleDoneConnecting callback.Similar to NewAccountConnectWalletScreen, the router dependency is missing.
}, - // eslint-disable-next-line react-hooks/exhaustive-deps - [] + [router] );
♻️ Duplicate comments (5)
utils/xmtpRN/signIn.ts (1)
74-121: 🛠️ Refactor suggestionReduce code duplication with existing client creation function.
The implementation duplicates most of the logic from
createXmtpClientFromSigner. Consider extracting the common initialization logic into a shared function.Additionally, the log message on line 91 states "Instantiating client from signer" which is incorrect as we're using a Viem account.
- logger.debug("Instantiating client from signer"); + logger.debug("Instantiating client from Viem account");components/Onboarding/init-xmtp-client.ts (1)
55-91: 🛠️ Refactor suggestionReduce code duplication in XMTP client initialization.
The
initXmtpClientFromViemAccountfunction shares significant logic withinitXmtpClient. Consider extracting the common initialization logic.features/onboarding/Privy/usePrivySmartWalletConnection.ts (3)
81-89: 🛠️ Refactor suggestionAdd timeout for smart wallet operations.
The smart wallet operations don't have a timeout, which could lead to hanging operations if the network is slow or unresponsive.
Apply this diff to add a timeout:
+ const TIMEOUT_MS = 30000; + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => reject(new Error('Smart wallet operation timed out')), TIMEOUT_MS) + ); + const viemAccount: ViemAccount = { address: smartAccount.address, chainId: smartAccount.client.chain?.id ?? config.privy.defaultChain.id, signMessage: async ({ message }) => { - return smartAccount.signMessage({ message }); + return Promise.race([ + smartAccount.signMessage({ message }), + timeoutPromise + ]); }, };🧰 Tools
🪛 GitHub Check: tsc
[failure] 84-84:
Property 'defaultChain' does not exist on type '{ appId: string; clientId: string; } | { readonly appId: string; readonly clientId: string; }'.🪛 GitHub Actions: Typescript CI
[error] 84-84: Property 'defaultChain' does not exist on type '{ appId: string; clientId: string; } | { readonly appId: string; readonly clientId: string; }'
46-54:⚠️ Potential issueReset
creatingEmbeddedWallet.currentflag on error.In the embedded wallet creation logic, if
embeddedWallet.create()fails,creatingEmbeddedWallet.currentremainstrue, preventing further attempts to create the wallet.Apply this diff to reset the flag on error:
embeddedWallet .create() // TODO: Handle better error - .catch(ensureErrorHandler(onConnectionError)); + .catch((error) => { + creatingEmbeddedWallet.current = false; + ensureErrorHandler(onConnectionError)(error); + });
95-97:⚠️ Potential issueReset
initializingXmtp.currentflag on error.In the XMTP initialization logic, if initialization fails,
initializingXmtp.currentremainstrue, preventing further attempts to initialize XMTP.Apply this diff to reset the flag on error:
} catch (error) { + initializingXmtp.current = false; onConnectionError(ensureError(error)); }
🧹 Nitpick comments (38)
features/onboarding/components/onboarding-subtitle.tsx (2)
1-3: Consider organizing imports by external/internal dependencies.While the imports are well-structured, consider grouping them by:
- External dependencies (react, react-native)
- Internal dependencies (@/design-system)
+import { memo } from "react"; +import { TextStyle } from "react-native"; + import { AnimatedText, IAnimatedTextProps } from "@/design-system/Text"; -import { memo } from "react"; -import { TextStyle } from "react-native";
9-14: Add JSDoc documentation and consider explicit prop handling.While the implementation is solid, consider:
- Adding JSDoc documentation to describe the component's purpose and props
- Explicitly listing commonly used props instead of spreading all props
+/** + * A reusable subtitle component for onboarding screens with animation support. + * @param props - Extended AnimatedText props for customization + * @param props.style - Additional styles to apply to the text + * @param props.children - The text content to display + */ export const OnboardingSubtitle = memo(function OnboardingSubtitle( props: IAnimatedTextProps ) { const { style, ...rest } = props; return <AnimatedText size="sm" style={[$defaultStyle, style]} {...rest} />; });components/Onboarding/init-xmtp-client.ts (1)
63-65: Fix incorrect error message.The error message should reflect that we're checking for a Viem account, not a signer.
- throw new Error("No signer"); + throw new Error("No Viem account provided");features/onboarding/screens/OnboardingPrivateKeyScreen.tsx (1)
18-27: Consider using path aliases for improved maintainability.The deep nesting in import paths (
../../../) could be simplified using path aliases, similar to how@componentsand@i18nare used in other imports. This would make the imports more maintainable and less prone to errors when files are moved.Example transformation:
-import { OnboardingPictoTitleSubtitle } from "../../../components/Onboarding/OnboardingPictoTitleSubtitle"; +import { OnboardingPictoTitleSubtitle } from "@components/Onboarding/OnboardingPictoTitleSubtitle";features/onboarding/screens/OnboardingEphemeraScreen.tsx (2)
12-23: Consider using path aliases for deep imports.The deep relative paths ("../../../") can be fragile and harder to maintain. Consider using TypeScript path aliases to make imports more maintainable and less prone to breaking during file moves.
Example configuration in
tsconfig.json:{ "compilerOptions": { "baseUrl": ".", "paths": { "@onboarding/*": ["features/onboarding/*"], "@components/*": ["components/*"], "@design-system/*": ["design-system/*"], "@screens/*": ["screens/*"] } } }Then imports could be simplified to:
import { OnboardingPictoTitleSubtitle } from "@components/Onboarding/OnboardingPictoTitleSubtitle"; import { Text } from "@design-system/Text/Text";
42-42: Document the navigation flow change.The navigation target has been changed from "OnboardingUserProfile" to "OnboardingContactCard". While this appears to be part of a broader restructuring of the onboarding flow, it would be helpful to document why this change was made to maintain clarity for future developers.
Consider adding a comment explaining the navigation flow:
+ // Navigate to contact card creation after wallet generation + // as part of the new Privy passkey onboarding flow router.navigate("OnboardingContactCard");design-system/IconButton/IconButton.tsx (1)
20-23: Add explicit type for the ref parameter.While the implementation is correct, the ref parameter should be explicitly typed for better type safety.
export const IconButton = React.forwardRef(function IconButton( props: IIconButtonProps, - ref + ref: React.ForwardedRef<any> ) {design-system/IconButton/IconButton.styles.ts (1)
51-54: Use theme spacing system for xl size dimensions.The hard-coded values (56) break the pattern of using the theme's spacing system. Consider using theme spacing values for consistency.
xl: { - width: 56, - height: 56, + width: spacing.xxxl, + height: spacing.xxxl, },theme/timing.ts (1)
6-8: Maintain consistent naming convention across timing constants.The new timing constants introduce inconsistent naming patterns:
xSlowandxxSlowuse a different prefix style than existing constants"3xSlow"as a string literal key breaks the camelCase pattern and requires string indexingConsider using one of these consistent approaches:
- xSlow: 1000, - xxSlow: 2000, - "3xSlow": 3000, + // Option 1: Descriptive naming (matches existing pattern) + verySlow: 1000, + extraSlow: 2000, + ultraslow: 3000, + // Option 2: Numeric prefix + slow1x: 1000, + slow2x: 2000, + slow3x: 3000,features/onboarding/constants/animationConstants.ts (2)
3-3: Consider type safety for timing constant access.Using string literal indexing with
timing["3xSlow"]is fragile and could break if the key is renamed.Consider using a type-safe reference:
-export const ONBOARDING_ENTERING_DURATION = timing["3xSlow"]; +export const ONBOARDING_ENTERING_DURATION = timing.slow3x; // after renaming in timing.ts
5-9: Document the purpose of animation delays and consider timing intervals.The enum defines small incremental delays (50ms differences) which might not be perceptibly different to users.
Consider:
- Adding documentation explaining the purpose of these specific timings
- Using more distinct intervals for noticeable staging effects
+/** + * Defines sequential animation delays for onboarding elements. + * Each stage is delayed to create a staggered entrance effect. + */ export enum ONBOARDING_ENTERING_DELAY { - FIRST = 200, - SECOND = 250, - THIRD = 300, + FIRST = 200, + SECOND = 400, // More distinct delay + THIRD = 600, // More distinct delay }features/onboarding/passkey/passkeyAuthStore.tsx (2)
26-38: Add display name for better debugging.While the Provider implementation is solid, adding a display name would improve debugging experience.
export const PasskeyAuthStoreProvider = memo( ({ children, ...props }: IPasskeyAuthStoreProviderProps) => { const storeRef = useRef<IPasskeyAuthStore>(); if (!storeRef.current) { storeRef.current = createPasskeyAuthStore(props); } return ( <PasskeyAuthStoreContext.Provider value={storeRef.current}> {children} </PasskeyAuthStoreContext.Provider> ); } ); +PasskeyAuthStoreProvider.displayName = 'PasskeyAuthStoreProvider';
56-56: Consider secure data cleanup in reset.Since this store handles sensitive authentication data, consider implementing a more thorough cleanup in the
resetmethod to ensure all sensitive data is properly cleared.- reset: () => set(DEFAULT_PROPS), + reset: () => { + // Ensure sensitive data is properly cleared + set({ + ...DEFAULT_PROPS, + account: undefined, // Explicitly clear sensitive data + statusString: undefined, + }); + },design-system/Icon/Icon.tsx (1)
71-72: LGTM! Platform-appropriate icon mappings with suggestion.The mappings are well-chosen for iOS:
- Using "faceid" for biometric is appropriate
- Using "camera.fill" maintains iOS style consistency
However, consider extracting platform-specific icon names into a shared configuration to make platform differences more explicit and maintainable.
Example approach:
// platformIcons.ts export const platformSpecificIcons = { biometric: Platform.select({ ios: 'faceid', android: 'fingerprint' }), camera: Platform.select({ ios: 'camera.fill', android: 'camera' }) };features/onboarding/types/onboarding.types.ts (2)
1-5: Consider adding validation constraints to ProfileType.The type could benefit from additional constraints and documentation:
- Username length limits
- Allowed character patterns
- Avatar URL format
Consider adding JSDoc comments and using branded types:
+/** + * Represents a user profile with optional avatar and display name. + * @property username - Must be 3-30 characters, alphanumeric with underscores + * @property avatar - Optional URL to user's avatar image + * @property displayName - Optional human-readable name + */ export type ProfileType = { - avatar?: string; + avatar?: `https://${string}` | `data:image/${string}`; username: string; displayName?: string; };
7-18: Add specific error types for better error handling.The current error type could be more specific about possible error scenarios.
Consider using an enum for error types:
+export enum ProfileError { + USERNAME_TAKEN = "USERNAME_TAKEN", + INVALID_USERNAME = "INVALID_USERNAME", + INVALID_AVATAR_URL = "INVALID_AVATAR_URL", + SERVER_ERROR = "SERVER_ERROR" +} type CreateOrUpdateProfileError = { success: false; - errorMessage: string; + errorCode: ProfileError; + errorMessage: string; };features/onboarding/components/onboarding-icon-button.tsx (2)
10-15: Add accessibility props and improve memoization.The component could benefit from better accessibility support and memoization optimization.
Consider these improvements:
+import { useMemo } from "react"; export const OnboardingIconButton = memo(function OnboardingIconButton( - props: IIconButtonProps + props: IIconButtonProps & { accessibilityLabel?: string } ) { - const { style, ...rest } = props; + const { style, accessibilityLabel, ...rest } = props; + const combinedStyle = useMemo( + () => [$defaultStyle, style], + [style] + ); - return <IconButton style={[$defaultStyle, style]} {...rest} />; + return ( + <IconButton + style={combinedStyle} + accessibilityRole="button" + accessibilityLabel={accessibilityLabel} + {...rest} + /> + ); });
6-8: Consider making default style configurable.The hardcoded default style might not be suitable for all use cases.
Consider making it configurable:
+export const ONBOARDING_ICON_BUTTON_STYLES = { + default: { + alignSelf: "center", + }, +} as const; -const $defaultStyle: ViewStyle = { - alignSelf: "center", -}; +const $defaultStyle: ViewStyle = ONBOARDING_ICON_BUTTON_STYLES.default;features/onboarding/components/onboarding-title.tsx (1)
5-7: Consider using theme spacing for text alignment.For consistency with the theme system, consider defining the text alignment in a themed style function similar to other components in the PR.
-const $defaultStyle: TextStyle = { +const $defaultStyle: ThemedStyle<TextStyle> = () => ({ textAlign: "center", -}; +});theme/spacing.ts (1)
9-9: Consider documenting the use case for this spacing value.While the value fits the progression, adding a comment about its intended use case would help maintain consistency across the application.
+ /** Used for fine-tuned spacing in onboarding components */ "3xs": 3,features/onboarding/components/onboarding-footer-text.tsx (1)
12-24: Add JSDoc documentation for the component and its props.While the implementation is solid, adding documentation would improve maintainability.
+/** + * Renders footer text in onboarding screens with consistent styling and animations. + * @param props - Extended AnimatedText props for customization + */ export const OnboardingFooterText = memo(function OnboardingFooterText(features/onboarding/components/onboarding-contact-card-provider.tsx (1)
8-27: LGTM! Clean implementation of theme context.Good use of memoization and clear documentation. The implementation effectively handles theme inversion without prop drilling.
Consider adding prop-types validation for additional type safety in development:
+import PropTypes from 'prop-types'; export const OnboardingContactCardThemeProvider = memo( function OnboardingContactCardThemeProvider({ children, }: { children: React.ReactNode; }) { // ... implementation ... } ); +OnboardingContactCardThemeProvider.propTypes = { + children: PropTypes.node.isRequired, +};features/onboarding/components/onboarding-notification-row.tsx (1)
17-30: Extract styles to a separate styles object.Consider moving all styles to a single styles object for better maintainability.
-const $switchContainer: ViewStyle = { - flex: 1, - justifyContent: "flex-end", -}; - -const $container: ThemedStyle<ViewStyle> = ({ spacing }) => ({ - marginVertical: spacing.xs, - marginHorizontal: spacing.xxs, -}); - -const $flex2: ViewStyle = { - flex: 2, -}; +const styles = { + switchContainer: { + flex: 1, + justifyContent: "flex-end", + } as ViewStyle, + + container: ({ spacing }: Theme) => ({ + marginVertical: spacing.xs, + marginHorizontal: spacing.xxs, + }) as ThemedStyle<ViewStyle>, + + content: { + flex: 2, + } as ViewStyle, +};features/onboarding/hooks/useProfile.ts (2)
23-26: Consider using optional chaining for safer property access.The current implementation could throw if
currentUserUsernameis undefined.- const usernameWithoutSuffix = currentUserUsername?.name?.replace( - config.usernameSuffix, - "" - ); + const usernameWithoutSuffix = currentUserUsername?.name?.replace?.( + config.usernameSuffix, + "" + ) ?? "";
37-45: Consider memoizing the profile state initialization.The profile state initialization logic could be memoized to prevent unnecessary recalculations.
+ const initialProfile = useMemo(() => ({ + username: ephemeralAccount + ? defaultEphemeralUsername + : usernameWithoutSuffix || "", + avatar: currentUserUsername?.avatar || "", + displayName: ephemeralAccount + ? defaultEphemeralDisplayName + : currentUserUsername?.displayName || "", + }), [ + ephemeralAccount, + defaultEphemeralUsername, + usernameWithoutSuffix, + currentUserUsername?.avatar, + defaultEphemeralDisplayName, + currentUserUsername?.displayName + ]); - const [profile, setProfile] = useState<ProfileType>({ - username: ephemeralAccount - ? defaultEphemeralUsername - : usernameWithoutSuffix || "", - avatar: currentUserUsername?.avatar || "", - displayName: ephemeralAccount - ? defaultEphemeralDisplayName - : currentUserUsername?.displayName || "", - }); + const [profile, setProfile] = useState<ProfileType>(initialProfile);screens/NewAccount/NewAccountConnectWalletScreen.tsx (1)
Line range hint
29-34: Add error type definition and improve error handling.The error handling could be more robust with proper error typing and user feedback.
- const handleErrorConnecting = useCallback( - (arg: { error: Error }) => { - router.goBack(); - }, + const handleErrorConnecting = useCallback( + ({ error }: { error: Error }) => { + logger.error("[NewAccount] Wallet connection failed:", error); + router.goBack(); + // Consider showing a user-friendly error message + },features/onboarding/screens/OnboardingConnectWalletScreen.tsx (1)
Line range hint
36-41: Consider adding user feedback for connection errors.While the error is logged, the user experience could be improved with visual feedback.
const handleErrorConnecting = useCallback((arg: { error: Error }) => { logger.debug("[Onboarding] Error connecting wallet", arg.error); + // Consider showing a toast or alert to inform the user router.goBack(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [router]);features/onboarding/hooks/useAddPfp.ts (1)
34-62: Consider adding loading state during media operations.Media operations can take time. Adding a loading state would improve user experience.
+ const [isLoading, setIsLoading] = useState(false); const addPFP = useCallback(() => { const showOptions = () => showActionSheetWithOptions( { options: [ translate("userProfile.mediaOptions.takePhoto"), translate("userProfile.mediaOptions.chooseFromLibrary"), translate("userProfile.mediaOptions.cancel"), ], cancelButtonIndex: 2, + disabled: isLoading, ...actionSheetColors(colorScheme), }, async (selectedIndex?: number) => { + setIsLoading(true); try { switch (selectedIndex) { case 0: await openCamera(); break; case 1: await pickMedia(); break; default: break; } + } finally { + setIsLoading(false); + } } ); executeAfterKeyboardClosed(showOptions); - }, [colorScheme, openCamera, pickMedia]); + }, [colorScheme, openCamera, pickMedia, isLoading]); - return { addPFP, asset }; + return { addPFP, asset, isLoading };features/onboarding/components/onboarding-footer.tsx (1)
41-46: Consider adding keyboard dismiss on scroll.The keyboard handling is implemented well, but consider adding keyboard dismiss functionality when the user scrolls for better UX.
utils/str.ts (1)
44-46: Consider renaming the function and reducing code duplication.
- The term "Rando" seems informal for production code. Consider a more professional name like
formatRandomDisplayName.- This function shares logic with
formatEphemeralDisplayName. Consider refactoring to reuse code.-export const formatRandoDisplayName = (address: string) => - humanize(address.slice(2, 42), 2, " ", true); +export const formatRandomDisplayName = (address: string) => + formatEphemeralDisplayName(address);screens/NewAccount/NewAccountEphemeraScreen.tsx (1)
19-19: Improve error handling and user feedback.While the error is tracked with Sentry, consider providing user feedback when wallet generation fails.
} catch (error) { sentryTrackError(error); + // Show error toast or alert to inform user + Alert.alert( + translate("errors.title"), + translate("errors.walletGeneration") + ); }Also applies to: 38-38
features/onboarding/components/onboarding-contact-card.tsx (2)
63-65: Add loading state for PFP upload.The avatar press handler should show a loading state during PFP upload.
+const [isUploading, setIsUploading] = useState(false); + +const handlePFPUpload = useCallback(async () => { + setIsUploading(true); + try { + await addPFP(); + } finally { + setIsUploading(false); + } +}, [addPFP]); -<Pressable disabled={!editable} onPress={addPFP}> +<Pressable disabled={!editable || isUploading} onPress={handlePFPUpload}> <OnboardingEditableAvatar uri={pfpUri} name={displayName} + isLoading={isUploading} /> </Pressable>
51-62: Enhance accessibility support.Add appropriate accessibility labels and roles to improve the component's usability for screen readers.
<AnimatedVStack + accessible={true} + accessibilityRole="form" + accessibilityLabel={translate("onboarding.contactCard.accessibilityLabel")} style={{ marginVertical: spacing.lg, padding: spacing.lg,features/onboarding/hooks/useCreateOrUpdateProfileInfo.ts (2)
49-51: Remove debug logging in production code.The debug log contains what appears to be a development identifier "[OnboardingContactCardScreen]5" which should be removed or made more descriptive.
- logger.debug( - `[OnboardingContactCardScreen]5 ${profile.username.length} + ${profile.username}` - ); + logger.debug( + `[useCreateOrUpdateProfileInfo] Invalid username length: ${profile.username.length}, username: ${profile.username}` + );
107-125: Improve error handling granularity.The error handling is too generic. Different types of errors should be handled differently to provide better user feedback.
try { await claimProfile({ account: address, profile: { ...profile, avatar: publicAvatar }, }); await invalidateProfileSocialsQuery(address); return { success: true }; } catch (e: any) { logger.error(e, { context: "UserProfile: claiming and refreshing" }); + let errorMessage = "An unknown error occurred"; + if (e?.response?.status === 409) { + errorMessage = "Profile already exists"; + } else if (e?.response?.status === 413) { + errorMessage = "Profile data too large"; + } else if (e?.response?.data?.message) { + errorMessage = e.response.data.message; + } - setErrorMessage( - e?.response?.data?.message || "An unknown error occurred" - ); + setErrorMessage(errorMessage); setLoading(false); return { success: false, - errorMessage: - e?.response?.data?.message || "An unknown error occurred", + errorMessage, }; }screens/NewAccount/new-account-screen.tsx (1)
72-74: Remove or enhance debug logging.The debug log for status change lacks meaningful context about what the status represents.
- logger.debug("[NewAccountWelcomeScreenContent] onStatusChange", status); + logger.debug("[NewAccountWelcomeScreenContent] Connection status changed", { + status, + timestamp: new Date().toISOString(), + });features/onboarding/screens/onboarding-notifications-screen.tsx (1)
100-107: Consider consolidating disabled notification rows.The code has multiple
OnboardingNotificationRowcomponents with identical disabled states, which could be refactored to reduce duplication.Consider creating a configuration array:
const NOTIFICATION_TYPES = [ { title: "onboarding.notifications.mentionsOnly", description: "onboarding.notifications.mentionsOnlyDescription", }, { title: "onboarding.notifications.cash", description: "onboarding.notifications.cashDescription", }, ];Then map over it:
{NOTIFICATION_TYPES.map(({ title, description }) => ( <OnboardingNotificationRow key={title} title={translate(title)} description={translate(description)} disabled value={false} onToggle={() => {}} /> ))}Also applies to: 108-123
App.tsx (1)
176-176: Consider extracting inline styles to StyleSheet.The inline style
{ flex: 1 }should be moved to the StyleSheet definition for better performance and maintainability.const useStyles = () => { return useMemo( () => StyleSheet.create({ safe: { flex: 1, }, + root: { + flex: 1, + }, }), [] ); };Then update the component:
-<GestureHandlerRootView style={{ flex: 1 }}> +<GestureHandlerRootView style={styles.root}>🧰 Tools
🪛 GitHub Check: lint
[warning] 176-176:
Inline style: { flex: 1 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (81)
.github/workflows/build-app-release.yml(2 hunks).github/workflows/build-internal.yml(1 hunks)App.tsx(3 hunks)app.config.ts(1 hunks)components/Avatar.tsx(2 hunks)components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx(1 hunks)components/Onboarding/init-xmtp-client.ts(2 hunks)components/TransactionPreview/TransactionPreview.tsx(2 hunks)components/__tests__/Avatar.test.tsx(1 hunks)config.ts(5 hunks)design-system/Icon/Icon.android.tsx(1 hunks)design-system/Icon/Icon.tsx(1 hunks)design-system/Icon/Icon.types.ts(1 hunks)design-system/IconButton/IconButton.props.ts(1 hunks)design-system/IconButton/IconButton.styles.ts(3 hunks)design-system/IconButton/IconButton.tsx(3 hunks)design-system/IconButton/animated-icon-button.tsx(1 hunks)design-system/Text/Text.styles.ts(1 hunks)features/conversation-list/conversation-list.screen-header.tsx(3 hunks)features/conversation/conversation-message/conversation-message-content-types/conversation-message-simple-text.tsx(1 hunks)features/onboarding/Privy/usePrivySmartWalletConnection.ts(1 hunks)features/onboarding/components/onboarding-contact-card-provider.tsx(1 hunks)features/onboarding/components/onboarding-contact-card.tsx(1 hunks)features/onboarding/components/onboarding-editable-avatar.tsx(1 hunks)features/onboarding/components/onboarding-footer-text.tsx(1 hunks)features/onboarding/components/onboarding-footer.tsx(1 hunks)features/onboarding/components/onboarding-icon-button.tsx(1 hunks)features/onboarding/components/onboarding-notification-row.tsx(1 hunks)features/onboarding/components/onboarding-subtitle.tsx(1 hunks)features/onboarding/components/onboarding-title.tsx(1 hunks)features/onboarding/constants/animationConstants.ts(1 hunks)features/onboarding/hooks/useAddPfp.ts(1 hunks)features/onboarding/hooks/useCreateOrUpdateProfileInfo.ts(1 hunks)features/onboarding/hooks/useProfile.ts(1 hunks)features/onboarding/passkey/passkey.constants.ts(1 hunks)features/onboarding/passkey/passkeyAuthStore.tsx(1 hunks)features/onboarding/passkey/useCreatePasskey.ts(1 hunks)features/onboarding/passkey/useLoginWithPasskey.ts(1 hunks)features/onboarding/screens/OnboardingConnectWalletScreen.tsx(2 hunks)features/onboarding/screens/OnboardingEphemeraScreen.tsx(2 hunks)features/onboarding/screens/OnboardingPrivateKeyScreen.tsx(2 hunks)features/onboarding/screens/OnboardingPrivyScreen.tsx(2 hunks)features/onboarding/screens/onboarding-contact-card-screen.tsx(1 hunks)features/onboarding/screens/onboarding-notifications-screen.tsx(1 hunks)features/onboarding/screens/onboarding-weclome-screen.tsx(1 hunks)features/onboarding/types/onboarding.types.ts(1 hunks)features/onboarding/utils/formatRandomUserName.ts(1 hunks)i18n/translations/en.ts(2 hunks)i18n/translations/fr.ts(1 hunks)ios/Converse/Converse.entitlements(1 hunks)metro.config.js(2 hunks)navigation/OnboardingNavigator.tsx(2 hunks)package.json(6 hunks)screens/Main.tsx(0 hunks)screens/Navigation/Navigation.tsx(6 hunks)screens/NewAccount/NewAccountConnectWalletScreen.tsx(2 hunks)screens/NewAccount/NewAccountEphemeraScreen.tsx(2 hunks)screens/NewAccount/NewAccountPrivateKeyScreen.tsx(2 hunks)screens/NewAccount/NewAccountPrivyScreen.tsx(2 hunks)screens/NewAccount/NewAccountScreen.tsx(0 hunks)screens/NewAccount/NewAccountUserProfileScreen.tsx(0 hunks)screens/NewAccount/new-account-contact-card-screen.tsx(1 hunks)screens/NewAccount/new-account-screen.tsx(1 hunks)screens/Onboarding/OnboardingGetStartedScreen.tsx(0 hunks)screens/Onboarding/OnboardingNotificationsScreen.tsx(0 hunks)screens/Onboarding/OnboardingUserProfileScreen.tsx(0 hunks)screens/UserProfileScreen.tsx(3 hunks)scripts/build/ios/preview.js(1 hunks)scripts/build/ios/production.js(1 hunks)theme/border-radius.ts(1 hunks)theme/spacing.ts(1 hunks)theme/timing.ts(1 hunks)tsconfig.json(1 hunks)utils/api/profiles.ts(1 hunks)utils/events.ts(1 hunks)utils/getCapitalizedLettersForAvatar.ts(1 hunks)utils/getFirstLetterForAvatar.ts(0 hunks)utils/navigation.ts(1 hunks)utils/str.ts(1 hunks)utils/xmtpRN/signIn.ts(2 hunks)utils/xmtpRN/signer.ts(2 hunks)
💤 Files with no reviewable changes (7)
- screens/Main.tsx
- utils/getFirstLetterForAvatar.ts
- screens/NewAccount/NewAccountScreen.tsx
- screens/Onboarding/OnboardingNotificationsScreen.tsx
- screens/Onboarding/OnboardingGetStartedScreen.tsx
- screens/NewAccount/NewAccountUserProfileScreen.tsx
- screens/Onboarding/OnboardingUserProfileScreen.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- tsconfig.json
- utils/navigation.ts
- scripts/build/ios/preview.js
- ios/Converse/Converse.entitlements
- features/onboarding/passkey/passkey.constants.ts
- app.config.ts
- components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx
- metro.config.js
- utils/xmtpRN/signer.ts
- scripts/build/ios/production.js
- utils/events.ts
- components/TransactionPreview/TransactionPreview.tsx
- .github/workflows/build-internal.yml
- features/onboarding/passkey/useCreatePasskey.ts
- features/onboarding/passkey/useLoginWithPasskey.ts
- screens/Navigation/Navigation.tsx
- package.json
🧰 Additional context used
🪛 GitHub Check: lint
App.tsx
[warning] 176-176:
Inline style: { flex: 1 }
config.ts
[warning] 39-39:
Prefer named exports
🪛 GitHub Check: tsc
features/onboarding/Privy/usePrivySmartWalletConnection.ts
[failure] 84-84:
Property 'defaultChain' does not exist on type '{ appId: string; clientId: string; } | { readonly appId: string; readonly clientId: string; }'.
🪛 GitHub Actions: Typescript CI
features/onboarding/Privy/usePrivySmartWalletConnection.ts
[error] 84-84: Property 'defaultChain' does not exist on type '{ appId: string; clientId: string; } | { readonly appId: string; readonly clientId: string; }'
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (42)
features/onboarding/components/onboarding-subtitle.tsx (2)
5-7: Well-structured style implementation!Good practices observed:
- Using
$prefix for style constants- Proper TypeScript typing with
TextStyle- Clear, single-responsibility style object
1-14: Overall excellent implementation!The component is well-structured, properly typed, and follows React best practices. The use of memo for performance optimization and the clean separation of styles demonstrate good engineering practices.
features/onboarding/screens/OnboardingPrivyScreen.tsx (2)
6-19: LGTM! Import paths properly reorganized.The import path updates reflect a better organized directory structure with clear separation of components, features, and utilities.
37-37: Verify navigation flow consistency across onboarding screens.The navigation target has been updated to "OnboardingContactCard". Let's ensure this change is consistent with other onboarding screens.
✅ Verification successful
Navigation flow is consistent across onboarding screens
All onboarding screens correctly navigate to "OnboardingContactCard" when the Converse profile is missing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check navigation targets in other onboarding screens # Expected: All screens should navigate to "OnboardingContactCard" when profile is missing # Search for navigation patterns in onboarding screens rg -A 2 'isMissingConverseProfile\(\).*\{' features/onboarding/screens/Length of output: 1366
utils/xmtpRN/signIn.ts (1)
12-17: LGTM! Imports are properly organized.The new imports for Viem integration are correctly added and organized.
components/Onboarding/init-xmtp-client.ts (2)
9-20: LGTM! Imports are properly organized.The imports have been updated to use absolute paths and include necessary new functionality.
73-91: LGTM! Proper error handling and cleanup.The implementation maintains security best practices with:
- Proper validation of required parameters
- Comprehensive error handling
- Cleanup on failure using logoutAccount
features/onboarding/screens/OnboardingPrivateKeyScreen.tsx (1)
43-43: Navigation flow update looks good.The navigation to "OnboardingContactCard" aligns with the PR's objective of updating the onboarding workflow. The change is properly integrated into the existing conditional flow and includes error handling.
features/onboarding/screens/OnboardingEphemeraScreen.tsx (1)
41-43: Verify navigation consistency across onboarding screens.Let's ensure that the navigation flow changes are consistent across all onboarding-related screens.
✅ Verification successful
Navigation changes are consistent across onboarding screens
All onboarding screens consistently use the centralized
isMissingConverseProfilecheck and navigate to "OnboardingContactCard" when needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check navigation patterns in onboarding screens # Expected: All relevant screens should navigate to "OnboardingContactCard" when profile is missing # Search for navigation patterns in onboarding screens echo "Checking navigation patterns in onboarding screens..." rg -A 2 'router\.navigate.*OnboardingContactCard|router\.navigate.*OnboardingUserProfile' 'features/onboarding' # Search for isMissingConverseProfile usage echo "Checking profile check usage..." rg 'isMissingConverseProfile' 'features/onboarding'Length of output: 2513
design-system/IconButton/animated-icon-button.tsx (1)
1-8: Clean implementation of animated button wrapper!The implementation is well-structured, properly typed, and follows the single responsibility principle by cleanly wrapping the existing IconButton with animation capabilities.
design-system/IconButton/IconButton.props.ts (1)
10-10: LGTM! Clean type extension.The addition of the "xl" size option maintains consistency with the existing type pattern.
design-system/IconButton/IconButton.tsx (1)
125-125: LGTM! Proper ref forwarding to Icon component.The ref is correctly forwarded to the Icon component.
design-system/IconButton/IconButton.styles.ts (1)
138-140: Review xl icon size scaling.The xl size is using the same fontSize as lg (spacing.lg). Consider if xl icons should scale larger for better visual hierarchy.
features/onboarding/constants/animationConstants.ts (1)
1-9: Verify animation timing in the onboarding flow.Since this is part of the Privy Passkeys feature, ensure these animation timings provide a smooth user experience during the authentication flow.
Let's check for other animation-related constants that might need coordination:
✅ Verification successful
Animation timing implementation is consistent and well-structured
The animation constants are properly implemented and consistently used across the onboarding flow. The sequential delays (200ms, 250ms, 300ms) create a smooth cascading effect, while the consistent duration ensures visual harmony throughout the authentication experience.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other animation timing constants in the onboarding feature rg -t typescript "timing|duration|delay" features/onboarding/Length of output: 98
Script:
#!/bin/bash # Search for animation-related constants and timing usage rg -t ts -t tsx "timing\[|duration|delay|animation" features/ # Find files importing these constants rg -t ts -t tsx "from.*animationConstants" # Check usage of timing object across the codebase rg -t ts -t tsx "timing\[.+\]"Length of output: 225
Script:
#!/bin/bash # Search for animation-related constants and timing usage without file type restrictions rg "timing\[|ONBOARDING_ENTERING" # Find animation-related patterns rg "duration|delay|animation" features/onboarding/ # Find imports of timing object rg "import.*timing.*from.*theme"Length of output: 7764
features/conversation/conversation-message/conversation-message-content-types/conversation-message-simple-text.tsx (1)
36-36: LGTM! Correctly updated to use the new text size.The change maintains the original visual appearance while adopting the new naming convention.
features/onboarding/passkey/passkeyAuthStore.tsx (4)
4-22: Well-structured type definitions!The type definitions provide good type safety and follow TypeScript best practices. The separation between props and state is clean and logical.
50-51: Improve error handling in setLoadingThe
setLoadingimplementation clears errors when loading is true but not when it's false. This asymmetric error handling could lead to stale error states.setLoading: (loading) => - loading ? set({ loading, error: undefined }) : set({ loading: false }), + set({ loading, error: undefined }),
60-76: Well-implemented hooks with proper error handling!The hooks follow React best practices, provide descriptive error messages, and maintain type safety.
52-53: Review the automatic clearing of statusString in setError.The current implementation of
setErrorautomatically clearsstatusString. Consider whether this is always the desired behavior, as there might be cases where you want to preserve the status while showing an error.✅ Verification successful
The automatic clearing of statusString in setError is the correct behavior
The current implementation ensures clean state transitions by preventing the simultaneous display of both error and status messages. This is particularly important in authentication flows where showing outdated status messages alongside errors could be misleading to users.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of setError to verify if clearing statusString is always appropriate rg -A 5 "setError" --type typescriptLength of output: 71
Script:
#!/bin/bash # Search for setError in TypeScript and TypeScript React files rg "setError" -t ts -t tsx -A 5 # Search for statusString to understand its usage patterns rg "statusString" -t ts -t tsx -A 5 # Use ast-grep to find the store's structure ast-grep --pattern 'create<$_>($_)({ $$$ setError: $_, $$$ })'Length of output: 194
Script:
#!/bin/bash # Find relevant files first fd "passkey.*\.(ts|tsx)$" # Search for setError and statusString usage rg "setError" -B 2 -A 2 # Search for statusString patterns rg "statusString" -B 2 -A 2 # Search for store creation pattern rg "create.*\({" -B 1 -A 10Length of output: 11244
design-system/Icon/Icon.types.ts (1)
68-69: LGTM! New icons align with passkey feature.The addition of "biometric" and "camera" icons aligns well with the PR objectives for implementing Privy passkey login support.
design-system/Icon/Icon.android.tsx (1)
74-75: LGTM! Platform-appropriate icon mappings.The mappings are well-chosen:
- Using "fingerprint" for biometric is appropriate for Android
- Using "camera" maintains consistency with the icon name
features/onboarding/components/onboarding-title.tsx (1)
9-21: LGTM! Well-structured component with good practices.The component follows React best practices with proper memoization, type safety, and clean composition.
theme/border-radius.ts (1)
6-6: LGTM! Consistent with existing theme pattern.The new
lgsize follows the established progression and maintains type safety.features/onboarding/components/onboarding-footer-text.tsx (1)
6-10: LGTM! Well-structured themed styles.The themed style implementation properly uses the spacing system and maintains consistency.
screens/NewAccount/NewAccountPrivyScreen.tsx (2)
16-16: LGTM! Import path update aligns with feature reorganization.The import path change for
isMissingConverseProfilereflects proper organization of onboarding-related features.
34-34: Verify navigation flow change impact.The navigation target has been changed from UserProfile to ContactCard. This appears to be part of a larger refactor of the onboarding flow.
Let's verify the navigation flow changes across the codebase:
✅ Verification successful
Navigation flow changes are consistent
The navigation target change from UserProfile to ContactCard has been properly implemented across all relevant screens. The old UserProfile name only remains in an internal function name which doesn't affect the navigation flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other navigation changes to ensure consistency rg -A 2 "router\.navigate\(\"NewAccountContactCard\"\)" # Search for any remaining references to the old navigation target rg "NewAccountUserProfile"Length of output: 1164
Script:
#!/bin/bash # Check the contact card screen implementation cat screens/NewAccount/new-account-contact-card-screen.tsx # Search for any navigation calls to UserProfile rg "router\.navigate\(\"NewAccountUserProfile\"\)"Length of output: 9036
components/Avatar.tsx (1)
5-5: LGTM! Enhanced avatar initials display.The switch to
getCapitalizedLettersForAvatarimproves user representation by showing multiple initials instead of just the first letter.Also applies to: 25-25
features/onboarding/components/onboarding-footer.tsx (1)
36-39: Remove debug menu trigger before production.The comment "don't anticipate this will be here long" suggests this is temporary debug code. Consider removing the debug menu trigger before merging to production.
Let's check for other debug-related code:
screens/NewAccount/NewAccountPrivateKeyScreen.tsx (2)
11-15: LGTM! Import reorganization aligns with feature structure.The import path changes properly organize onboarding-related features and screens.
32-32: Navigation change consistent with new flow.The navigation target change to ContactCard aligns with the changes in NewAccountPrivyScreen.tsx, maintaining consistency in the onboarding flow.
utils/api/profiles.ts (1)
8-8: Verify consistent usage of ProfileType import across the codebase.The import path change reflects a code reorganization moving types to the features directory. Let's ensure all files using ProfileType are updated.
✅ Verification successful
Import path change for ProfileType is consistently applied
All files importing ProfileType are using the new path
@/features/onboarding/types/onboarding.types, and the type is properly defined in its new location. No remaining references to the old import path were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining imports from the old path rg "from.*@/screens/Onboarding/OnboardingUserProfileScreen.*ProfileType" # Search for consistency in new imports rg "from.*@/features/onboarding/types/onboarding\.types.*ProfileType"Length of output: 146
Script:
#!/bin/bash # Find all occurrences of ProfileType rg "ProfileType" -A 2 # Find type definition ast-grep --pattern 'type ProfileType = { $$$ }' # Find interface definition (alternative syntax) ast-grep --pattern 'interface ProfileType { $$$ }'Length of output: 3420
features/onboarding/hooks/useCreateOrUpdateProfileInfo.ts (1)
76-105: 🛠️ Refactor suggestionEnhance avatar handling with type checking and size limits.
The avatar handling logic needs additional validation:
- No type checking for the avatar input
- No file size limits
- No error handling for the compression process
let publicAvatar = ""; if (profile.avatar) { + const MAX_AVATAR_SIZE = 5 * 1024 * 1024; // 5MB + if (profile.avatar instanceof File && profile.avatar.size > MAX_AVATAR_SIZE) { + setErrorMessage(translate("userProfile.errors.avatarTooLarge")); + return { + success: false, + errorMessage: translate("userProfile.errors.avatarTooLarge"), + }; + } if (profile.avatar.startsWith("https://")) { publicAvatar = profile.avatar; } else { - const resizedImage = await compressAndResizeImage( - profile.avatar, - true - ); + try { + const resizedImage = await compressAndResizeImage( + profile.avatar, + true + ); + if (!resizedImage?.uri) { + throw new Error("Failed to compress image"); + } publicAvatar = await uploadFile({ account: address, filePath: resizedImage.uri, contentType: "image/jpeg", }); + } catch (e: any) { + logger.error(e, { context: "UserProfile: compressing profile pic" }); + setErrorMessage("Failed to process image"); + setLoading(false); + return { + success: false, + errorMessage: "Failed to process image", + }; + }features/conversation-list/conversation-list.screen-header.tsx (1)
84-84: Verify avatar loading performance.The
usePreferredAvatarUrihook is called in the render cycle. Consider implementing loading states and error handling for avatar loading failures.- const avatarUri = usePreferredAvatarUri(currentAccount!); + const { uri: avatarUri, loading: avatarLoading, error: avatarError } = usePreferredAvatarUri(currentAccount!); // In the Avatar component: <Avatar name={preferredName} uri={avatarUri} size={theme.avatarSize.sm} + loading={avatarLoading} + onError={() => logger.warn("Failed to load avatar", { account: currentAccount })} />Also applies to: 116-120
screens/NewAccount/new-account-contact-card-screen.tsx (1)
127-129: Inconsistent navigation logic between real and rando flows.The navigation logic differs between real and rando profiles:
- Real profiles: Checks for notifications permissions
- Rando profiles: Always navigates to "Chats"
This inconsistency might lead to a confusing user experience.
Let's verify if this is intentional:
Also applies to: 146-152
✅ Verification successful
Navigation logic difference is intentional
The different navigation flows are by design:
- Real profiles require proper onboarding including notification setup as part of the user experience
- Rando profiles are meant to be lightweight temporary accounts with streamlined creation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar navigation patterns rg -A 5 'navigation\.popTo\("Chats"\)' rg -A 5 'needToShowNotificationsPermissions'Length of output: 9453
i18n/translations/fr.ts (1)
3-42: LGTM! Well-structured and complete French translations.The translations are well-organized, maintain consistency with English translations, and follow proper French grammar and punctuation.
config.ts (1)
Line range hint
42-44: Resolve chain ID inconsistency.The default config uses Base Sepolia (0x14a34) while the prod config uses Base Mainnet (0x2105). This inconsistency could lead to confusion and potential issues.
Run this script to check for any remaining references to Base Sepolia:
Also applies to: 122-124
🧰 Tools
🪛 GitHub Check: lint
[warning] 39-39:
Prefer named exports.github/workflows/build-app-release.yml (2)
Line range hint
9-13: LGTM! Removed scheduled triggers to match current release schedule.The removal of scheduled triggers aligns with the PR objectives to update builds.
53-53: LGTM! Properly configured Privy client ID for both platforms.The environment variable
EXPO_PUBLIC_PRIVY_CLIENT_IDis correctly added to both Android and iOS builds, using GitHub's secure secrets mechanism.Also applies to: 101-101
App.tsx (2)
13-13: LGTM! Added smart wallet support and updated theme imports.The changes properly integrate Privy's smart wallet functionality and update theme imports to fix the crash mentioned in the PR objectives.
Also applies to: 19-19
164-187: LGTM! Properly configured providers with correct nesting order.The providers are correctly configured and nested in a logical order:
- PrivyProvider with required clientId and supportedChains
- SmartWalletsProvider for smart wallet support
- Remaining providers maintaining their original order
🧰 Tools
🪛 GitHub Check: lint
[warning] 176-176:
Inline style: { flex: 1 }i18n/translations/en.ts (2)
3-39: LGTM! Well-structured onboarding translations.The new onboarding section is comprehensive and well-organized, with clear and user-friendly messages for:
- Welcome screen
- Contact card setup
- Notification preferences
144-148: LGTM! Added required passkey translations.The passkey section includes all necessary strings for the new Privy passkey login feature.
| (theme: Theme) => // TODO: fix once we fixed IconProps | ||
| // : Partial<IIconProps> | ||
| { | ||
| const { colors, spacing } = theme; | ||
|
|
||
| if (action === "primary") { | ||
| switch (variant) { | ||
| case "fill": | ||
| props.color = colors.text.inverted.primary; | ||
| break; | ||
|
|
||
| case "outline": | ||
| case "subtle": | ||
| case "ghost": | ||
| props.color = colors.text.primary; | ||
| break; | ||
|
|
||
| default: | ||
| break; | ||
| } | ||
| } | ||
| const props: any = | ||
| // :Partial<IIconProps> | ||
| {}; | ||
|
|
||
| return props; | ||
| // Set icon size | ||
| const sizeMap = { | ||
| sm: spacing.xs, | ||
| md: spacing.sm, | ||
| lg: spacing.lg, | ||
| xl: spacing.lg, | ||
| }; | ||
|
|
||
| props.size = sizeMap[size]; | ||
|
|
||
| if (disabled && variant !== "fill") { | ||
| props.color = colors.text.tertiary; | ||
| return props; | ||
| } | ||
|
|
||
| if (action === "primary") { | ||
| switch (variant) { | ||
| case "fill": | ||
| props.color = colors.text.inverted.primary; | ||
| break; | ||
|
|
||
| case "outline": | ||
| case "subtle": | ||
| case "ghost": | ||
| props.color = colors.text.primary; | ||
| break; | ||
|
|
||
| default: | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return props; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address TODO comments and improve typing.
The implementation has multiple TODO comments and uses any type. Consider:
- Fixing the IconProps type as mentioned in the TODO
- Removing the commented-out type annotations
- Replacing
anywith proper typing
- (theme: Theme) => // TODO: fix once we fixed IconProps
- // : Partial<IIconProps>
+ (theme: Theme): IconProps =>
{
const { colors, spacing } = theme;
- const props: any =
- // :Partial<IIconProps>
- {};
+ const props: Partial<IconProps> = {};Committable suggestion skipped: line range outside the PR's diff.
| ["3xl"]: { fontSize: 48, lineHeight: 60 } satisfies TextStyle, | ||
| xxl: { fontSize: 40, lineHeight: 44 } satisfies TextStyle, |
There was a problem hiding this comment.
💡 Codebase verification
Update onboarding title component to use new size
The text size changes will affect the onboarding title component. Since "xxl" is now smaller (40/44 instead of 48/60), you should update features/onboarding/components/onboarding-title.tsx to use the new "3xl" size if you want to maintain the current visual dimensions.
🔗 Analysis chain
Verify all existing "xxl" usages across the codebase.
The text size changes look good and follow common design system practices. However, since "xxl" now has different dimensions (40/44 instead of 48/60), we should ensure all existing usages are updated appropriately.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usages of textSizeStyles.xxl that might need updating
rg -l 'textSizeStyles\.xxl|textSizeStyles\[.xxl.\]' --type typescript
Length of output: 106
Script:
#!/bin/bash
# Search for "xxl" text size usages in TypeScript and JavaScript files
rg -t ts -t tsx -t js -t jsx -A 2 '(textSizeStyles\.xxl|textSizeStyles\[.xxl.\]|size=["\']xxl["\']|size:\s*["\']xxl["\'])'
Length of output: 116
Script:
#!/bin/bash
# Search for direct property access of xxl
echo "=== Searching for textSizeStyles.xxl ==="
rg -t ts -t tsx -t js -t jsx 'textSizeStyles\.xxl'
echo -e "\n=== Searching for bracket notation access of xxl ==="
rg -t ts -t tsx -t js -t jsx 'textSizeStyles\[.(xxl|"xxl"|'\''xxl'\'').\]'
echo -e "\n=== Searching for size prop with xxl ==="
rg -t ts -t tsx -t js -t jsx 'size=["\x27]xxl["\x27]'
Length of output: 578
Script:
#!/bin/bash
# Find all TypeScript/JavaScript files and search for xxl usages
fd -e ts -e tsx -e js -e jsx | xargs rg -l '(textSizeStyles\.xxl|textSizeStyles\[.xxl.\]|size=["\x27]xxl["\x27]|size:\s*["\x27]xxl["\x27])'
Length of output: 193
Script:
#!/bin/bash
# View the usage context of xxl in the onboarding title component
rg -A 3 -B 3 '(textSizeStyles\.xxl|textSizeStyles\[.xxl.\]|size=["\x27]xxl["\x27]|size:\s*["\x27]xxl["\x27])' features/onboarding/components/onboarding-title.tsx
Length of output: 293
| export const formatRandomUserName = (displayName: string) => { | ||
| const randomUserGuid = v4().replace(/-/g, "").slice(0, 30); | ||
| const profileUserName = ( | ||
| displayName?.replace(/\s+/g, "") + randomUserGuid | ||
| ).slice(0, 30); | ||
| return profileUserName; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and handle edge cases.
The function needs defensive programming to handle edge cases:
displayNamecould be undefined, null, or empty string- Current implementation might generate usernames shorter than expected if displayName is empty
Consider this implementation:
export const formatRandomUserName = (displayName: string) => {
+ if (!displayName?.trim()) {
+ const randomId = v4().replace(/-/g, "").slice(0, 30);
+ return `user${randomId}`.slice(0, 30);
+ }
const randomUserGuid = v4().replace(/-/g, "").slice(0, 30);
const profileUserName = (
displayName?.replace(/\s+/g, "") + randomUserGuid
).slice(0, 30);
return profileUserName;
};📝 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.
| export const formatRandomUserName = (displayName: string) => { | |
| const randomUserGuid = v4().replace(/-/g, "").slice(0, 30); | |
| const profileUserName = ( | |
| displayName?.replace(/\s+/g, "") + randomUserGuid | |
| ).slice(0, 30); | |
| return profileUserName; | |
| }; | |
| export const formatRandomUserName = (displayName: string) => { | |
| if (!displayName?.trim()) { | |
| const randomId = v4().replace(/-/g, "").slice(0, 30); | |
| return `user${randomId}`.slice(0, 30); | |
| } | |
| const randomUserGuid = v4().replace(/-/g, "").slice(0, 30); | |
| const profileUserName = ( | |
| displayName?.replace(/\s+/g, "") + randomUserGuid | |
| ).slice(0, 30); | |
| return profileUserName; | |
| }; |
| const randomUsername = uuidv4().replace(/-/g, "").slice(0, 30); | ||
| logger.debug( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance username generation security and error handling.
The random username generation using UUID could be improved:
- Using UUID for usernames might create unnecessarily long strings
- The error handling in
handleRandoContinuecould be more specific
Consider this implementation:
+const generateRandomUsername = (length: number = 12): string => {
+ return Math.random()
+ .toString(36)
+ .substring(2, 2 + length);
+};
const handleRandoContinue = useCallback(async () => {
try {
- const randomUsername = uuidv4().replace(/-/g, "").slice(0, 30);
+ const randomUsername = generateRandomUsername();
logger.debug(
"[OnboardingContactCardScreen] handleRandoContinue",
randoDisplayName
);
const { success } = await createOrUpdateProfile({
profile: {
displayName: randoDisplayName,
username: randomUsername,
},
});
// ... rest of the code
} catch (error) {
- handleError(error as Error);
+ if (error instanceof Error) {
+ handleError(error);
+ } else {
+ handleError(new Error('Unknown error during profile creation'));
+ }
}
}, [randoDisplayName, createOrUpdateProfile, navigation, handleError]);Also applies to: 110-132
| } catch (error) { | ||
| handleError(error as Error); | ||
| } | ||
| }, [createOrUpdateProfile, profile, router, handleError, asset?.uri]); | ||
|
|
||
| const handleContinue = useCallback(() => { | ||
| logger.debug("[NewAccountContactCardScreen] handleContinue", type); | ||
| if (type === "real") { | ||
| handleRealContinue(); | ||
| } else { | ||
| handleRandoContinue(); | ||
| } | ||
| }, [type, handleRealContinue, handleRandoContinue]); | ||
|
|
||
| return ( | ||
| <Screen | ||
| preset="scroll" | ||
| contentContainerStyle={$flex1} | ||
| safeAreaEdges={["bottom"]} | ||
| > | ||
| <Center style={$centerContainerStyle}> | ||
| <VStack style={$flex1}> | ||
| {type === "real" ? ( | ||
| <OnboardingTitle entering={titleAnimation} size={"xl"}> | ||
| {translate("onboarding.contactCard.title")} | ||
| </OnboardingTitle> | ||
| ) : ( | ||
| <OnboardingTitle entering={titleAnimation} size={"xl"}> | ||
| {translate("onboarding.contactCard.randoTitle")} | ||
| </OnboardingTitle> | ||
| )} | ||
| {type === "real" ? ( | ||
| <OnboardingSubtitle | ||
| style={themed($subtitleStyle)} | ||
| entering={subtitleAnimation} | ||
| > | ||
| {translate("onboarding.contactCard.subtitle")} | ||
| </OnboardingSubtitle> | ||
| ) : ( | ||
| <OnboardingSubtitle | ||
| style={themed($subtitleStyle)} | ||
| entering={subtitleAnimation} | ||
| > | ||
| {translate("onboarding.contactCard.randoSubtitle")} | ||
| </OnboardingSubtitle> | ||
| )} | ||
| <OnboardingContactCardThemeProvider> | ||
| {type === "real" ? ( | ||
| <OnboardingContactCard | ||
| addPFP={addPFP} | ||
| pfpUri={asset?.uri} | ||
| displayName={profile.displayName} | ||
| setDisplayName={(displayName) => | ||
| setProfile({ ...profile, displayName }) | ||
| } | ||
| /> | ||
| ) : ( | ||
| <OnboardingContactCard | ||
| editable={false} | ||
| addPFP={() => {}} | ||
| pfpUri={undefined} | ||
| displayName={randoDisplayName} | ||
| setDisplayName={() => {}} | ||
| /> | ||
| )} | ||
| </OnboardingContactCardThemeProvider> | ||
|
|
||
| <AnimatedVStack | ||
| entering={animation | ||
| .fadeInDownSlow() | ||
| .delay(ONBOARDING_ENTERING_DELAY.THIRD) | ||
| .duration(ONBOARDING_ENTERING_DURATION)} | ||
| > | ||
| <AnimatedText style={$subtextStyle} color={"secondary"}> | ||
| {type === "real" | ||
| ? translate("onboarding.contactCard.body") | ||
| : translate("onboarding.contactCard.randoBody")} | ||
| </AnimatedText> | ||
| <Pressable onPress={toggleType}> | ||
| <AnimatedText | ||
| style={$subtextPressableStyle} | ||
| color={"secondary"} | ||
| > | ||
| {type === "real" | ||
| ? translate("onboarding.contactCard.bodyPressable") | ||
| : translate("onboarding.contactCard.randoPressable")} | ||
| </AnimatedText> | ||
| </Pressable> | ||
| </AnimatedVStack> | ||
| </VStack> | ||
| </Center> | ||
| <OnboardingFooter | ||
| text={translate("onboarding.contactCard.continue")} | ||
| iconName="chevron.right" | ||
| onPress={handleContinue} | ||
| disabled={loading || (type === "real" && !profile.displayName)} | ||
| /> | ||
| </Screen> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting shared logic to avoid code duplication.
This component shares significant code with OnboardingContactCardScreen. Consider creating a shared hook or component to reduce duplication.
Create a custom hook:
function useContactCardLogic({
onSuccess,
navigation,
}: {
onSuccess: (type: "real" | "rando") => void;
navigation: any;
}) {
const [type, setType] = useState<"real" | "rando">("real");
// ... shared state and handlers
return {
type,
toggleType,
handleContinue: () => {
if (type === "real") {
handleRealContinue();
} else {
handleRandoContinue();
}
},
// ... other shared logic
};
}Then use it in both components:
function NewAccountContactCardScreen() {
const logic = useContactCardLogic({
onSuccess: (type) => {
if (type === "real") {
// Navigate to notifications
} else {
navigation.popTo("Chats");
}
},
navigation,
});
// ... render UI using logic
}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
app.json(1 hunks)package.json(8 hunks)polyfills.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app.json
- package.json
🔇 Additional comments (1)
polyfills.ts (1)
1-4: Verify if the import order is correct for Privy integration.The comment states "DO NOT CHANGE THE ORDER OF THESE IMPORTS" and the new import has been placed in the first group. We should verify if this placement is correct for Privy Passkeys functionality.
Let's check Privy's documentation and usage patterns:
✅ Verification successful
The import order is correct for Privy integration
The current order is appropriate as WalletConnect's compatibility layer should be loaded first, followed by more generic polyfills like fast-text-encoding. This ordering ensures core functionality is available before other polyfills are applied.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Privy-related polyfill requirements rg -i "privy.*polyfill|polyfill.*privy" -g "*.md" -g "package.json" # Look for any initialization or setup patterns in Privy-related code ast-grep --pattern 'import { Privy $_ } from "$_"'Length of output: 120
Script:
#!/bin/bash # Search for WalletConnect polyfill requirements rg -i "walletconnect.*polyfill|polyfill.*walletconnect" -g "*.md" -g "*.ts" -g "*.js" # Look for any React Native polyfill patterns rg -i "react.?native.*polyfill|polyfill.*react.?native" -g "*.md" -g "*.ts" -g "*.js" # Search for any setup or initialization files fd -e ts -e js -e md -g "*setup*" -g "*init*" -g "*config*" # Check package.json for related dependencies cat package.json | jq '.dependencies, .devDependencies'Length of output: 8477
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
App.tsx (3)
8-10: Document Privy's development requirements more formally.The comment indicates a specific requirement for Privy to work, but it's not clear why this is needed or what the implications are. Consider:
- Adding this information to the project's README or development documentation
- Explaining why
--no-dev --minifyflags are required- Documenting any potential impact on the development workflow
Would you like me to help create a documentation section for Privy integration requirements?
171-171: Consider future chain support requirements.Currently, only the Base chain is supported (
supportedChains={[base]}). Consider:
- Whether other chains will be needed in the future
- Making the supported chains list configurable through environment variables
- Adding documentation about supported chains
Would you like me to propose a more flexible chain configuration approach?
173-189: Document provider hierarchy dependencies.The nesting order of providers (
PrivyProvider > SmartWalletsProvider > ThirdwebProvider) is crucial for proper functionality. Consider adding documentation that explains:
- Why this specific order is required
- Dependencies between providers
- Potential issues if the order is changed
Would you like me to help create a provider architecture documentation section?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
App.tsx(3 hunks)app.json(1 hunks)package.json(7 hunks)polyfills.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app.json
- polyfills.ts
- package.json
🔇 Additional comments (1)
App.tsx (1)
167-172: Verify secure handling of Privy credentials.The
clientIdis being passed through config. Ensure that:
- The environment variable
PRIVY_CLIENT_IDis properly set in CI/CD- The value is not exposed in client-side logs
- Error handling exists for missing credentials
Let's verify the environment variable handling:
✅ Verification successful
Privy credentials are properly handled ✅
The implementation follows secure patterns:
- Uses Expo's public environment variables convention
- Configured correctly in CI/CD workflows
- No unsafe logging detected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if PRIVY_CLIENT_ID is properly configured in workflow files rg -l "PRIVY_CLIENT_ID" .github/workflows/ # Check for potential credential logging rg -i "privy.*client.*id" -g "!*.md" -g "!*.env*"Length of output: 352
alexrisch
force-pushed
the
ar/privy-passkey
branch
2 times, most recently
from
e75436c to
b2c158f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
navigation/OnboardingNavigator.tsx (1)
Line range hint
15-28: Remove unused screen type from OnboardingParamList.The
OnboardingGetStartedscreen type is defined but its corresponding screen component has been removed from the navigator. This could lead to runtime errors if navigation to this screen is attempted.Apply this diff to remove the unused type:
type OnboardingParamList = { OnboardingWelcome: undefined; OnboardingContactCard: undefined; - OnboardingGetStarted: undefined; OnboardingPrivy: undefined; OnboardingConnectWallet: { address: string; }; OnboardingNotifications: undefined; OnboardingPrivateKey: undefined; OnboardingEphemeral: undefined; };
♻️ Duplicate comments (2)
utils/xmtpRN/signIn.ts (1)
74-121: 🛠️ Refactor suggestionReduce code duplication with existing client creation function.
The implementation is correct but duplicates most of the logic from
createXmtpClientFromSigner. A previous review already suggested extracting the common initialization logic into a shared function.Please refer to the previous review comment for the suggested refactoring approach that extracts the common initialization logic into a shared function.
components/Onboarding/init-xmtp-client.ts (1)
55-91: 🛠️ Refactor suggestionReduce code duplication in XMTP client initialization.
The implementation duplicates significant logic from
initXmtpClient. A previous review already suggested extracting the common initialization logic.Please refer to the previous review comment for the suggested refactoring approach that extracts the common initialization logic into a shared function.
🧹 Nitpick comments (24)
i18n/translations/fr.ts (2)
14-15: Consider combining split text for better maintainability.The
bodyPressabletext is unnecessarily split across two lines. Consider combining them into a single line:- bodyPressable: - "ou optez pour le mode Rando pour plus de confidentialité.", + bodyPressable: "ou optez pour le mode Rando pour plus de confidentialité.",
31-32: Consider using more formal language for consistency.The phrase "Me notifier" is more casual. Consider using a more formal alternative that matches the tone of the rest of the UI:
- "Me notifier lorsque mes contacts approuvés m'envoient des messages", + "Être notifié(e) lorsque mes contacts approuvés m'envoient des messages",utils/xmtpRN/signIn.ts (1)
91-91: Update log message to reflect Viem account usage.The log message incorrectly states "from signer" when it's actually creating from a Viem account.
- logger.debug("Instantiating client from signer"); + logger.debug("Instantiating client from Viem account");components/Onboarding/init-xmtp-client.ts (1)
63-65: Fix error message to reference Viem account.The error message incorrectly refers to a signer instead of a Viem account.
- throw new Error("No signer"); + throw new Error("No Viem account provided");design-system/IconButton/IconButton.styles.ts (2)
138-140: Maintain size progression for "xl" icons.The "xl" size uses the same fontSize (spacing.lg) as "md" size, breaking the size progression pattern. Consider using a larger fontSize for "xl" to maintain visual hierarchy.
xl: { - fontSize: spacing.lg, + fontSize: spacing.xxl, },
192-197: Maintain size progression for icon size mapping.The "xl" size uses the same value (spacing.lg) as "lg" size in the size mapping. This breaks the visual progression of icon sizes.
const sizeMap = { sm: spacing.xs, md: spacing.sm, lg: spacing.lg, - xl: spacing.lg, + xl: spacing.xl, };features/onboarding/components/onboarding-editable-avatar.tsx (2)
13-19: Consider using theme border radius constant.The style implementation looks good, but consider using a theme constant for the border radius instead of a magic number.
const $placeholderStyle: ThemedStyle<ViewStyle> = (theme) => ({ - borderRadius: 9999, + borderRadius: theme.borderRadius.full, // or similar theme constant width: theme.avatarSize.xxl, height: theme.avatarSize.xxl, backgroundColor: theme.colors.fill.minimal, borderWidth: theme.borderWidth.sm, });
21-24: Add JSDoc documentation.Add documentation to describe the component's purpose, props, and usage examples.
+/** + * A component that renders an editable avatar for the onboarding flow. + * Shows a camera placeholder when no image is provided. + * + * @param {string} [uri] - The URI of the avatar image + * @param {string} [name] - The name to use for the avatar fallback + * + * @example + * <OnboardingEditableAvatar + * uri="https://example.com/avatar.jpg" + * name="John Doe" + * /> + */ export const OnboardingEditableAvatar = ({screens/Navigation/Navigation.tsx (1)
209-218: Consider adding screen transition animations for a smoother UX.The new screens have
headerShown: false, but no explicit transition animations are defined. Consider adding custom transitions for a more polished user experience.options={{ headerShown: false, + animation: 'slide_from_right', + animationDuration: 200, }}Also applies to: 277-283
navigation/OnboardingNavigator.tsx (1)
7-13: Standardize import path styles.The import statements mix relative (
../features/) and absolute (@/features/) paths. Consider standardizing to absolute imports for better maintainability.Apply this diff to standardize the imports:
-import { OnboardingConnectWalletScreen } from "../features/onboarding/screens/OnboardingConnectWalletScreen"; -import { OnboardingEphemeraScreen } from "../features/onboarding/screens/OnboardingEphemeraScreen"; -import { OnboardingNotificationsScreen } from "../features/onboarding/screens/onboarding-notifications-screen"; -import { OnboardingPrivateKeyScreen } from "../features/onboarding/screens/OnboardingPrivateKeyScreen"; -import { OnboardingPrivyScreen } from "../features/onboarding/screens/OnboardingPrivyScreen"; +import { OnboardingConnectWalletScreen } from "@/features/onboarding/screens/OnboardingConnectWalletScreen"; +import { OnboardingEphemeraScreen } from "@/features/onboarding/screens/OnboardingEphemeraScreen"; +import { OnboardingNotificationsScreen } from "@/features/onboarding/screens/onboarding-notifications-screen"; +import { OnboardingPrivateKeyScreen } from "@/features/onboarding/screens/OnboardingPrivateKeyScreen"; +import { OnboardingPrivyScreen } from "@/features/onboarding/screens/OnboardingPrivyScreen";features/onboarding/passkey/useLoginWithPasskey.ts (2)
1-31: Consider consolidating authentication stores.The hook currently depends on two separate stores (
PasskeyAuthStoreandPrivyAuthStore) which could lead to synchronization issues and increased complexity. Consider consolidating authentication-related state into a single store for better maintainability.
36-65: Improve error handling specificity.The catch block uses a generic
anytype for the error. Consider:
- Using a more specific error type
- Adding specific error handling for different failure scenarios (e.g., network errors, invalid credentials)
- } catch (e: any) { + } catch (e) { + if (e instanceof PrivyError) { + setError(e.message); + } else if (e instanceof NetworkError) { + setError("Network connection failed"); + } else { + setError("Unknown error occurred during login"); + } captureErrorWithToast(e); }features/onboarding/passkey/passkeyAuthStore.tsx (1)
56-56: Consider granular reset functionality.The current
reset()implementation resets all state to default values. Consider adding more granular reset functions for specific state subsets or allowing selective state reset.- reset: () => set(DEFAULT_PROPS), + reset: (keys?: (keyof IPasskeyAuthStoreProps)[]) => + set(keys ? + keys.reduce((acc, key) => ({ ...acc, [key]: DEFAULT_PROPS[key] }), {}) : + DEFAULT_PROPS + ),screens/NewAccount/new-account-screen.tsx (3)
63-87: Enhance error handling and logging.
- Consider adding more context to error messages for better debugging
- Debug logging should be conditionally enabled only in development
const handleError = useCallback( (error: Error) => { logger.error(error); - setError(error.message); + setError(`Account creation failed: ${error.message}`); captureErrorWithToast(error); }, [setError] ); const onStatusChange = useCallback((status: string) => { - logger.debug("[NewAccountWelcomeScreenContent] onStatusChange", status); + if (__DEV__) { + logger.debug("[NewAccountWelcomeScreenContent] onStatusChange", status); + } }, []);
88-93: Add retry mechanism for connection failures.Consider implementing a retry mechanism for failed wallet connections to improve reliability.
const MAX_RETRIES = 3; const RETRY_DELAY = 1000; const onConnectionError = useCallback( async (error: Error, retryCount = 0) => { if (retryCount < MAX_RETRIES) { setTimeout(() => { // Retry connection handleCreateAccountWithPasskey().catch(e => onConnectionError(e, retryCount + 1) ); }, RETRY_DELAY * (retryCount + 1)); } else { handleError(error); } }, [handleError, handleCreateAccountWithPasskey] );
131-136: Enhance loading state feedback.Consider adding a loading indicator or progress feedback during the connection process to improve user experience.
<OnboardingFooter text={translate("onboarding.welcome.createContactCard")} iconName="biometric" onPress={handleCreateAccountWithPasskey} disabled={loading} + loading={loading} + loadingText={translate("onboarding.welcome.connecting")} />features/onboarding/screens/OnboardingPrivyScreen.tsx (1)
6-19: Consider using path aliases to simplify import statementsThe updated import paths use multiple
../segments, which can make the code harder to read and maintain. Configuring path aliases can simplify these imports and enhance code readability.For example, you can set up path aliases in your
tsconfig.json:{ "compilerOptions": { "baseUrl": ".", "paths": { "@components/*": ["src/components/*"], "@data/*": ["src/data/*"], "@features/*": ["src/features/*"], "@navigation/*": ["src/navigation/*"], "@styles/*": ["src/styles/*"] } } }Then, update your import statements:
-import { OnboardingPictoTitleSubtitle } from "../../../components/Onboarding/OnboardingPictoTitleSubtitle"; +import { OnboardingPictoTitleSubtitle } from "@components/Onboarding/OnboardingPictoTitleSubtitle";screens/UserProfileScreen.tsx (2)
Line range hint
104-111: Add visual feedback during profile updates.Consider showing a loading state or progress indicator when the profile is being updated.
<Button variant="text" + loading={loading} title={ profile?.avatar ? translate("userProfile.buttons.changeProfilePicture") : translate("userProfile.buttons.addProfilePicture") } onPress={addPFP} />
157-235: Enhance style organization and color management.Consider these improvements:
- Extract platform-specific styles into separate objects
- Define color constants for better maintainability
const COLORS = { danger: dangerColor, text: { primary: textPrimaryColor, secondary: textSecondaryColor, }, separator: itemSeparatorColor, } as const; const platformStyles = { android: { profileInput: { paddingHorizontal: 16, borderWidth: 1, borderRadius: 4, }, // ... other Android styles }, ios: { // ... iOS styles }, } as const;features/onboarding/screens/onboarding-notifications-screen.tsx (5)
29-32: Consider using theme-based spacing.Instead of hardcoding the margin value, consider using theme-based spacing for better maintainability and consistency across the app.
const $screenContainer: ViewStyle = { flex: 1, - marginHorizontal: 16, + marginHorizontal: theme.spacing.md, // or appropriate theme spacing value };
57-61: Document Android-specific behavior.The Android-specific handling needs better documentation to explain the workflow and potential edge cases.
- // Android 13 always show denied first but sometimes - // it will still show the popup. If not, go to Settings! + // On Android 13+, the system initially returns "denied" status even when + // the permission dialog hasn't been shown yet. We need to: + // 1. Check if it's a genuine denial (user clicked "Deny") + // 2. Open system settings if the user needs to manually enable permissions + // Reference: [Add link to Android documentation or relevant issue]
72-76: Consider persisting notification preferences.The
isEssentialsEnabledstate is not persisted. Consider saving this preference to ensure it survives app restarts.
108-123: Improve UX for disabled notification options.The disabled notification options (
mentionsOnlyandcash) might confuse users. Consider:
- Adding tooltips or explanations for why they're disabled
- Adding a "Coming Soon" badge
- Removing them if they're not yet ready for production
85-92: Extract inline styles to constants.Move inline styles to named constants for better maintainability:
+const $avatarContainer: ViewStyle = { + marginHorizontal: 8, +}; + +const $subtitleContainer: ViewStyle = { + marginTop: 16, + marginBottom: 24, +}; <Avatar - style={{ marginHorizontal: 8 }} + style={$avatarContainer} size={theme.avatarSize.sm} uri={avatarUri} name={displayName} /> <OnboardingSubtitle - style={{ marginTop: 16, marginBottom: 24 }} + style={$subtitleContainer} >Also applies to: 97-99
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (81)
.github/workflows/build-app-release.yml(2 hunks).github/workflows/build-internal.yml(1 hunks)App.tsx(3 hunks)app.config.ts(1 hunks)components/Avatar.tsx(2 hunks)components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx(1 hunks)components/Onboarding/init-xmtp-client.ts(2 hunks)components/TransactionPreview/TransactionPreview.tsx(2 hunks)components/__tests__/Avatar.test.tsx(1 hunks)config.ts(5 hunks)design-system/Icon/Icon.android.tsx(1 hunks)design-system/Icon/Icon.tsx(1 hunks)design-system/Icon/Icon.types.ts(1 hunks)design-system/IconButton/IconButton.props.ts(1 hunks)design-system/IconButton/IconButton.styles.ts(3 hunks)design-system/IconButton/IconButton.tsx(3 hunks)design-system/IconButton/animated-icon-button.tsx(1 hunks)design-system/Text/Text.styles.ts(1 hunks)features/conversation/conversation-message/conversation-message-content-types/conversation-message-simple-text.tsx(1 hunks)features/onboarding/Privy/usePrivySmartWalletConnection.ts(1 hunks)features/onboarding/components/onboarding-contact-card-provider.tsx(1 hunks)features/onboarding/components/onboarding-contact-card.tsx(1 hunks)features/onboarding/components/onboarding-editable-avatar.tsx(1 hunks)features/onboarding/components/onboarding-footer-text.tsx(1 hunks)features/onboarding/components/onboarding-footer.tsx(1 hunks)features/onboarding/components/onboarding-icon-button.tsx(1 hunks)features/onboarding/components/onboarding-notification-row.tsx(1 hunks)features/onboarding/components/onboarding-subtitle.tsx(1 hunks)features/onboarding/components/onboarding-title.tsx(1 hunks)features/onboarding/constants/animationConstants.ts(1 hunks)features/onboarding/hooks/useAddPfp.ts(1 hunks)features/onboarding/hooks/useCreateOrUpdateProfileInfo.ts(1 hunks)features/onboarding/hooks/useProfile.ts(1 hunks)features/onboarding/passkey/passkey.constants.ts(1 hunks)features/onboarding/passkey/passkeyAuthStore.tsx(1 hunks)features/onboarding/passkey/useCreatePasskey.ts(1 hunks)features/onboarding/passkey/useLoginWithPasskey.ts(1 hunks)features/onboarding/screens/OnboardingConnectWalletScreen.tsx(2 hunks)features/onboarding/screens/OnboardingEphemeraScreen.tsx(2 hunks)features/onboarding/screens/OnboardingPrivateKeyScreen.tsx(2 hunks)features/onboarding/screens/OnboardingPrivyScreen.tsx(2 hunks)features/onboarding/screens/onboarding-contact-card-screen.tsx(1 hunks)features/onboarding/screens/onboarding-notifications-screen.tsx(1 hunks)features/onboarding/screens/onboarding-weclome-screen.tsx(1 hunks)features/onboarding/types/onboarding.types.ts(1 hunks)features/onboarding/utils/formatRandomUserName.ts(1 hunks)i18n/translations/en.ts(2 hunks)i18n/translations/fr.ts(1 hunks)ios/Converse/Converse.entitlements(1 hunks)metro.config.js(2 hunks)navigation/OnboardingNavigator.tsx(2 hunks)package.json(7 hunks)polyfills.ts(1 hunks)screens/Main.tsx(0 hunks)screens/Navigation/Navigation.tsx(6 hunks)screens/NewAccount/NewAccountConnectWalletScreen.tsx(2 hunks)screens/NewAccount/NewAccountEphemeraScreen.tsx(2 hunks)screens/NewAccount/NewAccountPrivateKeyScreen.tsx(2 hunks)screens/NewAccount/NewAccountPrivyScreen.tsx(2 hunks)screens/NewAccount/NewAccountScreen.tsx(0 hunks)screens/NewAccount/NewAccountUserProfileScreen.tsx(0 hunks)screens/NewAccount/new-account-contact-card-screen.tsx(1 hunks)screens/NewAccount/new-account-screen.tsx(1 hunks)screens/Onboarding/OnboardingGetStartedScreen.tsx(0 hunks)screens/Onboarding/OnboardingNotificationsScreen.tsx(0 hunks)screens/Onboarding/OnboardingUserProfileScreen.tsx(0 hunks)screens/UserProfileScreen.tsx(3 hunks)scripts/build/ios/preview.js(1 hunks)scripts/build/ios/production.js(1 hunks)theme/border-radius.ts(1 hunks)theme/spacing.ts(1 hunks)theme/timing.ts(1 hunks)tsconfig.json(1 hunks)utils/api/profiles.ts(1 hunks)utils/events.ts(1 hunks)utils/getCapitalizedLettersForAvatar.ts(1 hunks)utils/getFirstLetterForAvatar.ts(0 hunks)utils/navigation.ts(1 hunks)utils/str.ts(1 hunks)utils/xmtpRN/signIn.ts(2 hunks)utils/xmtpRN/signer.ts(2 hunks)
💤 Files with no reviewable changes (7)
- screens/Main.tsx
- screens/NewAccount/NewAccountScreen.tsx
- utils/getFirstLetterForAvatar.ts
- screens/NewAccount/NewAccountUserProfileScreen.tsx
- screens/Onboarding/OnboardingGetStartedScreen.tsx
- screens/Onboarding/OnboardingUserProfileScreen.tsx
- screens/Onboarding/OnboardingNotificationsScreen.tsx
🚧 Files skipped from review as they are similar to previous changes (57)
- theme/border-radius.ts
- tsconfig.json
- theme/spacing.ts
- features/conversation/conversation-message/conversation-message-content-types/conversation-message-simple-text.tsx
- utils/api/profiles.ts
- design-system/Icon/Icon.tsx
- features/onboarding/components/onboarding-icon-button.tsx
- components/tests/Avatar.test.tsx
- design-system/IconButton/IconButton.props.ts
- design-system/Icon/Icon.types.ts
- ios/Converse/Converse.entitlements
- features/onboarding/types/onboarding.types.ts
- utils/xmtpRN/signer.ts
- features/onboarding/components/onboarding-title.tsx
- features/onboarding/components/onboarding-contact-card-provider.tsx
- scripts/build/ios/preview.js
- features/onboarding/screens/OnboardingConnectWalletScreen.tsx
- design-system/IconButton/animated-icon-button.tsx
- features/onboarding/constants/animationConstants.ts
- features/onboarding/passkey/passkey.constants.ts
- .github/workflows/build-internal.yml
- features/onboarding/utils/formatRandomUserName.ts
- utils/str.ts
- theme/timing.ts
- design-system/Icon/Icon.android.tsx
- polyfills.ts
- components/Onboarding/ConnectViaWallet/ConnectViaWalletTableViewItems.tsx
- design-system/IconButton/IconButton.tsx
- components/Avatar.tsx
- screens/NewAccount/NewAccountConnectWalletScreen.tsx
- metro.config.js
- utils/events.ts
- utils/getCapitalizedLettersForAvatar.ts
- features/onboarding/components/onboarding-footer-text.tsx
- screens/NewAccount/NewAccountPrivyScreen.tsx
- design-system/Text/Text.styles.ts
- features/onboarding/components/onboarding-footer.tsx
- features/onboarding/hooks/useProfile.ts
- features/onboarding/screens/onboarding-contact-card-screen.tsx
- .github/workflows/build-app-release.yml
- features/onboarding/screens/OnboardingEphemeraScreen.tsx
- features/onboarding/components/onboarding-notification-row.tsx
- features/onboarding/hooks/useCreateOrUpdateProfileInfo.ts
- components/TransactionPreview/TransactionPreview.tsx
- screens/NewAccount/NewAccountPrivateKeyScreen.tsx
- app.config.ts
- features/onboarding/Privy/usePrivySmartWalletConnection.ts
- features/onboarding/components/onboarding-contact-card.tsx
- features/onboarding/hooks/useAddPfp.ts
- features/onboarding/passkey/useCreatePasskey.ts
- package.json
- features/onboarding/screens/OnboardingPrivateKeyScreen.tsx
- utils/navigation.ts
- i18n/translations/en.ts
- features/onboarding/components/onboarding-subtitle.tsx
- scripts/build/ios/production.js
- features/onboarding/screens/onboarding-weclome-screen.tsx
🧰 Additional context used
🪛 GitHub Check: lint
App.tsx
[warning] 179-179:
Inline style: { flex: 1 }
config.ts
[warning] 39-39:
Prefer named exports
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (32)
i18n/translations/fr.ts (2)
3-43: LGTM! Well-structured and grammatically correct French translations.The new
onboardingsection is well-organized with proper nesting and complete translations for all UI elements. The French translations are grammatically correct and maintain a consistent tone throughout the section.
Line range hint
1-999: LGTM! Complete and well-structured translations.The French translations file is well-organized, complete, and maintains proper hierarchical structure. All the features mentioned in the AI summary are properly translated, and the translations maintain consistency in tone and style throughout the file.
utils/xmtpRN/signIn.ts (1)
12-17: LGTM! Import changes align with new Viem account support.The new imports for
ViemAccountandviemAccountToXmtpSignerare appropriately added to support the new functionality.components/Onboarding/init-xmtp-client.ts (2)
9-20: LGTM! Import organization and new dependencies.The imports are well-organized using absolute paths, and the new dependencies for Viem account support are correctly added.
73-90: LGTM! Error handling and logging implementation.The error handling follows the established pattern, including proper cleanup with
logoutAccountand error logging.design-system/IconButton/IconButton.styles.ts (1)
182-190: Address TODO comments and improve typing.The implementation has multiple TODO comments and uses
anytype. Consider:
- Fixing the IconProps type as mentioned in the TODO
- Removing the commented-out type annotations
- Replacing
anywith proper typing- ( - theme: Theme // TODO: fix once we fixed IconProps - ) => // : Partial<IIconProps> + (theme: Theme): Partial<IconProps> => { const { colors, spacing } = theme; - const props: any = - // :Partial<IIconProps> - {}; + const props: Partial<IconProps> = {};features/onboarding/components/onboarding-editable-avatar.tsx (2)
1-11: LGTM! Clean imports and well-defined types.The imports are well-organized and the type definition is clear with proper optional props.
21-43:⚠️ Potential issueAdd missing functionality for editable avatar.
The component is named "Editable" but lacks the following essential features:
- Press handler for editing
- Loading state during image upload
- Error handling for invalid URIs
- Accessibility support
Here's how to implement these features:
export const OnboardingEditableAvatar = ({ uri, name, + onEdit, + isLoading, }: IOnboardingEditableAvatarProps) => { const { theme, themed } = useAppTheme(); + const [hasError, setHasError] = useState(false); + if (!uri && !name) { return ( - <Center style={themed($placeholderStyle)}> + <Pressable + onPress={onEdit} + accessibilityRole="button" + accessibilityLabel="Add profile picture" + accessibilityHint="Opens image picker" + > + <Center style={themed($placeholderStyle)}> + {isLoading ? ( + <ActivityIndicator color={theme.colors.text.primary} /> + ) : ( <Icon icon="camera" size={theme.iconSize.md} color={theme.colors.text.primary} /> + )} </Center> + </Pressable> ); } return ( - <VStack style={{ width: theme.avatarSize.xxl }}> + <Pressable + onPress={onEdit} + accessibilityRole="button" + accessibilityLabel={`Edit profile picture${name ? ` for ${name}` : ''}`} + accessibilityHint="Opens image picker" + > + <VStack style={{ width: theme.avatarSize.xxl }}> <Avatar - uri={uri} + uri={hasError ? undefined : uri} name={name} size={theme.avatarSize.xxl} + onError={() => setHasError(true)} /> + {isLoading && ( + <Center + style={[ + themed($placeholderStyle), + { position: 'absolute', top: 0, left: 0 } + ]} + > + <ActivityIndicator color={theme.colors.text.primary} /> + </Center> + )} </VStack> + </Pressable> ); };Don't forget to update the type definition:
type IOnboardingEditableAvatarProps = { uri?: string; name?: string; + onEdit: () => void; + isLoading?: boolean; };Likely invalid or redundant comment.
screens/Navigation/Navigation.tsx (2)
47-49: LGTM! Clean organization of new screen imports.The new screen imports are well-organized and follow the established pattern.
55-57: LGTM! Navigation params are properly typed.The new route definitions are correctly added to the NavigationParamList type.
Also applies to: 69-69
screens/NewAccount/NewAccountEphemeraScreen.tsx (1)
19-19: LGTM! Navigation flow updated correctly.The import path and navigation target have been properly updated to reflect the new screen structure.
Also applies to: 38-38
screens/NewAccount/new-account-contact-card-screen.tsx (1)
1-247: Consider extracting shared logic to avoid code duplication.This component shares significant code with
OnboardingContactCardScreen. Consider creating a shared hook or component to reduce duplication.navigation/OnboardingNavigator.tsx (2)
12-12: Fix typo in import path.The import path contains a typo:
onboarding-weclome-screenshould beonboarding-welcome-screen.
50-70: Verify the onboarding flow sequence.The screen order has been modified with the introduction of new screens. Please confirm that this sequence aligns with the intended user experience for Privy passkey login:
- Welcome
- Contact Card
- Notifications
- Privy
- Connect Wallet
- Private Key
- Ephemeral
Let's check for any potential navigation dead-ends or circular references:
✅ Verification successful
Navigation flow implementation verified successfully
The screen sequence in the navigator matches the implemented navigation flow, with proper transitions between screens. The Welcome screen correctly navigates to Contact Card, and all screens have the necessary navigation capabilities implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check navigation patterns in onboarding screens # Look for navigation.navigate calls to verify the flow echo "Checking navigation patterns in onboarding screens..." rg -A 2 "navigation\.navigate\([\'\"]Onboarding" --type typescript # Look for potential dead-ends (screens without outgoing navigation) echo "Checking for potential dead-ends..." for file in $(fd "Onboarding.*Screen\.tsx$"); do if ! rg -q "navigation\.navigate" "$file"; then echo "Potential dead-end in $file" fi doneLength of output: 1648
Script:
#!/bin/bash # Search for navigation patterns with corrected regex echo "Checking navigation patterns..." rg "navigation\.(navigate|replace|reset|goBack)" --type typescript features/onboarding/ # Check for navigation-related imports and custom hooks echo -e "\nChecking navigation imports and hooks..." rg "import.*navigation" --type typescript features/onboarding/ # Look for button components that might handle navigation echo -e "\nChecking for button components..." rg "(Button|Pressable|TouchableOpacity|onPress)" --type typescript features/onboarding/Length of output: 602
Script:
#!/bin/bash # Search for navigation patterns in all relevant files echo "Checking navigation patterns..." rg "navigation\.(navigate|replace|reset|goBack)" features/onboarding/screens/ # Check for navigation-related imports and hooks echo -e "\nChecking navigation imports and hooks..." rg "import.*navigation" features/onboarding/screens/ # Look for button components and onPress handlers echo -e "\nChecking for interactive components..." rg "(Button|Pressable|TouchableOpacity)" features/onboarding/screens/ # Look for specific screen transitions echo -e "\nChecking screen transitions..." rg "Onboarding[A-Za-z]+Screen" features/onboarding/screens/ -A 2 -B 2Length of output: 10260
features/onboarding/passkey/useLoginWithPasskey.ts (2)
32-34: LGTM!The effect correctly synchronizes the login state with the UI.
67-70: LGTM!The hook's return value is correctly implemented and well-documented.
features/onboarding/passkey/passkeyAuthStore.tsx (2)
50-51: Improve error handling in setLoading.The
setLoadingimplementation clears errors when loading is true but not when it's false. This asymmetric error handling could lead to stale error states.
60-76: LGTM!The hook implementations follow React context best practices with proper error handling.
screens/NewAccount/new-account-screen.tsx (1)
43-51: Reduce code duplication with OnboardingWelcomeScreen.The
NewAccountScreenand its content component are nearly identical toOnboardingWelcomeScreen. Consider extracting the common functionality into a shared component.features/onboarding/screens/OnboardingPrivyScreen.tsx (1)
37-37: Navigation update aligns with onboarding flow changesThe navigation target is correctly updated to
"OnboardingContactCard", aligning with the restructured onboarding process to enhance user experience.screens/UserProfileScreen.tsx (3)
Line range hint
3-28: LGTM! Well-organized imports.The imports are properly organized and use consistent path conventions.
156-156: Improve type safety inuseUserProfileStyles.The function uses
anytypes for its parameters, which could lead to runtime errors.
Line range hint
89-102: Add validation for avatar URI and loading state.The avatar handling lacks input validation and loading state feedback:
- Validate the avatar URI format and size before setting it in the profile
- Show a loading indicator during avatar upload
<Pressable onPress={addPFP}> <Avatar uri={profile?.avatar || asset?.uri} style={styles.avatar} + loading={asset?.loading} name={profile.displayName || profile.username} /> </Pressable>features/onboarding/screens/onboarding-notifications-screen.tsx (3)
1-28: LGTM! Well-organized imports.The imports are logically grouped and all dependencies are properly declared.
52-70: Improve error handling inhandleEnableNotifications.The error handling needs enhancement as previously suggested.
34-40: Add runtime validation for currentAccount.The non-null assertion (
!) oncurrentAccountcould lead to runtime errors if the account is unexpectedly null. Consider adding a runtime check or error boundary.- const currentAccount = useCurrentAccount()!; + const currentAccount = useCurrentAccount(); + if (!currentAccount) { + // Handle the null case appropriately + return null; // or show an error state + }config.ts (3)
39-39: Remove duplicated privy config in prod environment.The privy configuration is duplicated between defaultConfig and prod environment.
🧰 Tools
🪛 GitHub Check: lint
[warning] 39-39:
Prefer named exports
79-79: Verify domain configuration consistency.The new
appDomainvalues match their respectivewebsiteDomainvalues. This is good for consistency, but please ensure:
- DNS records are properly configured for all domains
- SSL certificates are valid
Also applies to: 99-99, 134-134
Line range hint
42-44: Verify chain configuration for production readiness.The configuration properly distinguishes between Base Sepolia (testnet) for development and Base Mainnet for production. However, please ensure:
- All smart contract interactions are tested on Base Mainnet
- Contract addresses are verified on both chains
Also applies to: 122-124
✅ Verification successful
Chain configuration and contract addresses verified successfully
Both USDC contract addresses are properly deployed and operational on their respective networks:
- Base Sepolia (testnet): 0x036CbD53842c5426634e7929541eC2318f3dCF7e
- Base Mainnet: 0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify contract addresses on both chains echo "Checking Base Sepolia USDC contract..." curl -s -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_getCode","params":["0x036CbD53842c5426634e7929541eC2318f3dCF7e", "latest"],"id":1}' "https://sepolia.base.org" | jq .result echo "Checking Base Mainnet USDC contract..." curl -s -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_getCode","params":["0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913", "latest"],"id":1}' "https://mainnet.base.org" | jq .resultLength of output: 7913
🧰 Tools
🪛 GitHub Check: lint
[warning] 39-39:
Prefer named exportsApp.tsx (3)
7-10: Document minification requirement in build documentation.The comment indicates a critical requirement for Privy to work with minification. This should be documented in the build process.
Would you like me to help create a documentation PR or issue to properly document this requirement?
167-190: Provider configuration and nesting looks good!The provider hierarchy is correct:
- PrivyProvider with all required props (appId, clientId, storage, supportedChains)
- SmartWalletsProvider nested within PrivyProvider
- Remaining providers maintain their original order
🧰 Tools
🪛 GitHub Check: lint
[warning] 179-179:
Inline style: { flex: 1 }
55-55: Verify chain configuration consistency with config.ts.The Base chain configuration in PrivyProvider should match the chain configuration in config.ts based on the environment.
Also applies to: 171-171
✅ Verification successful
Chain configuration is properly configured ✅
The chain configuration is correctly implemented with environment-specific settings:
- Development/Preview environments use Base Sepolia (0x14a34)
- Production environment uses Base Mainnet (0x2105)
The
basechain import in App.tsx is consistent with this setup as it's used in the PrivyProvider configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for chain configurations to ensure consistency echo "Checking chain configurations..." rg -A 2 "transactionChainId|supportedChains"Length of output: 2435
Script:
#!/bin/bash # Check base chain usage and environment configuration echo "Checking base chain configuration..." rg -A 5 "import.*base.*from.*viem/chains" echo -e "\nChecking environment-based configuration..." rg -A 10 "export const config = {" config.tsLength of output: 1191
Script:
#!/bin/bash # Check configuration structure with fixed regex echo "Checking config structure..." rg -A 10 "export const config =" config.ts echo -e "\nChecking environment variables usage..." rg -B 2 -A 2 "process\.env\." config.tsLength of output: 1874
Script:
#!/bin/bash # Get complete config structure and chain selection logic echo "Checking complete config file..." cat config.ts echo -e "\nChecking chain-related conditionals..." rg -B 5 -A 5 "transactionChainId.*Base"Length of output: 5746
|
|
||
| const handleRandoContinue = useCallback(async () => { | ||
| try { | ||
| const randomUsername = uuidv4().replace(/-/g, "").slice(0, 30); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve random username generation.
The current random username generation methods could be more robust:
- UUID-based generation might create usernames that are hard to read
- Using display name for username generation could lead to collisions
+const generateRandomUsername = (base?: string) => {
+ const randomSuffix = Math.random().toString(36).substring(2, 8);
+ if (base) {
+ const sanitized = base.toLowerCase().replace(/[^a-z0-9]/g, '');
+ return `${sanitized}_${randomSuffix}`.slice(0, 30);
+ }
+ return `user_${randomSuffix}`;
+};
-const randomUsername = uuidv4().replace(/-/g, "").slice(0, 30);
+const randomUsername = generateRandomUsername();
-const profileUserName = formatRandomUserName(profile.displayName ?? "");
+const profileUserName = generateRandomUsername(profile.displayName);Also applies to: 133-133
| const handleError = useCallback((error: Error) => { | ||
| logger.error(error); | ||
| captureErrorWithToast(error); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (errorMessage) { | ||
| handleError(new Error(errorMessage)); | ||
| } | ||
| }, [errorMessage, handleError]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling with specific error types.
The current error handling is too generic. Consider creating specific error types for different failure scenarios.
+type ProfileError = {
+ code: 'VALIDATION_ERROR' | 'NETWORK_ERROR' | 'UNKNOWN_ERROR';
+ message: string;
+};
-const handleError = useCallback((error: Error) => {
+const handleError = useCallback((error: Error | ProfileError) => {
logger.error(error);
- captureErrorWithToast(error);
+ if ('code' in error) {
+ captureErrorWithToast(error.message);
+ } else {
+ captureErrorWithToast('An unexpected error occurred');
+ }
}, []);Committable suggestion skipped: line range outside the PR's diff.
| const handleRealContinue = useCallback(async () => { | ||
| try { | ||
| const profileUserName = formatRandomUserName(profile.displayName ?? ""); | ||
| const newProfile: ProfileType = { | ||
| ...profile, | ||
| username: profileUserName, | ||
| avatar: asset?.uri, | ||
| }; | ||
| const { success } = await createOrUpdateProfile({ | ||
| profile: newProfile, | ||
| }); | ||
| if (success) { | ||
| navigation.popTo("Chats"); | ||
| } else { | ||
| throw new Error("Failed to create or update profile"); | ||
| } | ||
| } catch (error) { | ||
| handleError(error as Error); | ||
| } | ||
| }, [profile, asset?.uri, createOrUpdateProfile, navigation, handleError]); |
There was a problem hiding this comment.
Add input validation for profile data.
The profile update logic lacks validation for the display name and username format.
const handleRealContinue = useCallback(async () => {
try {
+ if (!profile.displayName?.trim()) {
+ throw { code: 'VALIDATION_ERROR', message: 'Display name is required' };
+ }
+ if (profile.displayName.length > 50) {
+ throw { code: 'VALIDATION_ERROR', message: 'Display name is too long' };
+ }
const profileUserName = formatRandomUserName(profile.displayName ?? "");
+ if (profileUserName.length < 3 || profileUserName.length > 30) {
+ throw { code: 'VALIDATION_ERROR', message: 'Invalid username length' };
+ }
const newProfile: ProfileType = {📝 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.
| const handleRealContinue = useCallback(async () => { | |
| try { | |
| const profileUserName = formatRandomUserName(profile.displayName ?? ""); | |
| const newProfile: ProfileType = { | |
| ...profile, | |
| username: profileUserName, | |
| avatar: asset?.uri, | |
| }; | |
| const { success } = await createOrUpdateProfile({ | |
| profile: newProfile, | |
| }); | |
| if (success) { | |
| navigation.popTo("Chats"); | |
| } else { | |
| throw new Error("Failed to create or update profile"); | |
| } | |
| } catch (error) { | |
| handleError(error as Error); | |
| } | |
| }, [profile, asset?.uri, createOrUpdateProfile, navigation, handleError]); | |
| const handleRealContinue = useCallback(async () => { | |
| try { | |
| if (!profile.displayName?.trim()) { | |
| throw { code: 'VALIDATION_ERROR', message: 'Display name is required' }; | |
| } | |
| if (profile.displayName.length > 50) { | |
| throw { code: 'VALIDATION_ERROR', message: 'Display name is too long' }; | |
| } | |
| const profileUserName = formatRandomUserName(profile.displayName ?? ""); | |
| if (profileUserName.length < 3 || profileUserName.length > 30) { | |
| throw { code: 'VALIDATION_ERROR', message: 'Invalid username length' }; | |
| } | |
| const newProfile: ProfileType = { | |
| ...profile, | |
| username: profileUserName, | |
| avatar: asset?.uri, | |
| }; | |
| const { success } = await createOrUpdateProfile({ | |
| profile: newProfile, | |
| }); | |
| if (success) { | |
| navigation.popTo("Chats"); | |
| } else { | |
| throw new Error("Failed to create or update profile"); | |
| } | |
| } catch (error) { | |
| handleError(error as Error); | |
| } | |
| }, [profile, asset?.uri, createOrUpdateProfile, navigation, handleError]); |
config.ts
Outdated
| @@ -77,6 +76,7 @@ const ENV = { | |||
| appCheckDebugToken: isAndroid | |||
| ? process.env.EXPO_PUBLIC_FIREBASE_APP_CHECK_DEBUG_TOKEN_ANDROID | |||
| : process.env.EXPO_PUBLIC_FIREBASE_APP_CHECK_DEBUG_TOKEN_IOS, | |||
| appDomain: "dev.converse.xyz", | |||
There was a problem hiding this comment.
Did this fix anything?
| const initializingXmtp = useRef(false); | ||
|
|
||
| // Let's make sure we start with a clean state | ||
| useEffect(() => { |
There was a problem hiding this comment.
We could clean up a lot of this state if we utilize React Query here. It would be a little odd in the sense that we would have to pass the client from usePrivy to the hook, since Privy doesn't look like it exposes any imperative APIs.
But it would take care of cleaning up a lot of the management around whether or not we're currently creating and we would get the status changes for free and catching errors would be taken care of for free. Then, on success, we could initialize XMTP.
I'm happy to take a look at this with you to clean things up once you've got a final iteration on Privy and we've consolidated and agreed upon using it, which it seems like we already have.
There was a problem hiding this comment.
Another spot this would clean up: #1511 (comment)
There was a problem hiding this comment.
I believe this is another spot. We react query would help us out and simplifying a lot of this code, and we would just have to extract the imperative function of actually updating or creating the profile info.
| @@ -0,0 +1,4 @@ | |||
| import config from "@config"; | |||
|
|
|||
| export const RPID = config.appDomain; | |||
There was a problem hiding this comment.
Could you maybe just add a link to documentation on what this is
There was a problem hiding this comment.
We should delete tie this and redo when we're adding transactions
There was a problem hiding this comment.
We are moving away from this pattern and I think this would go away completely if we use react query from managing these async operations
There was a problem hiding this comment.
I think we could use react-query instead of this store
| const { logout, user: privyUser } = usePrivy(); | ||
|
|
||
| const { signupWithPasskey, state: signupState } = useSignupWithPasskey(); | ||
|
|
There was a problem hiding this comment.
Some documentation: https://docs.privy.io/guide/expo/authentication/passkey
| @@ -25,7 +25,7 @@ export const OnboardingConnectWalletScreen = memo( | |||
|
|
|||
| const handleDoneConnecting = useCallback(() => { | |||
| if (isMissingConverseProfile()) { | |||
| router.navigate("OnboardingUserProfile"); | |||
| router.navigate("OnboardingContactCard"); | |||
There was a problem hiding this comment.
I feel like renaming this route to OnboardingCreateContactCard makes more sense because that's what the route screen is doing.
alexrisch
force-pushed
the
ar/privy-passkey
branch
from
616d55d to
f62df01
Compare
There was a problem hiding this comment.
- Make sure to name all files with lower-kebab.
- Please add comments in places that it's hard to understand why.
- I saw @technoplato saying we could use react-query at some places. I think I agree.
| @@ -49,6 +52,44 @@ export async function initXmtpClient(args: { | |||
| } | |||
| } | |||
|
|
|||
| export async function initXmtpClientFromViemAccount(args: { | |||
There was a problem hiding this comment.
I thought we wouldn't need this anymore. Because all identity would be from Passkeys/Privy and then we would only connect external wallet to this account.
There was a problem hiding this comment.
We need to create an XMTP signer in some way, it's expecting some way to do the signing
The signing is then handled within the app and doesn't need to move out, but it still needs something to authorize a signature
| const { themeScheme, setThemeContextOverride, ThemeProvider } = | ||
| useThemeProvider(); | ||
|
|
||
| const invertedTheme = themeScheme === "light" ? "dark" : "light"; |
There was a problem hiding this comment.
We have stuff like theme.colors.fill.inverted.light etc... no?
| @@ -0,0 +1,114 @@ | |||
| import { AnimatedVStack, VStack } from "@/design-system/VStack"; | |||
There was a problem hiding this comment.
Why not using Louis' contact-card.tsx ?
There was a problem hiding this comment.
Wasn't merged at the time, that card would also need to be updated to support editing as well, happy to make the update since it's been merged
| const $flex2: ViewStyle = { | ||
| flex: 2, | ||
| }; |
There was a problem hiding this comment.
Prefer $textsContainer or something like that
| @@ -0,0 +1,9 @@ | |||
| import { timing } from "@/theme/timing"; | |||
There was a problem hiding this comment.
lowercase kebab style name 🙏
There was a problem hiding this comment.
I think we could use react-query instead of this store
| const $flex1: ViewStyle = { | ||
| flex: 1, | ||
| }; |
There was a problem hiding this comment.
we have something you can use in theme/styles.ts for those utility style
|
|
||
| const handleError = useCallback( | ||
| (error: Error) => { | ||
| logger.error(error); |
There was a problem hiding this comment.
don't need if we have captureErrorWithToast no?
There was a problem hiding this comment.
Yeah was during debugging, but it was because we didn't log to sentry outside of dev
Added support for privy passkey login Added support for smart contract wallets provided by privy Updated app entitlements Updated builds for new env variables Updated packages for better support Fixed crash with app theme Removed noisy logs
Updated onboarding and new account experience Still a number of files to delete but there will be a bunch of conflicts so trying to avoid that for now
Privy for some reason requires to be imported as *
use theme animations Remove transaction frames work
alexrisch
force-pushed
the
ar/privy-passkey
branch
from
e3f942e to
ebd5140
Compare
Added support for privy passkey login
Added support for smart contract wallets provided by privy Updated app entitlements
Updated builds for new env variables
Updated packages for better support
Fixed crash with app theme
Removed noisy logs
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the concise release notes:
Release Notes
New Features
User Experience
Technical Improvements
Platform Support