fix(expo): link iOS SDK through podspec SPM dependencies#8927
Conversation
🦋 Changeset detectedLatest commit: ea697ef 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.
|
|
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), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Clerk iOS SDK is now linked via React Native's native SPM podspec support rather than manual Expo config plugin injection. ChangesiOS SPM Podspec Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/eslint-plugin
@clerk/expo
@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: |
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
🧹 Nitpick comments (1)
packages/expo/ios/ClerkExpo.podspec (1)
20-21: 💤 Low valueConsider externalizing the Clerk iOS SDK version.
The
clerk_ios_version = '1.2.4'is hardcoded in the podspec. Every Clerk iOS SDK update will require manually editing this file. Consider whether the version could be read frompackage.jsonor another configuration source to reduce maintenance overhead.🤖 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/ios/ClerkExpo.podspec` around lines 20 - 21, The clerk_ios_version variable is hardcoded as '1.2.4' in the podspec file, requiring manual updates whenever the Clerk iOS SDK version changes. Externalize this version by reading it from package.json or another centralized configuration source instead of hardcoding it. Modify the clerk_ios_version assignment to dynamically fetch the version from the appropriate configuration file, reducing maintenance overhead and keeping the dependency version synchronized across your project.
🤖 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/ios/ClerkExpoBridge.swift`:
- Around line 36-58: The clerkNativeBridgeReadyObservers hash table is accessed
without synchronization in the methods addClerkNativeBridgeReadyObserver,
removeClerkNativeBridgeReadyObserver, and emitClerkNativeBridgeReady, and
similarly clerkNativeClientChangedEmitter is accessed in
setClerkNativeClientChangedEmitter and emitClerkNativeClientChanged without
synchronization. Since NSHashTable is not thread-safe, concurrent calls from
different threads can corrupt the data structure or cause crashes. Create a
private serial DispatchQueue and wrap all accesses to both
clerkNativeBridgeReadyObservers and clerkNativeClientChangedEmitter within
synchronized blocks using this queue, ensuring all read and write operations are
serialized and thread-safe.
---
Nitpick comments:
In `@packages/expo/ios/ClerkExpo.podspec`:
- Around line 20-21: The clerk_ios_version variable is hardcoded as '1.2.4' in
the podspec file, requiring manual updates whenever the Clerk iOS SDK version
changes. Externalize this version by reading it from package.json or another
centralized configuration source instead of hardcoding it. Modify the
clerk_ios_version assignment to dynamically fetch the version from the
appropriate configuration file, reducing maintenance overhead and keeping the
dependency version synchronized across your project.
🪄 Autofix (Beta)
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), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1cfed06c-5d52-4b43-bbe1-7a2655f71514
📒 Files selected for processing (8)
.changeset/tidy-expo-ios-import.mdpackages/expo/app.plugin.jspackages/expo/ios/ClerkExpo.podspecpackages/expo/ios/ClerkExpoBridge.swiftpackages/expo/ios/ClerkExpoModule.swiftpackages/expo/ios/ClerkNativeBridge.swiftpackages/expo/package.jsonpackages/expo/src/__tests__/appPlugin.injectClerkExpoVersion.test.js
💤 Files with no reviewable changes (1)
- packages/expo/src/tests/appPlugin.injectClerkExpoVersion.test.js
mikepitre
force-pushed
the
mike/fix-expo-ios-spm-podspec-linking
branch
from
5faa0bc to
edbb210
Compare
mikepitre
force-pushed
the
mike/fix-expo-ios-spm-podspec-linking
branch
from
edbb210 to
f7088d0
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7088d0604
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83e37e979b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| products: ['ClerkKit', 'ClerkKitUI'] | ||
| ) | ||
| else | ||
| raise 'ClerkExpo requires React Native 0.75 or newer for iOS Swift Package Manager dependencies.' |
There was a problem hiding this comment.
For apps still on React Native 0.73/0.74 (which satisfied @clerk/expo's previous peer range and can receive this patch via a caret range), spm_dependency is not defined by RN's CocoaPods scripts, so this raise makes every iOS pod install fail even though those apps built with the old Xcode-project injection path. Because this is being released as a patch, please keep a fallback for those RN versions or avoid shipping this path to them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Our supported expo range is >=53, so supporting 0.73 and 0.74 is already a mismatch. Expo 53 contains React Native 0.79
wobsoriano
left a comment
wobsoriano
left a comment
There was a problem hiding this comment.
Tested this locally in both Expo SDK 54 and 56. Runs successfully ✅
2 participants
Summary
clerk-iosfromClerkExpo.podspecusing React Native'sspm_dependency, so theClerkExpopod target can importClerkKitandClerkKitUIdirectly.ClerkExpopod target; module and view wrappers callClerkNativeBridge.shareddirectly.@clerk/expoReact Native peer dependency from>=0.73to>=0.75, since this fix relies on React Native's podspec Swift Package Manager dependency support.Motivation
Expo SDK 54 generates
import ClerkExpofor Expo modules while SDK 56 generatesinternal import ClerkExpo. The import form is not the real problem; the issue was thatClerkExpodepended onClerkKit/ClerkKitUIthrough app-target project mutation instead of declaring the Swift package dependency on the pod target that actually imports those modules.Moving the SPM dependency into the podspec keeps the dependency attached to the correct native target and avoids having to special-case Expo's generated import style.
Because the Clerk iOS SDK is now visible to the pod target, the separate
ClerkExpoBridge.swiftprotocol/global registry layer is no longer needed. The remaining readiness and native-client-change coordination lives onClerkNativeBridge.Verification
git diff --checkpnpm --filter @clerk/expo test@clerk/expofrom a packed tarballnpx expo prebuild --clean --platform iospassed for both appsclerk-ios1.2.4through SPMAuthView,UserButton, JS sign-in, and JS sign-outSummary by CodeRabbit
Release Notes
Bug Fixes
Chores
Tests
Compatibility