fix(expo): Pass proxyUrl through to the native component SDKs - #9373
fix(expo): Pass proxyUrl through to the native component SDKs#9373wobsoriano wants to merge 9 commits into
Conversation
🦋 Changeset detectedLatest commit: 718a067 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: 6 reviews are currently available. Based on recent review activity, included reviews refill at 10 per hour. 📝 WalkthroughWalkthroughExpo native configuration now accepts an optional Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change forwards the configured proxy URL to native authentication components; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/expo/src/provider/nativeClientSync.tsx (1)
1025-1080: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSet native readiness only after configuration succeeds.
Line 1025 sets
didAttemptConfigurebeforeClerkExpo.configure()resolves. If the native bridge rejects, Line 1080 still marks this configuration as ready.NativeClientSynccan then run while the native SDK still uses the previous proxy configuration.Set the success flag after
ClerkExpo.configure()resolves.Proposed fix
- let didAttemptConfigure = false; + let didConfigure = false; ... - didAttemptConfigure = true; await ClerkExpo.configure(publishableKey, initialJsDeviceToken, nativeProxyUrl); + didConfigure = true; ... - if (didAttemptConfigure && isCurrentConfiguration()) { + if (didConfigure && isCurrentConfiguration()) { setReadyConfigKey(configuringConfigKey); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/expo/src/provider/nativeClientSync.tsx` around lines 1025 - 1080, Move the `didAttemptConfigure = true` assignment in the configuration flow to immediately after `await ClerkExpo.configure(...)` resolves successfully. Keep the `finally` readiness update guarded by `didAttemptConfigure` and `isCurrentConfiguration()` so failed configuration attempts cannot mark the native configuration as ready.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsx`:
- Around line 196-209: Add rerender coverage to the native ClerkProvider
synchronization tests around the existing proxyUrl test: update proxyUrl, then
remove it, and await each synchronization before asserting. Verify native
configure receives the updated URL and the normalized null value, with each
expected configuration applied once and in order.
---
Outside diff comments:
In `@packages/expo/src/provider/nativeClientSync.tsx`:
- Around line 1025-1080: Move the `didAttemptConfigure = true` assignment in the
configuration flow to immediately after `await ClerkExpo.configure(...)`
resolves successfully. Keep the `finally` readiness update guarded by
`didAttemptConfigure` and `isCurrentConfiguration()` so failed configuration
attempts cannot mark the native configuration as ready.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: bfa7c768-e12c-4560-b118-aee0da42b1f8
📒 Files selected for processing (10)
.changeset/expo-native-proxy-url.mdpackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.ktpackages/expo/ios/ClerkExpoModule.swiftpackages/expo/ios/ClerkNativeBridge.swiftpackages/expo/src/provider/ClerkProvider.tsxpackages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsxpackages/expo/src/provider/nativeClientSync.tsxpackages/expo/src/specs/NativeClerkModule.android.tspackages/expo/src/specs/NativeClerkModule.tspackages/expo/src/utils/native-module.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
…-url # Conflicts: # packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt # packages/expo/src/provider/nativeClientSync.tsx
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 participant
Description
The native components ignored the
proxyUrlpassed to<ClerkProvider>. The bridgeconfigure()call only forwarded the publishable key, so clerk-ios and clerk-android derived the Frontend API origin from the key and called it directly. On proxy-configured instances that domain may not serve a certificate, so native components failed at TLS while the JS auth path worked.configure()now carriesproxyUrlacross the bridge intoClerk.Options(proxyUrl:)on iOS andClerkConfigurationOptions(proxyUrl = ...)on Android. Bootstrap and reconfigure are keyed on publishable key plus proxy URL, and both sides of the bridge ship together, so there is no version-skew risk.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change