fix(expo): align native prebuilt component APIs#8699
Conversation
🦋 Changeset detectedLatest commit: dca9e99 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/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: |
| private var pendingProfilePromise: Promise? = null | ||
| val event = Arguments.createMap().apply { | ||
| putString("type", type) | ||
| if (sessionId == null) { |
There was a problem hiding this comment.
You're putting session id in this map either way. Could we simplify the expression?
There was a problem hiding this comment.
Yep, agreed. I simplified this to write the nullable sessionId directly with putString("sessionId", sessionId), which keeps the same JS shape without the explicit null branch.
| } | ||
| } | ||
|
|
||
| val content = @androidx.compose.runtime.Composable { |
There was a problem hiding this comment.
Nit: could we import this package? Should be able to do @Composable
There was a problem hiding this comment.
Yep, done. I imported Composable and changed the local lambda to val userButtonContent: @Composable () -> Unit = { ... } so this reads like the other Compose wrappers.
| } | ||
|
|
||
| val content = @androidx.compose.runtime.Composable { | ||
| MaterialTheme { |
There was a problem hiding this comment.
Would be curious what this is actually doing. It may be needed I'm just curious
There was a problem hiding this comment.
This is the Compose host setup for rendering Clerk Android UI inside a React Native view. The Clerk Compose components expect lifecycle/view model/saved-state owners, but RN-hosted views do not reliably inherit those locals, so we provide the Activity owners explicitly. I added a short comment here to make that intent clearer.
chriscanin
left a comment
There was a problem hiding this comment.
Hi Mike, things are looking really good in here, I have tested each screen thourouhgly side by side with the native ios quickstart application. This review is for iOS only right now. I am happy to test the android, but I wanted to put this out there first before moving on.
In my testing I found 3 (minor) issues that could be considered not 1:1:
- No grabber pill on the top of the RN Modal for sign in, the user button sheet has one.
- After sign-out, AuthView modal auto-opens instead of returning to the unauthenticated Sign In button state.
- Phone number sign-in/up tab tapping
Use phone numberdoes nothing, but this is a known issue: #8659
Side note: Docs and quickstarts will need updates to go along with this branch. I am happy to provide those once we are reviewed and satisfied with this PR.
I have also left 3 code comments / questions in this review as well.
| hasInitialized = true | ||
| presentAuthModal() | ||
| updateView() | ||
| } else if window == nil && hasInitialized && currentDismissable && !didCompleteAuthentication && !dismissalEventSent { |
There was a problem hiding this comment.
"dismissed" is suppressed when didCompleteAuthentication == true, so a successful sign-in never fires onDismiss. When AuthView is wrapped in a <Modal>, the consumer's visible state stays true after sign-in.
On the next sign-out the modal re-opens on its own.
Can we emit "dismissed" on auth success too?
| private var currentDismissable: Bool = false | ||
| private var hasInitialized: Bool = false | ||
| private var didSignOut = false | ||
| private var dismissalEventSent = false |
There was a problem hiding this comment.
The default for isDismissable changed from true to false for both ClerkAuthNativeView and ClerkUserProfileNativeView. This is a behavioral break for anyone relying on the old default to render the dismiss button.
This will not affect people in prod, but developers may be confused on what may be causing issues.
Can we add an explicit "Breaking" callout to the changeset / changelog so it shows up in release notes?
| fatalError("init(coder:) has not been implemented") | ||
| } | ||
|
|
||
| override public func didMoveToWindow() { |
There was a problem hiding this comment.
ClerkUserButtonNativeView adds its hosting controller as a child VC (addChild + didMove(toParent:)) but never detaches it so there's no window == nil branch and no removeFromSuperview override, unlike ClerkAuthNativeView / ClerkUserProfileNativeView.
A child VC isn't auto-removed from its parent when its view leaves the window, so on unmount the controller stays retained and its SwiftUI keeps living. This isn't about emitting a "dismissed" event as I see UserButton is event-less (as it should be). It's purely the teardown/detach for lifecycle parity.
Is it worth adding detachHostingController() here to match the other two views?
Summary
This PR aligns the Expo native prebuilt component surface with the way the Clerk iOS and Android SDKs are used directly: native views render content, app code owns presentation, and auth state is observed through the provider/hooks instead of component-specific completion callbacks.
Important pieces
Make
AuthViewandUserProfileViewapp-presented contentpresentAuth/presentUserProfilehelper paths, Android presentation activities, the user-profile modal hook, and the stale inline component aliases.clerk-iospatterns like presentingAuthView()in a sheet. The prebuilt view should not decide whether it is full screen, sheet, route, or inline content.Wrap the platform-native
UserButtonUserButtonview managers on iOS and Android and removes the JS avatar/button implementation.UserButtonshould behave like the native SDK button, including presenting the native user profile itself. JS should not reimplement the avatar, tap behavior, or profile presentation.Centralize native-to-JS auth state sync in
ClerkProviderAuthViewandUserProfileView.useAuth()already owns auth state for app developers. Component-level callbacks likeonCompleteduplicate that responsibility and make examples clunkier.Keep JS sign-out and native sign-out in sync
NativeSessionSyncnow avoids clearing a persisted native session during cold start before JS has adopted it, while still propagating a real JS sign-out to native.AuthViewsign-in, nativeUserButtonsign-out, and JSsignOut()should all converge on one session state.Trim bridge surface area
Before / after examples
Auth presentation
Before: Expo exposed imperative native presentation helpers, so presentation was owned by the SDK bridge.
After:
AuthViewis content. The app chooses sheet, route, full screen, or inline presentation.Full-screen/root auth
Before: full-screen behavior came from native presentation-specific bridge code.
After: render
AuthViewwherever the app wants it to live.User button
Before:
UserButtonreimplemented avatar rendering in JS, fetched native user state, and manually called the native profile presenter.After:
UserButtonis backed by the platform-native Clerk button. Tapping it opens the native user profile, just like the iOS and Android SDKs.User profile presentation
Before: profile presentation could be driven through Expo-specific imperative helpers.
After:
UserProfileViewis content. The app owns the presentation surface.Auth completion
Before:
AuthViewparsed native completion events and synced JS state from inside the component.After: auth completion is observed through Clerk auth state. The component stays presentation/content-only.
Testing
pnpm --filter @clerk/expo format:checkpnpm --filter @clerk/expo test@clerk/react/internalresolver issue when lintingClerkProvider.tsx.clerk-expo-quickstart:npx tsc --noEmitclerk-expo-quickstart:npx expo run:ios --device 9A1571DF-CC3D-465F-A2E1-C89EC2388B31Notes
UserButtonkeeps a tiny36x36host style because React Native/Yoga does not infer the intrinsic size of the native host view. The native SDK still owns rendering and presentation.minor, notmajor, because these prebuilt components are still beta and this is expected beta-surface iteration rather than a stable package-level breaking release.