refactor(ui): Use existing organizationMemberships and remove infinite scrolling#8350
Conversation
…s disabled When the Organizations feature was disabled in the dashboard, the OAuth consent screen triggered the "Organizations feature required" dev dialog because useOrganizationList was called unconditionally whenever enableOrgSelection was set. Extracts the hook and OrgSelect into a child component (OrgSelection) that only mounts when organizationSettings.enabled is true, so the hook never fires on disabled instances.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 7c32196 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
📝 WalkthroughWalkthroughAdds a changeset for Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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. 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: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 37-76: Add a regression test that covers the
disabled-organizations path where enableOrgSelection=true but
organizationSettings.enabled=false: create a unit/test rendering of the
OAuthConsent flow (or the OrgSelection consumer) that stubs/mocks
useOrganizationList to return memberships and organizationSettings to have
enabled=false, then assert that the developer/org selection dialog does not open
and that OrgSelect (component OrgSelect and hidden input organization_id) is not
rendered; repeat similar assertions for the other referenced branches (lines
~88-90, 160-175, 274-279) to ensure coverage for the disabled organization
branch.
🪄 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: 12d5ad94-6965-49e9-a7a7-e4753efaad02
📒 Files selected for processing (1)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx
… to prevent spurious dev dialog When enableOrgSelection is passed but the Organizations feature is disabled in the dashboard, the component previously called useOrganizationList unconditionally which triggered the "Organizations feature required" dev dialog. The fix moves the hook into a child component (OrgSelectionGate) that only mounts when both flags are true, using a context provider pattern to surface the org select node and hidden input to their render sites without prop drilling. Tests updated to use f.withOrganizations() where required and a new assertion confirms the hook is not called when orgs are disabled.
…ist for org select Replaces the useOrganizationList hook with user.organizationMemberships, which is populated synchronously from the user object and contains the complete list of memberships without pagination. This eliminates the loading gate, the double spinner, and the useAttemptToEnableOrganizations call that was triggering the dev dialog for instances with the Organizations feature disabled. The org select is now gated on both ctx.enableOrgSelection and organizationSettings.enabled, and renders immediately.
…s-organization-check
| footer={hasMore ? <InfiniteListSpinner ref={loadMoreRef} /> : null} | ||
| onReachEnd={hasMore ? onLoadMore : undefined} | ||
| /> | ||
| <SelectOptionList /> |
There was a problem hiding this comment.
I will remove the added infinite scrolling props to this shared element in a follow up PR
Description
When the Organizations feature was disabled in the dashboard, the OAuth consent screen triggered the "Organizations feature required" dev dialog. This happened because
useOrganizationListwas called unconditionally wheneverenableOrgSelectionwas set — even passingundefinedto the hook still invokesuseAttemptToEnableOrganizationsinternally.The fix replaces
useOrganizationListentirely withuser.organizationMemberships, which is populated synchronously from the user object alongside the initial session load. The FAPI includes the full, unpaginated list of memberships in the user response (confirmed in the Go backend — no cap or truncation), so we get everything needed (org id, name, imageUrl) with no extra fetch, no loading state, and no spinner.The org select is now gated on both
ctx.enableOrgSelectionandorganizationSettings.enabled, and renders immediately without pop-in.Resolves SDK-63
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change