fix(expo): deduplicate native client startup requests#9140
Open
mikepitre wants to merge 1 commit into
Open
Conversation
🦋 Changeset detectedLatest commit: 245258e 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.
|
Contributor
📝 WalkthroughWalkthroughExpo native and JavaScript client synchronization now gates startup on readiness, avoids redundant refreshes, suppresses duplicate client-change events, queues changes before readiness, and conditionally manages native event subscriptions. Android, iOS, provider wiring, hooks, tests, and release metadata are updated. ChangesExpo client synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ClerkProvider
participant NativeClientSync
participant ClerkInstance
participant NativeModule
ClerkProvider->>NativeClientSync: wait for native and Clerk readiness
NativeClientSync->>ClerkInstance: read cached token and status
NativeClientSync->>NativeModule: configure or synchronize token state
NativeModule-->>NativeClientSync: client state and native events
NativeClientSync->>ClerkInstance: apply queued refresh when enabled
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@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: |
mikepitre
changed the title
Contributor
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}
Contributor
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/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt (1)
403-436: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftHandle cleared device tokens in the native sync path.
syncClientStateFromJs(..., null, ..., true)still leaves the previous native device token in place on both platforms, so clearingCLERK_CLIENT_JWT_KEYcan repopulate stale client state instead of clearing it.
packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt#L403-L436: add an explicit clear branch whendidChangeDeviceTokenis true anddeviceTokenis null/blank.packages/expo/ios/ClerkNativeBridge.swift#L317-L329: handle theniltoken path the same way.🤖 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/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt` around lines 403 - 436, Handle cleared device tokens in syncClientStateFromJs on both platforms: in packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt lines 403-436, add an explicit branch for didChangeDeviceToken with a null or blank deviceToken that clears the native device token; in packages/expo/ios/ClerkNativeBridge.swift lines 317-329, apply the equivalent nil-token clearing behavior. Ensure the existing non-empty token update path remains unchanged.Source: Learnings
🤖 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/nativeClientSync.tsx`:
- Around line 814-826: Update the exported hook useNativeClientBootstrap with an
explicit named return interface describing isMountedRef and isNativeClientReady,
and annotate its return type accordingly. Also annotate the exported
useNativeClientEventSync hook with : void, preserving both hooks’ existing
behavior.
---
Outside diff comments:
In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt`:
- Around line 403-436: Handle cleared device tokens in syncClientStateFromJs on
both platforms: in
packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt lines
403-436, add an explicit branch for didChangeDeviceToken with a null or blank
deviceToken that clears the native device token; in
packages/expo/ios/ClerkNativeBridge.swift lines 317-329, apply the equivalent
nil-token clearing behavior. Ensure the existing non-empty token update path
remains unchanged.
🪄 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 Plus
Run ID: b1301bea-1056-418a-bc97-fe47cecfdcff
📒 Files selected for processing (8)
.changeset/calm-clients-start.mdpackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.ktpackages/expo/ios/ClerkNativeBridge.swiftpackages/expo/src/hooks/__tests__/useNativeClientEvents.test.tspackages/expo/src/hooks/useNativeClientEvents.tspackages/expo/src/provider/ClerkProvider.tsxpackages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsxpackages/expo/src/provider/nativeClientSync.tsx
Comment on lines
814
to
826
| export function useNativeClientBootstrap({ | ||
| publishableKey, | ||
| nativeRefreshFromJsControllerRef, | ||
| suppressTokenCacheNotificationsRef, | ||
| tokenCache, | ||
| clerkInstance, | ||
| }: { | ||
| publishableKey: string; | ||
| nativeRefreshFromJsControllerRef: MutableRefObject<NativeRefreshFromJsController | null>; | ||
| suppressTokenCacheNotificationsRef: MutableRefObject<number>; | ||
| tokenCache: TokenCache | undefined; | ||
| clerkInstance: SyncableClerkInstance | null | undefined; | ||
| }) { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant sections
wc -l packages/expo/src/provider/nativeClientSync.tsx
ast-grep outline packages/expo/src/provider/nativeClientSync.tsx --view expanded | sed -n '1,220p'
printf '\n--- lines 780-980 ---\n'
sed -n '780,980p' packages/expo/src/provider/nativeClientSync.tsx
printf '\n--- exported hook definitions ---\n'
rg -n "export function useNativeClient(Bootstrap|EventSync)|function useNativeClient(Bootstrap|EventSync)" packages/expo/src/provider/nativeClientSync.tsxRepository: clerk/javascript
Length of output: 8748
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant slices if the first command is not enough
sed -n '814,826p' packages/expo/src/provider/nativeClientSync.tsx
printf '\n---\n'
sed -n '944,960p' packages/expo/src/provider/nativeClientSync.tsx
printf '\n--- JSDoc/export context ---\n'
rg -n "useNativeClientBootstrap|useNativeClientEventSync|`@public`|`@since`|reference" packages/expo/src/provider/nativeClientSync.tsxRepository: clerk/javascript
Length of output: 1325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find nearby exported hooks in this file and similar files to compare return-type style
rg -n "export function use[A-Z].*\(" packages/expo/src packages -g '*.ts' -g '*.tsx' | sed -n '1,120p'Repository: clerk/javascript
Length of output: 14928
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- useNativeClientBootstrap body end ---'
sed -n '814,910p' packages/expo/src/provider/nativeClientSync.tsx
printf '\n%s\n' '--- useNativeClientEventSync body end ---'
sed -n '944,1001p' packages/expo/src/provider/nativeClientSync.tsx
printf '\n%s\n' '--- package export surface ---'
git ls-files packages/expo | rg '(^|/)index\.(ts|tsx|js|jsx|mjs|cjs)$|package\.json$'
printf '\n%s\n' '--- references to nativeClientSync.tsx ---'
rg -n "nativeClientSync\.tsx|useNativeClientBootstrap|useNativeClientEventSync|useSyncableTokenCache" packages/expo packages -g '!**/dist/**' -g '!**/build/**'Repository: clerk/javascript
Length of output: 8177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- provider barrel(s) ---'
git ls-files packages/expo/src/provider
printf '\n%s\n' '--- provider index / exports ---'
for f in packages/expo/src/provider/index.ts packages/expo/src/provider/index.tsx packages/expo/src/index.ts packages/expo/src/hooks/index.ts; do
if [ -f "$f" ]; then
echo "### $f"
sed -n '1,220p' "$f"
echo
fi
done
printf '%s\n' '--- package entrypoints mentioning provider ---'
rg -n "provider/|ClerkProvider|useNativeClientBootstrap|useNativeClientEventSync|nativeClientSync" packages/expo/src/index.ts packages/expo/src/provider packages/expo/src/hooks -g '*.ts' -g '*.tsx'Repository: clerk/javascript
Length of output: 9635
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether a result type already exists or is used nearby
rg -n "type .*Bootstrap|interface .*Bootstrap|UseNativeClientBootstrap|UseNativeClientEventSync|isNativeClientReady|isMountedRef" packages/expo/src/provider/nativeClientSync.tsx packages/expo/src/provider/ClerkProvider.tsxRepository: clerk/javascript
Length of output: 1435
Add explicit return types to these exported hooks. useNativeClientBootstrap should use a named result interface for { isMountedRef, isNativeClientReady }, and useNativeClientEventSync should be annotated : void.
🤖 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 814 - 826,
Update the exported hook useNativeClientBootstrap with an explicit named return
interface describing isMountedRef and isNativeClientReady, and annotate its
return type accordingly. Also annotate the exported useNativeClientEventSync
hook with : void, preserving both hooks’ existing behavior.
Sources: Coding guidelines, Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Expo native-client startup could trigger repeated client refreshes across the JS and native runtimes. In the captured SDK 56 launch, that produced 11 Clerk requests, including eight identical
GET /v1/clientcalls.The duplicate requests came from native configuration racing JS client hydration, observer callbacks echoing bootstrap and JS-originated changes, and device-token updates being followed by a second explicit refresh. This change waits for the JS Clerk instance before reading its initial token, configures native once with that token, and reconciles the two clients after both sides are ready. Native observers now seed their initial snapshot and suppress bootstrap or JS-originated echoes without dropping later native changes.
Token-cache notifications are deduplicated, client changes that occur before native is ready are coalesced, and configure failures still release the synchronization barrier. Focused tests cover initial bootstrap, token rotation, remount recovery, configure failure, and native-to-JS reconciliation.
With the SDK 56 hosted-auth example unchanged, process-cold launches on iOS and Android each produce four Clerk requests: one JS and one native request for
/v1/environment, plus one JS and one native request for/v1/client.Summary by CodeRabbit
Improvements
New Features
enabledoption to control native client event subscriptions and synchronization.