Tryout wasm2js for controller-rs

[JunichiSugiura]

Ref: https://github.com/conduition/expo-wasm-demo

[claude]

Claude finished @JunichiSugiura's task —— View job

---

PR Review Complete ✅

I've completed a comprehensive review of this React Native/Expo PR that introduces wasm2js for controller-rs and switches from pnpm to bun.

Todo List:

• Check existing reviews and comments
• Create pending review and get PR diff
• Analyze key changed files for React Native/Expo best practices
• Add inline comments for specific issues found
• Submit comprehensive review
• Provide final summary

Review Summary:
The PR shows solid technical implementation with successful WASM integration and clean package manager migration. Key feedback focuses on:

• Code Quality: Refactor duplicated API logic in connect.tsx into reusable utilities
• User Experience: Replace Alert.alert with toast notifications for better mobile UX
• Type Safety: Improve TypeScript definitions for WASM interfaces and WebAuthn objects
• Performance: Monitor bundle size impact from new dependencies
• Testing: Consider adding unit tests and updating visual regression tests

The overall architecture is sound and the wasm2js integration is implemented correctly. The review provides specific recommendations for production readiness.

[claude]

React Native/Expo Code Review

This PR introduces significant changes including wasm2js integration for controller-rs and a migration from pnpm to bun. Overall, the implementation looks solid but there are several areas for improvement:

🚀 Positives

• Clean Expo Router Setup: Good use of Expo Router with proper stack navigation and screen configuration
• Package Manager Migration: The switch to bun with proper lock file management is well-executed
• WASM Integration: Successfully integrates WASM modules with proper Metro config
• TypeScript Configuration: Well-structured path mapping and module resolution

🔧 Areas for Improvement

Code Duplication in connect.tsx (lines 50-315)

The signup and login functions contain nearly identical JSON parsing and error handling logic. This should be refactored:

• Extract API response parsing into a reusable utility function
• Create a common GraphQL request helper
• Consider using a proper GraphQL client library for better error handling

Error Handling & User Experience

• connect.tsx line 334: Using Alert.alert for errors isn't optimal for React Native UX - consider using toast notifications
• connect.tsx line 322: Empty username validation could be more robust (trim, length checks)
• connect.tsx line 409: 500ms debounce for user existence checking is reasonable but could be configurable

Performance & Bundle Size

• metro.config.js line 10: Adding "wasm" to assetExts is correct for WASM support
• package.json line 16: The postinstall script runs controller build - ensure this doesn't significantly impact install times
• The large number of Radix UI dependencies might impact bundle size - consider tree-shaking

Type Safety Issues

• wasm2js.tsx line 8: Hardcoded string parameters in CartridgeAccount.new() should use proper types
• connect.tsx line 40: ChallengeOptions type is too generic - consider defining proper WebAuthn types
• tsconfig.json line 15: "noImplicitAny": false reduces type safety - consider enabling it gradually

React Native Best Practices

• connect.tsx line 414: Consider using KeyboardAvoidingView for better keyboard handling
• app/_layout.tsx line 17: Good use of GestureHandlerRootView but verify gesture handling is needed throughout the app
• connect.tsx line 435: StatusBar component should be configured consistently across all screens

🔒 Security & API Considerations

• src/utils/const.ts: Hardcoded API URL is fine for this use case
• connect.tsx: Passkey implementation follows good security practices
• Consider adding request timeout configurations for API calls

📱 Platform Compatibility

• app.json lines 40-42: Good addition of associatedDomains for iOS passkey support
• The React Native Web compatibility should be tested with the new WASM integration
• Ensure WASM modules work consistently across iOS/Android/Web platforms

🧪 Testing & Development

• No visual regression test updates included - may need screenshot regeneration
• Consider adding unit tests for the new authentication logic
• WASM integration should include error fallbacks for unsupported platforms

🎯 Recommendations

1. Refactor API Logic: Extract repeated GraphQL request patterns into reusable utilities
2. Improve Error UX: Replace Alert.alert with toast notifications for better user experience
3. Enhance Type Safety: Define proper types for WASM interfaces and WebAuthn objects
4. Add Error Boundaries: Wrap WASM components in error boundaries for graceful degradation
5. Consider Bundle Analysis: Monitor impact of new dependencies on app size

The overall architecture and approach are sound. The wasm2js integration is implemented correctly and the package manager migration is clean. Focus on addressing the code duplication and enhancing error handling for a production-ready implementation.