test(shared): add query-client hotload compatibility regression#8618
test(shared): add query-client hotload compatibility regression#8618jacekradko wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 573c23b The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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.
|
|
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 Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a new Vitest test suite for Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/shared/src/react/hooks/__tests__/useOrganizationList.hotload-compat.spec.tsx`:
- Around line 76-80: The test registers the listener inside React.useEffect
(clerk.on('queryClientStatus', setLoaded)) which can miss the microtask emission
of readiness earlier (lines ~106-122), causing flakiness; fix by subscribing
synchronously or doing an immediate readiness check after subscription: in the
block where setQueryClientLoaded and clerk.on('queryClientStatus', setLoaded)
are used (the useEffect), either switch to React.useLayoutEffect so the listener
is added earlier, or keep useEffect but add an immediate check after registering
the listener (e.g. if (typeof clerk.isQueryClientReady === 'function' ?
clerk.isQueryClientReady() : clerk.queryClientStatus === 'ready')
setQueryClientLoaded(true)) so the test flips queryClientLoaded to true even if
the event already fired.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ef73791d-50d7-418b-af0e-158fed50d8ca
📒 Files selected for processing (2)
.changeset/query-client-compat-regression.mdpackages/shared/src/react/hooks/__tests__/useOrganizationList.hotload-compat.spec.tsx
@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: |
Adds a regression test that locks in
useOrganizationListbehavior against the legacy@clerk/sharedquery-client bridge (pre-#8434), where the hook readsClerk.__internal_queryClientand waits on thequeryClientStatusevent before swapping out the mock client.This is the path older framework SDKs still take when they hotload the latest
clerk-js, so the test pins down the contract: the hook starts loaded with the mock client, fires/me/organization_membershipsonce the real client resolves, and settles into a non-loading state with the expected count. Empty changeset since it's test-only.