feat(expo): re-introduce two-way JS/native session sync#8088
Open
chriscanin wants to merge 10 commits intomainfrom
Open
feat(expo): re-introduce two-way JS/native session sync#8088chriscanin wants to merge 10 commits intomainfrom
chriscanin wants to merge 10 commits intomainfrom
Conversation
Re-applies the changes from #8032 which was reverted in #8065. This PR exists for visibility and review before re-merging. Original changes: - Two-way JS/native token sync for expo native components - Native session cleared on sign-out - Improved initialization error handling with timeout/failure messages - Additional debug logging in development
🦋 Changeset detectedLatest commit: dd96357 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/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@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: |
Contributor
|
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:
📝 WalkthroughWalkthroughReintroduces two-way JS/native session synchronization for Expo: adds a changeset; bumps Android Clerk API/UI versions; updates Android ClerkExpoModule to guard initialization, write/read device tokens, add timeouts and session-waiting; adds getClientToken and keychain-based token/state tracking, conditional configure/refresh, and revised signOut/getSession behavior in iOS ClerkViewFactory and templates; introduces module-level auth-state emission; and integrates NativeSessionSync and token-sync logic into the React provider, UserButton, and useUserProfileModal. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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: 4
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/hooks/useUserProfileModal.ts (1)
61-127:⚠️ Potential issue | 🟠 MajorAdd regression coverage before reintroducing this flow.
This hook now depends on a multi-step contract across JS, the native module, and native UI (
getSession→configure→ modal presentation → post-dismiss sign-out reconciliation), but the PR adds no automated coverage for it. Since this exact auth-sync work was already reverted once, we need regression tests around token sync and native-driven sign-out before merge.As per coding guidelines, "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/src/hooks/useUserProfileModal.ts` around lines 61 - 127, The new presentUserProfile flow (presentUserProfile hook) requires regression tests to cover the JS↔native token sync and native-driven sign-out reconciliation; add tests that mock ClerkExpo.getSession, ClerkExpo.configure, ClerkExpo.presentUserProfile, ClerkExpo.signOut, tokenCache.getToken, and the clerk.signOut method to assert: (1) when native has no session but tokenCache.getToken returns a bearer token, ClerkExpo.configure is called with clerk.publishableKey and that token and presentUserProfile is invoked; (2) when post-modal ClerkExpo.getSession returns null but hadNativeSessionBefore was true, clerk.signOut (and ClerkExpo.signOut if present) are called; and (3) the inverse case where configure produces a native session avoids JS signOut; target tests at the unit/integration layer exercising presentUserProfile (the useUserProfileModal hook) and use Jest mocks/spies for ClerkExpo.getSession, configure, presentUserProfile, signOut and tokenCache.getToken to verify the exact call sequence and side effects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt`:
- Around line 253-257: The signOut(promise: Promise) branch that returns when
!Clerk.isInitialized currently resolves without clearing the stored device
token; update signOut to always remove the stored DEVICE_TOKEN from
SharedPreferences (the same key written by configure()) before resolving, even
when Clerk.isInitialized is false. Locate the signOut method in
ClerkExpoModule.kt and add logic to obtain the module's SharedPreferences and
remove the "DEVICE_TOKEN" entry (or the constant used for that key), then
proceed to promise.resolve(null).
In `@packages/expo/ios/ClerkExpoModule.swift`:
- Around line 23-26: The template is missing the required protocol method
getClientToken() which breaks conformance to ClerkViewFactoryProtocol; add a
concrete implementation of getClientToken() inside the ClerkViewFactory class
(the type that implements ClerkViewFactoryProtocol) that returns the correct
client token string or nil as appropriate, ensuring the method signature exactly
matches getClientToken() -> String? and is public/internal consistent with other
protocol methods; update any backing storage or token retrieval logic used by
configure/getSession to return the same token value.
In `@packages/expo/ios/templates/ClerkViewFactory.swift`:
- Around line 323-330: The signOut() method currently returns early when there
is no live native session, skipping Clerk.clearAllKeychainItems() and
Self.clerkConfigured = false; change the control flow in signOut() so that
clearing native keychain state and resetting Self.clerkConfigured always runs
regardless of whether Clerk.shared.session?.id is present—attempt the signOut
call only if sessionId exists (try await Clerk.shared.auth.signOut(sessionId:
sessionId)), but move Clerk.clearAllKeychainItems() and Self.clerkConfigured =
false outside/after that guard/conditional so they execute unconditionally;
update the signOut() function in ClerkViewFactory.swift accordingly.
---
Outside diff comments:
In `@packages/expo/src/hooks/useUserProfileModal.ts`:
- Around line 61-127: The new presentUserProfile flow (presentUserProfile hook)
requires regression tests to cover the JS↔native token sync and native-driven
sign-out reconciliation; add tests that mock ClerkExpo.getSession,
ClerkExpo.configure, ClerkExpo.presentUserProfile, ClerkExpo.signOut,
tokenCache.getToken, and the clerk.signOut method to assert: (1) when native has
no session but tokenCache.getToken returns a bearer token, ClerkExpo.configure
is called with clerk.publishableKey and that token and presentUserProfile is
invoked; (2) when post-modal ClerkExpo.getSession returns null but
hadNativeSessionBefore was true, clerk.signOut (and ClerkExpo.signOut if
present) are called; and (3) the inverse case where configure produces a native
session avoids JS signOut; target tests at the unit/integration layer exercising
presentUserProfile (the useUserProfileModal hook) and use Jest mocks/spies for
ClerkExpo.getSession, configure, presentUserProfile, signOut and
tokenCache.getToken to verify the exact call sequence and side effects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2579af5-b567-4b39-af9d-c538c6937b51
📒 Files selected for processing (9)
.changeset/native-session-sync-v2.mdpackages/expo/android/build.gradlepackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.ktpackages/expo/ios/ClerkExpoModule.swiftpackages/expo/ios/ClerkViewFactory.swiftpackages/expo/ios/templates/ClerkViewFactory.swiftpackages/expo/src/hooks/useUserProfileModal.tspackages/expo/src/native/UserButton.tsxpackages/expo/src/provider/ClerkProvider.tsx
packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt
Show resolved
Hide resolved
packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt
Show resolved
Hide resolved
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 (2)
packages/expo/src/provider/ClerkProvider.tsx (2)
364-367:⚠️ Potential issue | 🟡 MinorMissing
tokenCachein dependency array.Same issue as above - the effect uses
tokenCache(line 330) for saving native client tokens but doesn't include it in dependencies. This could cause tokens to be saved to a stale cache reference.Suggested fix
- }, [nativeAuthState, clerkInstance]); + }, [nativeAuthState, clerkInstance, tokenCache]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/src/provider/ClerkProvider.tsx` around lines 364 - 367, The effect that calls syncNativeAuthToJs uses tokenCache but doesn't list it in the dependency array, risking saving tokens to a stale cache; update the useEffect (the one that invokes syncNativeAuthToJs) to include tokenCache alongside nativeAuthState and clerkInstance in its dependency array so the effect re-runs when tokenCache changes, ensuring syncNativeAuthToJs always captures the current tokenCache reference.
308-311:⚠️ Potential issue | 🟡 MinorMissing
tokenCachein dependency array.The effect uses
tokenCache(line 192) but it's not included in the dependency array. IftokenCachechanges after mount, the effect will use a stale reference, potentially reading tokens from the wrong cache.Suggested fix
- }, [pk, clerkInstance]); + }, [pk, clerkInstance, tokenCache]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/src/provider/ClerkProvider.tsx` around lines 308 - 311, The effect that sets isMountedRef.current to false in the cleanup references tokenCache elsewhere in the effect but the dependency array only includes pk and clerkInstance; update the effect's dependency array to include tokenCache (or a stable identifier derived from it) so the effect re-runs when tokenCache changes, ensuring the effect does not capture a stale tokenCache; locate the useEffect that returns the cleanup setting isMountedRef.current = false (the closure referencing isMountedRef, pk, clerkInstance, and tokenCache) and add tokenCache to its dependency list (or memoize tokenCache where used) to fix the stale reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/src/provider/ClerkProvider.tsx`:
- Around line 66-136: Add unit/integration tests exercising the
NativeSessionSync component's sync paths: write tests that mount
NativeSessionSync (or the parent ClerkProvider) and simulate JS→native sync by
toggling useAuth isSignedIn, mocking
NativeClerkModule.configure/getSession/signOut and defaultTokenCache.getToken
(use CLERK_CLIENT_JWT_KEY), asserting configure is called with publishableKey
and token and that signOut is invoked on sign-out; also add tests for native→JS
token sync by mocking getSession to return a native session and ensuring
hasSyncedRef prevents redundant configure calls; target tests around
NativeSessionSync, syncToNative, and NativeClerkModule behaviors to cover error
branches and the effectiveTokenCache fallback.
---
Outside diff comments:
In `@packages/expo/src/provider/ClerkProvider.tsx`:
- Around line 364-367: The effect that calls syncNativeAuthToJs uses tokenCache
but doesn't list it in the dependency array, risking saving tokens to a stale
cache; update the useEffect (the one that invokes syncNativeAuthToJs) to include
tokenCache alongside nativeAuthState and clerkInstance in its dependency array
so the effect re-runs when tokenCache changes, ensuring syncNativeAuthToJs
always captures the current tokenCache reference.
- Around line 308-311: The effect that sets isMountedRef.current to false in the
cleanup references tokenCache elsewhere in the effect but the dependency array
only includes pk and clerkInstance; update the effect's dependency array to
include tokenCache (or a stable identifier derived from it) so the effect
re-runs when tokenCache changes, ensuring the effect does not capture a stale
tokenCache; locate the useEffect that returns the cleanup setting
isMountedRef.current = false (the closure referencing isMountedRef, pk,
clerkInstance, and tokenCache) and add tokenCache to its dependency list (or
memoize tokenCache where used) to fix the stale reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: fed743a6-c865-4570-98a6-5768d0a655e0
📒 Files selected for processing (1)
packages/expo/src/provider/ClerkProvider.tsx
Comment on lines
+66
to
+136
| function NativeSessionSync({ | ||
| publishableKey, | ||
| tokenCache, | ||
| }: { | ||
| publishableKey: string; | ||
| tokenCache: TokenCache | undefined; | ||
| }) { | ||
| const { isSignedIn } = useAuth(); | ||
| const hasSyncedRef = useRef(false); | ||
| // Use the provided tokenCache, falling back to the default SecureStore cache | ||
| const effectiveTokenCache = tokenCache ?? defaultTokenCache; | ||
|
|
||
| useEffect(() => { | ||
| if (!isSignedIn) { | ||
| hasSyncedRef.current = false; | ||
|
|
||
| // Clear the native session so native components (UserButton, etc.) | ||
| // don't continue showing a signed-in state after JS-side sign out. | ||
| const ClerkExpo = NativeClerkModule; | ||
| if (ClerkExpo?.signOut) { | ||
| void ClerkExpo.signOut().catch((error: unknown) => { | ||
| if (__DEV__) { | ||
| console.warn('[NativeSessionSync] Failed to clear native session:', error); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (hasSyncedRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| const syncToNative = async () => { | ||
| try { | ||
| const ClerkExpo = NativeClerkModule; | ||
| if (!ClerkExpo?.configure || !ClerkExpo?.getSession) { | ||
| return; | ||
| } | ||
|
|
||
| // Check if native already has a session (e.g. auth via AuthView or initial load) | ||
| const nativeSession = (await ClerkExpo.getSession()) as { | ||
| sessionId?: string; | ||
| session?: { id: string }; | ||
| } | null; | ||
| const hasNativeSession = !!(nativeSession?.sessionId || nativeSession?.session?.id); | ||
|
|
||
| if (hasNativeSession) { | ||
| hasSyncedRef.current = true; | ||
| return; | ||
| } | ||
|
|
||
| // Read the JS SDK's client JWT and push it to the native SDK | ||
| const bearerToken = (await effectiveTokenCache?.getToken(CLERK_CLIENT_JWT_KEY)) ?? null; | ||
| if (bearerToken) { | ||
| await ClerkExpo.configure(publishableKey, bearerToken); | ||
| hasSyncedRef.current = true; | ||
| } | ||
| } catch (error) { | ||
| if (__DEV__) { | ||
| console.warn('[NativeSessionSync] Failed to sync JS session to native:', error); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| void syncToNative(); | ||
| }, [isSignedIn, publishableKey, effectiveTokenCache]); | ||
|
|
||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding tests for the new session sync logic.
The NativeSessionSync component and the two-way sync paths introduce significant new functionality. Tests covering JS→native sync (sign-in/sign-out flows) and native→JS token sync would help ensure this behavior doesn't regress.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/expo/src/provider/ClerkProvider.tsx` around lines 66 - 136, Add
unit/integration tests exercising the NativeSessionSync component's sync paths:
write tests that mount NativeSessionSync (or the parent ClerkProvider) and
simulate JS→native sync by toggling useAuth isSignedIn, mocking
NativeClerkModule.configure/getSession/signOut and defaultTokenCache.getToken
(use CLERK_CLIENT_JWT_KEY), asserting configure is called with publishableKey
and token and that signOut is invoked on sign-out; also add tests for native→JS
token sync by mocking getSession to return a native session and ensuring
hasSyncedRef prevents redundant configure calls; target tests around
NativeSessionSync, syncToNative, and NativeClerkModule behaviors to cover error
branches and the effectiveTokenCache fallback.
Member
Author
There was a problem hiding this comment.
Will leave testing to another PR, test coverage needs to be added in many places, not just here.
Contributor
There was a problem hiding this comment.
@chriscanin, understood — happy to defer that to a follow-up PR. Would you like me to open a GitHub issue to track adding test coverage for NativeSessionSync and the broader two-way session sync paths so it doesn't get lost?
On cold start with a bearer token, configure() resolved as soon as Clerk.isInitialized flipped. Callers like useUserProfileModal and UserButton that immediately call getSession() could observe null. Now we wait for Clerk.sessionFlow to emit a non-null session (up to 5s) before resolving, matching the already-initialized path.
The session sync code uses Clerk.updateDeviceToken() which was added in clerk-android 1.0.10. Bumps from 1.0.9 to 1.0.10.
- Android: clear DEVICE_TOKEN from SharedPreferences in signOut() even when native SDK is not initialized, preventing stale token on next init - iOS: signOut() now always clears keychain data and resets state, even when there is no active native session - iOS templates: add missing getClientToken() for protocol conformance, fix signOut() to always clear keychain
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)
271-280:⚠️ Potential issue | 🟠 MajorAndroid
signOutdoes not clearDEVICE_TOKENwhen Clerk is initialized.When Clerk is initialized,
signOut()callsClerk.auth.signOut()without clearingDEVICE_TOKENfrom SharedPreferences. iOS always clears keychain items regardless of session state with an explicit comment: "This ensures the native SDK doesn't boot with a stale token on next launch."If the native SDK's
signOut()doesn't automatically clear the persisted device token, the nextconfigure()call could rehydrate a stale client token, creating a security and data isolation issue.The Android code already demonstrates awareness of this requirement—it clears the token when Clerk is not initialized with the exact same reasoning. The initialized path should match this behavior to be consistent with iOS.
Suggested fix to match iOS behavior
coroutineScope.launch { try { Clerk.auth.signOut() + // Clear DEVICE_TOKEN to match iOS behavior (Clerk.clearAllKeychainItems) + reactApplicationContext.getSharedPreferences("clerk_preferences", Context.MODE_PRIVATE) + .edit() + .remove("DEVICE_TOKEN") + .apply() promise.resolve(null) } catch (e: Exception) { promise.reject("E_SIGN_OUT_FAILED", e.message ?: "Sign out failed", e) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt` around lines 271 - 280, In signOut() inside ClerkExpoModule.kt, after calling Clerk.auth.signOut() make sure to also remove the persisted DEVICE_TOKEN from Android storage (same SharedPreferences key/mechanism used elsewhere) so the native SDK doesn't rehydrate a stale token; update the Clerk.auth.signOut() success path to call the same SharedPreferences removal logic that the "not initialized" branch uses (i.e., retrieve the same prefs, remove the DEVICE_TOKEN key and apply/commit) before resolving the promise. Ensure you reference the DEVICE_TOKEN constant and reuse the existing SharedPreferences helper code (or mirror its remove logic) so both initialized and uninitialized signOut paths behave identically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/ios/ClerkViewFactory.swift`:
- Around line 199-205: The getSession() method currently only guards on
Clerk.shared.session and can return stale data if Self.clerkConfigured is false;
update the guard in public func getSession() async -> [String: Any]? to check
both Self.clerkConfigured and let session = Clerk.shared.session (i.e., use
guard Self.clerkConfigured, let session = Clerk.shared.session else { return nil
}) before calling Self.sessionPayload(from: session, user: session.user ??
Clerk.shared.user) so it matches the template behavior.
---
Outside diff comments:
In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt`:
- Around line 271-280: In signOut() inside ClerkExpoModule.kt, after calling
Clerk.auth.signOut() make sure to also remove the persisted DEVICE_TOKEN from
Android storage (same SharedPreferences key/mechanism used elsewhere) so the
native SDK doesn't rehydrate a stale token; update the Clerk.auth.signOut()
success path to call the same SharedPreferences removal logic that the "not
initialized" branch uses (i.e., retrieve the same prefs, remove the DEVICE_TOKEN
key and apply/commit) before resolving the promise. Ensure you reference the
DEVICE_TOKEN constant and reuse the existing SharedPreferences helper code (or
mirror its remove logic) so both initialized and uninitialized signOut paths
behave identically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ba48ae8-ab0b-4854-ade3-4fed915c855c
📒 Files selected for processing (4)
packages/expo/android/build.gradlepackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.ktpackages/expo/ios/ClerkViewFactory.swiftpackages/expo/ios/templates/ClerkViewFactory.swift
Match the template's guard so getSession() returns nil after signOut() resets clerkConfigured, preventing stale session data.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Clerk.shared force-unwraps and crashes if accessed before configure(). The JS provider calls signOut() on native before Clerk is initialized, causing an immediate crash on launch. Guard all Clerk.shared access behind Self.clerkConfigured check.
Member
Author
|
!snapshot |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/expo/ios/templates/ClerkViewFactory.swift (1)
327-334:⚠️ Potential issue | 🔴 CriticalAlways clear native state even when
auth.signOutthrows.Line 330 can throw before Lines 332-334 run. A transient sign-out failure would leave the keychain and
clerkConfiguredflag intact, so stale native auth can rehydrate on the next open.Suggested fix
`@MainActor` public func signOut() async throws { - if Self.clerkConfigured { - if let sessionId = Clerk.shared.session?.id { - try await Clerk.shared.auth.signOut(sessionId: sessionId) - } - Clerk.clearAllKeychainItems() - } - Self.clerkConfigured = false + guard Self.clerkConfigured else { return } + defer { + Clerk.clearAllKeychainItems() + Self.clerkConfigured = false + } + + if let sessionId = Clerk.shared.session?.id { + try await Clerk.shared.auth.signOut(sessionId: sessionId) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/ios/templates/ClerkViewFactory.swift` around lines 327 - 334, In signOut(), ensure native state is always cleared even if Clerk.shared.auth.signOut(sessionId:) throws by moving Clerk.clearAllKeychainItems() and Self.clerkConfigured = false into a guaranteed-finalization path (e.g., a defer block at the start of public func signOut() or a finally-equivalent after a do/catch) so that regardless of errors from Clerk.shared.auth.signOut the keychain is cleared and the clerkConfigured flag is reset; reference the signOut function, Clerk.shared.auth.signOut, Clerk.clearAllKeychainItems(), and Self.clerkConfigured when locating and applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/ios/ClerkViewFactory.swift`:
- Around line 56-58: The code in ClerkViewFactory silently swallows errors from
Clerk.shared.refreshClient() (called in shouldRefreshConfiguredClient(for:)) and
only clears keychain credentials in a conditional block that can be skipped if
clerkConfigured is false or if auth.signOut() throws; change the refresh call to
propagate failures (replace try? await Clerk.shared.refreshClient() with a
throwing try await and let configure() surface the error or handle it
explicitly), and ensure keychain cleanup always runs on sign-out by moving the
keychain deletion into a guaranteed cleanup path (use defer or a
finally-equivalent) so that keychain removal executes regardless of
clerkConfigured or auth.signOut() throwing; update the related methods
(configure(), the branch that calls shouldRefreshConfiguredClient(for:), and the
sign-out flow invoking auth.signOut()) accordingly and add tests for
pre-configure sign-out and refresh/sign-out error propagation.
In `@packages/expo/ios/templates/ClerkViewFactory.swift`:
- Around line 61-63: In ClerkViewFactory.configure(), do not swallow errors from
Clerk.shared.refreshClient(): replace the 'try? await' usage with a propagating
call (use 'try await' so errors bubble up from refreshClient()) inside the
existing early-return branch that checks Self.clerkConfigured and bearerToken;
this ensures the async throws contract of configure() is honored and callers
(e.g., ClerkExpoModule) can handle failures.
---
Duplicate comments:
In `@packages/expo/ios/templates/ClerkViewFactory.swift`:
- Around line 327-334: In signOut(), ensure native state is always cleared even
if Clerk.shared.auth.signOut(sessionId:) throws by moving
Clerk.clearAllKeychainItems() and Self.clerkConfigured = false into a
guaranteed-finalization path (e.g., a defer block at the start of public func
signOut() or a finally-equivalent after a do/catch) so that regardless of errors
from Clerk.shared.auth.signOut the keychain is cleared and the clerkConfigured
flag is reset; reference the signOut function, Clerk.shared.auth.signOut,
Clerk.clearAllKeychainItems(), and Self.clerkConfigured when locating and
applying the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 17a948ad-051e-4b69-81ec-445bf8a23a37
📒 Files selected for processing (2)
packages/expo/ios/ClerkViewFactory.swiftpackages/expo/ios/templates/ClerkViewFactory.swift
If auth.signOut() throws (e.g. network error), clearAllKeychainItems() was skipped, leaving stale tokens. Using defer ensures keychain is always cleared regardless of whether the remote sign-out succeeds.
Member
Author
|
!snapshot |
Contributor
|
Hey @chriscanin - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.3.7-snapshot.v20260325172303 --save-exact
npm i @clerk/astro@3.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/backend@3.2.3-snapshot.v20260325172303 --save-exact
npm i @clerk/chrome-extension@3.1.5-snapshot.v20260325172303 --save-exact
npm i @clerk/clerk-js@6.3.3-snapshot.v20260325172303 --save-exact
npm i @clerk/dev-cli@0.1.1-snapshot.v20260325172303 --save-exact
npm i @clerk/expo@3.1.5-snapshot.v20260325172303 --save-exact
npm i @clerk/expo-passkeys@1.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/express@2.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/fastify@3.1.5-snapshot.v20260325172303 --save-exact
npm i @clerk/hono@0.1.5-snapshot.v20260325172303 --save-exact
npm i @clerk/localizations@4.2.4-snapshot.v20260325172303 --save-exact
npm i @clerk/msw@0.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/nextjs@7.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/nuxt@2.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/react@6.1.3-snapshot.v20260325172303 --save-exact
npm i @clerk/react-router@3.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/shared@4.3.3-snapshot.v20260325172303 --save-exact
npm i @clerk/tanstack-react-start@1.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/testing@2.0.7-snapshot.v20260325172303 --save-exact
npm i @clerk/ui@1.2.4-snapshot.v20260325172303 --save-exact
npm i @clerk/upgrade@2.0.3-snapshot.v20260325172303 --save-exact
npm i @clerk/vue@2.0.7-snapshot.v20260325172303 --save-exact |
brkalow
approved these changes
Mar 25, 2026
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
2 participants
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.
Summary
Re-applies the changes from #8032 which was reverted in #8065 due to premature merge.
This PR exists for visibility and review before re-merging. The code is identical to the original #8032:
Context
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements