fix(expo): Allow disabling native client sync - #9396
Conversation
🦋 Changeset detectedLatest commit: 1ae7c7c 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/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@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
@clerk/expoCurrent version: 4.2.5 Subpath
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthroughAdds the experimental Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
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/provider/nativeClientSync.tsx (1)
987-991: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInvalidate bootstrap work when
enabledchanges.When
enabledchanges tofalse, the next effect setsisMountedRef.currentback totrue. An in-flightconfigureNativeClerkcan then continue token-cache reads, native configuration, or synchronization while disabled. Clear the started key and readiness state when disabled, and use an effect-local cancellation token or generation counter. Include tests for both enable transitions.🤖 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 987 - 991, Update the effect governing native bootstrap around configureNativeClerk so disabling enabled invalidates in-flight work: clear startedPublishableKeyRef and readiness state, and use an effect-local cancellation token or generation counter checked before token-cache reads, native configuration, and synchronization. Ensure re-enabling starts a fresh bootstrap for the publishableKey, and add tests covering both enabled-to-disabled and disabled-to-enabled transitions.
🧹 Nitpick comments (1)
packages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsx (1)
175-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for runtime opt-out transitions.
This test covers only the initially disabled provider. Add cases that disable synchronization during bootstrap and re-enable it after disabling. Assert that no native work continues while disabled and that a fresh bootstrap handles the later enable.
As per coding guidelines, “Unit tests are required for all new functionality. Include tests for all new features. Verify proper error handling and edge cases.”
🤖 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/__tests__/ClerkProvider.nativeClientSync.test.tsx` around lines 175 - 200, Extend the native client synchronization tests around ClerkProvider to cover runtime opt-out transitions: disable synchronization during bootstrap and verify no native configuration, token retrieval, listener setup, or state syncing continues, then re-enable it and verify a fresh bootstrap performs the expected native work. Reuse the existing mocks and synchronization controls in the test suite, and preserve the current initially-disabled assertions.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@packages/expo/src/provider/nativeClientSync.tsx`:
- Around line 987-991: Update the effect governing native bootstrap around
configureNativeClerk so disabling enabled invalidates in-flight work: clear
startedPublishableKeyRef and readiness state, and use an effect-local
cancellation token or generation counter checked before token-cache reads,
native configuration, and synchronization. Ensure re-enabling starts a fresh
bootstrap for the publishableKey, and add tests covering both
enabled-to-disabled and disabled-to-enabled transitions.
---
Nitpick comments:
In
`@packages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsx`:
- Around line 175-200: Extend the native client synchronization tests around
ClerkProvider to cover runtime opt-out transitions: disable synchronization
during bootstrap and verify no native configuration, token retrieval, listener
setup, or state syncing continues, then re-enable it and verify a fresh
bootstrap performs the expected native work. Reuse the existing mocks and
synchronization controls in the test suite, and preserve the current
initially-disabled assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a94a3db7-7ced-4f48-9b1f-ef7d75ee28e5
📒 Files selected for processing (4)
.changeset/calm-pandas-sync.mdpackages/expo/src/provider/ClerkProvider.tsxpackages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsxpackages/expo/src/provider/nativeClientSync.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
Description
Adds an experimental
ClerkProviderprop to disable native client synchronization for Expo apps that do not use Clerk native components. This provides a temporary escape hatch for apps affected by native and JS client desynchronization.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change