refactor(shared,clerk-js,react): move QueryClient ownership into @clerk/shared#8434
refactor(shared,clerk-js,react): move QueryClient ownership into @clerk/shared#8434jacekradko wants to merge 11 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 626d5f5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
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 |
…-ownership # Conflicts: # packages/clerk-js/package.json # packages/shared/package.json # pnpm-lock.yaml
|
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:
📝 WalkthroughWalkthroughQueryClient ownership for clerk-rq is moved out of Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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. Comment |
@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: |
Summary
QueryClientfrom@clerk/clerk-jsinto@clerk/shared. Both theQueryObserverand theQueryobjects it observes now resolve to a single@tanstack/query-coreinstance.clerk.__internal_queryClientgetter from both@clerk/clerk-jsand@clerk/react'sIsomorphicClerk, plus the dynamicquery-coreimport andqueryClientStatusevent.@tanstack/query-corefrom@clerk/clerk-js's dependencies (still a dep of@clerk/shared).Why
#8432 patched the symptom of #8428 by republishing the CDN bundle so its embedded
query-corematches what consumers'@clerk/sharedresolves to. That fix is correct but structural — every futurequery-corerelease that adds anObserver → Querymethod call could re-trigger the same crash class until both sides are forced to share one resolution.This PR removes the cross-bundle contract entirely. The
QueryClientis owned by an internal singleton in@clerk/shared, lazily instantiated on the browser only. Both theQueryObserver(constructor lives in@clerk/shared) and theQueryobjects (created viaclient.getQueryCache().build(...)) now come from one resolution — the gap #8428 exploited can no longer exist.Key insight: clerk-js never mutated the QueryClient — every
setQueryData/invalidateQueries/removeQueriescall already lives in@clerk/sharedhooks. So no push API back into clerk-js is needed; clerk-js simply stops creating the client.Behavior changes
clerk.__internal_queryClientremoved. Undocumented (leading underscore +@ts-expect-errorat every call site), but technically observable. No code in this monorepo reaches for it outside the call sites I removed; possible external consumers should now construct their ownQueryClientif they need one.getClerkQueryClient()returnsundefinedserver-side, so per-request renders never share a cache across requests. Hooks fall back to the same proxy mock + synthetic pending state that existed before.queryClientStatusevent removed. It was emitted once when clerk-js's lazyquery-coreimport resolved; with the singleton synchronously available on the browser there's nothing to signal. No code outside the removed listeners consumed it.Test plan
pnpm --filter @clerk/shared test— 1069/1069pnpm --filter @clerk/clerk-js test— 688/688pnpm --filter @clerk/ui test— 1681/1681 (incl. SubscriptionDetails / Checkout, the regression-prone billing UI tests)pnpm --filter @clerk/clerk-js build— clean, noquery-core-vendors_*.jschunk in dist (confirms the dep was actually dropped)pnpm --filter @clerk/shared buildStacked on #8432 — base will be
mainonce #8432 lands.