fix(expo): synchronous QueryClient for React Native#8087
fix(expo): synchronous QueryClient for React Native#8087chriscanin wants to merge 5 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: db9bc31 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: |
📝 WalkthroughWalkthroughAdds a changeset file for a patch release and updates the Clerk Expo provider to define a lazy, synchronous getter for the internal TanStack QueryClient ( 🚥 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 |
…lity
clerk-js uses rspack code-splitting to lazily load QueryClient via
dynamic import('./query-core'). On React Native, Metro bundles everything
into a single file, so rspack's chunk loading never resolves and
__internal_queryClient stays undefined.
The useClerkQueryClient hook falls back to a no-op mock proxy when the
query client is undefined, causing hooks like useOrganizationList,
useSessionList, etc. to silently return empty data.
Fix: use Object.defineProperty to override the broken getter on the Clerk
instance with a synchronous version that creates a QueryClient via
require('@tanstack/query-core') on first access.
chriscanin
force-pushed
the
chris/fix-query-client-rn
branch
from
8cc330d to
c9d5b96
Compare
Code reviewFound 1 issue:
javascript/packages/expo/src/provider/singleton/createClerkInstance.ts Lines 121 to 123 in 944baf9 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. @chriscanin I think it makes sense to add |
@jacekradko I have added @tanstack/query-core as a direct dependency in @clerk/expo's package.json and pinned to the same version @clerk/clerk-js uses (5.90.16). This ensures the require() call resolves correctly with strict package managers like pnpm. |
The require('@tanstack/query-core') call fails with strict package
managers like pnpm since it was only available as a transitive dep
via @clerk/clerk-js. Pin to the same version clerk-js uses (5.90.16).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/package.json (1)
113-121:⚠️ Potential issue | 🟠 MajorAdd a regression test for the React Native query-client fallback path before merge.
This PR changes runtime behavior for query hooks but does not add/modify tests; without coverage, the empty-data regression can silently return. Please add at least one automated test that validates the synchronous QueryClient path (e.g.,
useOrganizationList/useSessionListreturns real data instead of the no-op client behavior).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/package.json` around lines 113 - 121, Add a regression test that exercises the React Native synchronous QueryClient fallback path to ensure hooks return real data (not the no-op client); specifically, create a test for a hook like useOrganizationList or useSessionList that forces the RN environment or mocks react-native/url-polyfill so the code takes the synchronous QueryClient branch, seed a mocked network response, call the hook (via a test renderer or React Testing Library), and assert it receives actual data instead of the empty/no-op result; place the test in the package's test suite, name it clearly (e.g., "react-native query-client fallback returns data"), and ensure it fails if the fallback returns the no-op client behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/expo/package.json`:
- Around line 113-121: Add a regression test that exercises the React Native
synchronous QueryClient fallback path to ensure hooks return real data (not the
no-op client); specifically, create a test for a hook like useOrganizationList
or useSessionList that forces the RN environment or mocks
react-native/url-polyfill so the code takes the synchronous QueryClient branch,
seed a mocked network response, call the hook (via a test renderer or React
Testing Library), and assert it receives actual data instead of the empty/no-op
result; place the test in the package's test suite, name it clearly (e.g.,
"react-native query-client fallback returns data"), and ensure it fails if the
fallback returns the no-op client behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 636aae78-90f1-4e12-8ea8-259ed03a7ddb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/expo/package.json
|
!snapshot |
|
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.v20260325160806 --save-exact
npm i @clerk/astro@3.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/backend@3.2.3-snapshot.v20260325160806 --save-exact
npm i @clerk/chrome-extension@3.1.5-snapshot.v20260325160806 --save-exact
npm i @clerk/clerk-js@6.3.3-snapshot.v20260325160806 --save-exact
npm i @clerk/dev-cli@0.1.1-snapshot.v20260325160806 --save-exact
npm i @clerk/expo@3.1.5-snapshot.v20260325160806 --save-exact
npm i @clerk/expo-passkeys@1.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/express@2.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/fastify@3.1.5-snapshot.v20260325160806 --save-exact
npm i @clerk/hono@0.1.5-snapshot.v20260325160806 --save-exact
npm i @clerk/localizations@4.2.4-snapshot.v20260325160806 --save-exact
npm i @clerk/msw@0.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/nextjs@7.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/nuxt@2.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/react@6.1.3-snapshot.v20260325160806 --save-exact
npm i @clerk/react-router@3.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/shared@4.3.3-snapshot.v20260325160806 --save-exact
npm i @clerk/tanstack-react-start@1.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/testing@2.0.7-snapshot.v20260325160806 --save-exact
npm i @clerk/ui@1.2.4-snapshot.v20260325160806 --save-exact
npm i @clerk/upgrade@2.0.3-snapshot.v20260325160806 --save-exact
npm i @clerk/vue@2.0.7-snapshot.v20260325160806 --save-exact |
|
@jacekradko , this has been tested locally, and is confirmed to fix. After snapshot test: |
|
@chriscanin I think we should address this when bundling clerk-js, instead of creating a separate dependency on the package in @jacekradko what do you think? |
Hi @brkalow @jacekradko , I spent some time yesterday looking into this and claude-ing, as this issue is effecting users who are attempting to use organizations, and are currently blocked. The fix described below would resolve this at a bundler level instead of creating a separate dependency on the package in @clerk/expo. This should bake @tanstack/query-core into the bundle, rather than trying to resolve it during async chunk loading. In the const clerkNative = merge(
entryForVariant(variants.clerkNative),
common({ mode, variant: variants.clerkNative }),
commonForProd(),
commonForProdChunked(),
{
output: {
publicPath: '',
},
module: {
parser: {
javascript: {
dynamicImportMode: 'eager',
},
},
},
optimization: {
splitChunks: false,
},
},
);This makes ALL dynamic
The webpack docs page for module.parser.javascript.dynamicImportMode only lists it as an option without explaining what it does. The actual explanation lives on the Module Methods page under the
Built NOTE: This applies to ALL dynamic imports in the native build, not just Thoughts on this? |
|
@chriscanin @brkalow What do you think of this approach: #8170 ? Essentially curating the native build entry point to statically import the query client |
3 participants
Summary
clerk-jslazily loadsQueryClientfrom@tanstack/query-corevia rspack's dynamicimport('./query-core')code-splittingl.e("675")) never resolves, soclerk.__internal_queryClientstaysundefineduseClerkQueryClienthook falls back to a no-op mock proxy when undefined, causinguseOrganizationList,useSessionList, and other query-based hooks to silently return empty dataObject.definePropertyoverride increateClerkInstance.tsthat synchronously creates aQueryClientviarequire('@tanstack/query-core')on first access, returning the expected{ __tag: 'clerk-rq-client', client }shapeReported by
Donovan (internal) —
useOrganizationListreturns empty data on React NativeTest plan
@clerk/expobuilds successfully with the changeuseOrganizationListreturns data on React Native after fixuseSessionList, etc.) work correctly🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores