Repository navigation
fix(expo): Pass proxyUrl through to the native component SDKs - #9373
wobsoriano wants to merge 11 commits into
Conversation
🦋 Changeset detectedLatest commit: b21bc31 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-biometrics
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/mosaic
@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. |
…-url # Conflicts: # packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt # packages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsx # packages/expo/src/provider/nativeClientSync.tsx
This branch was successfully deployed
1 participant
Description
Native components ignored the
proxyUrlpassed to<ClerkProvider>. Only the publishable key crossed the bridge, so clerk-ios and clerk-android called the Frontend API domain derived from the key. On proxied instances that domain may not serve a certificate, so native components failed at TLS while the JS auth path worked.A new
configureWithOptionsbridge method passesproxyUrlto both native SDKs, and aproxyUrlchange reconfigures them. The existingconfiguremethod keeps its signature. JS falls back to it when the binary predates this change, so an OTA update is safe and the proxy applies after the next native build.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change