fix(expo): initialize Android native components without event methods#8920
Conversation
🦋 Changeset detectedLatest commit: 57c1d23 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.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThe PR refactors Clerk Expo's native module initialization and readiness synchronization. Event listener methods ( ChangesExpo Native Module Readiness and Event Listener Refactor
Sequence Diagram(s)sequenceDiagram
participant App
participant useNativeClientEvents
participant getNativeClientEventEmitter
participant Platform
participant ClerkExpo
participant DeviceEventEmitter
participant RNEventEmitter as Native Event Handler
App->>useNativeClientEvents: Render with native client
useNativeClientEvents->>getNativeClientEventEmitter: Call in useEffect
getNativeClientEventEmitter->>Platform: Check Platform.OS
alt iOS Platform
Platform-->>getNativeClientEventEmitter: 'ios'
getNativeClientEventEmitter-->>useNativeClientEvents: Return DeviceEventEmitter
useNativeClientEvents->>DeviceEventEmitter: subscribe('clerkNativeClientChanged')
RNEventEmitter-->>useNativeClientEvents: Fire event on client change
else Android with addListener
Platform-->>getNativeClientEventEmitter: 'android'
getNativeClientEventEmitter->>ClerkExpo: Check addListener exists
ClerkExpo-->>getNativeClientEventEmitter: ✓ addListener exists
getNativeClientEventEmitter-->>useNativeClientEvents: Return ClerkExpo
useNativeClientEvents->>ClerkExpo: addListener('clerkNativeClientChanged')
RNEventEmitter-->>useNativeClientEvents: Fire event on client change
else Android without addListener
Platform-->>getNativeClientEventEmitter: 'android'
getNativeClientEventEmitter->>ClerkExpo: Check addListener exists
ClerkExpo-->>getNativeClientEventEmitter: ✗ addListener undefined
getNativeClientEventEmitter-->>useNativeClientEvents: Return null
useNativeClientEvents->>useNativeClientEvents: Early return (no subscription)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: |
1646878 to
cc8c9c7
Compare
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57c1d23b83
ℹ️ 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".
|
|
||
| const sourceId = `${nativeClientSyncSourceIdPrefix}-${generation}`; | ||
| await ClerkExpo.syncFromJsClientToken(bearerToken, sourceId); | ||
| await ClerkExpo.syncFromJsClientToken(bearerToken, sourceId, options.shouldRefreshClient); |
There was a problem hiding this comment.
Preserve the two-argument native sync call
Passing shouldRefreshClient unconditionally changes the JS/native bridge arity for syncFromJsClientToken. Apps can receive this JS through an OTA/update while still running the native module from the previous release, whose iOS and Android exports only accept (clientToken, sourceId); in that mixed-version case the bridge rejects or misroutes the promise callbacks before native sync runs, so token-cache changes stop syncing. Keep the existing two-argument call for cases with old semantics, or gate a new native method/capability before sending the third argument.
Useful? React with 👍 / 👎.
| return ClerkExpo as RefreshClientEventEmitter; | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Route Android native-client events without addListener
For the Android module shape this patch now accepts for bootstrap (configure/getClientToken/syncFromJsClientToken but no addListener), this returns null, so useNativeClientEvents never subscribes to clerkNativeClientChanged. The Android module still emits that event from Clerk.clientFlow, and NativeClientSync relies on it to reload JS state/token cache after native components change auth state; with no listener, native-driven sign-ins or session changes leave JS signed out/stale. Use a fallback emitter or otherwise handle native-to-JS sync for the no-addListener path.
Useful? React with 👍 / 👎.
wobsoriano
left a comment
There was a problem hiding this comment.
Looks good 👍🏼
Tested locally in ios and android simulators
Summary
Repro
On the native-components quickstart using the published 3.4.6 package, Android can render the AuthView shell without fields because the bootstrap code rejects the native module before calling configure. Packing this branch into that app restores the expected AuthView fields and social buttons.
Validation
Use the native-components quickstart with a package tarball from this branch, then run clean native prebuilds for both platforms. iOS and Android both build/run, and Android AuthView renders the expected fields/buttons.
Summary by CodeRabbit