Skip to content

feat(clerk-js): Portal option#7241

Closed
alexcarpenter wants to merge 5 commits intomainfrom
alexcarpenter/portal-options
Closed

feat(clerk-js): Portal option#7241
alexcarpenter wants to merge 5 commits intomainfrom
alexcarpenter/portal-options

Conversation

@alexcarpenter
Copy link
Member

@alexcarpenter alexcarpenter commented Nov 17, 2025

Description

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • New Features
    • Global portal configuration and a PortalProvider let you control where modals/popovers render.
    • UI components (Organization Switcher, User Button, User Profile modal, Select, Menu, Tooltip, Popover, Modal) now honor portal settings, including dynamic root callbacks.
    • Positioning automatically switches between fixed and absolute when a custom portal/root is used for more stable placement.
  • Public API
    • New PortalProvider export and portal configuration available for consumers.

@changeset-bot
Copy link

changeset-bot bot commented Nov 17, 2025

⚠️ No Changeset found

Latest commit: eaa0c4e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Nov 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Ready Preview Comment Nov 17, 2025 10:45pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

Walkthrough

Adds a PortalConfig type and a PortalContext/Provider; threads portal config through contexts and components, resolves portal roots at render time, and updates Popover/Modal/Menu/Select/Tooltip/UserButton/OrganizationSwitcher flows to conditionally portal and select fixed vs absolute positioning.

Changes

Cohort / File(s) Summary
Shared types
packages/shared/src/types/clerk.ts
Add exported `PortalConfig = boolean
Portal context / provider
packages/clerk-js/src/ui/contexts/PortalContext.tsx, packages/clerk-js/src/ui/contexts/index.ts, packages/react/src/components/PortalProvider.tsx, packages/react/src/components/index.ts
New PortalContext, PortalProvider and usePortalContext (and re-exports); React-side PortalProvider and hook to supply/consume PortalConfig.
Popover core & hooks
packages/clerk-js/src/ui/elements/Popover.tsx, packages/clerk-js/src/ui/hooks/usePopover.ts
Popover.portal prop becomes PortalConfig; resolve portal/root (function/ref/element), compute shouldPortal, unify rendering into FloatingPortal or inline FloatingNode; expose/apply strategy ('absolute'
Modal / Lazy modal wiring
packages/clerk-js/src/ui/elements/Modal.tsx, packages/clerk-js/src/ui/lazyModules/providers.tsx, packages/clerk-js/src/ui/Components.tsx
Widen portalRoot to accept PortalConfig; add resolvePortalRoot logic; LazyModalRenderer and Components thread portal into Modal/UserProfileModal rendering.
Top-level UI components
packages/clerk-js/src/ui/components/UserButton/index.tsx, packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx, packages/clerk-js/src/ui/components/OrganizationSwitcher/index.tsx, packages/clerk-js/src/ui/Components.tsx, packages/nextjs/src/client-boundary/uiComponents.tsx, packages/nextjs/src/index.ts
Read portal from context or props, compute resolved portal and use fixed positioning when a custom portal/root is present; pass portal into Popover and openUserProfile calls; export PortalProvider in nextjs re-exports.
Form controls & overlays
packages/clerk-js/src/ui/elements/Menu.tsx, packages/clerk-js/src/ui/elements/Select.tsx, packages/clerk-js/src/ui/elements/Tooltip.tsx
Replace boolean portal props with PortalConfig, read portal from context/prop, resolve portalRoot, choose strategy ('fixed' when portalling) and conditionally render via FloatingPortal or inline.
Public UI wiring
packages/clerk-js/src/ui/Contexts usages and packages/react/src/components/uiComponents.tsx
Propagate portalFromContext into component props forwarded to renderers (ClerkHostRenderer/ClerkHost render paths); internal wiring only.

Sequence Diagram(s)

sequenceDiagram
  actor U as UI Component (UserButton / OrgSwitcher / Select / Tooltip)
  participant C as PortalContext
  participant P as Popover / Modal
  participant F as FloatingPortal / Inline

  U->>C: read portal (PortalConfig | undefined)
  C-->>U: portal value (boolean | function | undefined)
  U->>P: render Popover/Modal with portal=PortalConfig
  P->>P: resolvePortalRoot(portal) => HTMLElement | null
  alt portal === false or resolved root null
    P->>F: render inline (FloatingNode) [strategy: absolute]
    F-->>P: content inline
  else resolved root exists
    P->>F: render via FloatingPortal(root) [strategy: fixed]
    F-->>P: content in portal root
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

  • Review focus:
    • PortalConfig typing and propagation across shared types.
    • resolvePortalRoot logic (function return values, ref vs element handling).
    • Popover/Modal unified rendering (FloatingPortal vs inline) and early-return behavior.
    • Correct selection and consistent application of strategy ('fixed' vs 'absolute') across components (Select, Menu, Tooltip, UserButton, OrganizationSwitcher).
    • Places where portal is forwarded to openUserProfile / LazyModalRenderer.

Poem

🐰 I hopped through props and found a door,
A root that whispers "float" or "stay ashore."
I threaded portals, snug and neat,
Floating where overlays and pages meet,
A tiny hop — interfaces soar! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(clerk-js): Portal option' accurately describes the main change: adding portal configuration as a feature to clerk-js.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch alexcarpenter/portal-options

Comment @coderabbitai help to get the list of available commands and usage tips.

@alexcarpenter
Copy link
Member Author

!snapshot

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 17, 2025

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@7241

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@7241

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@7241

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@7241

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@7241

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@7241

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@7241

@clerk/clerk-expo

npm i https://pkg.pr.new/@clerk/clerk-expo@7241

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@7241

@clerk/express

npm i https://pkg.pr.new/@clerk/express@7241

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@7241

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@7241

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@7241

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@7241

@clerk/clerk-react

npm i https://pkg.pr.new/@clerk/clerk-react@7241

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@7241

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@7241

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@7241

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@7241

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@7241

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@7241

@clerk/types

npm i https://pkg.pr.new/@clerk/types@7241

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@7241

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@7241

commit: eaa0c4e

@clerk-cookie
Copy link
Collaborator

Hey @alexcarpenter - the snapshot version command generated the following package versions:

Package Version
@clerk/agent-toolkit 0.2.2-snapshot.v20251117204446
@clerk/astro 2.16.0-snapshot.v20251117204446
@clerk/backend 2.23.0-snapshot.v20251117204446
@clerk/chrome-extension 2.8.2-snapshot.v20251117204446
@clerk/clerk-js 5.109.0-snapshot.v20251117204446
@clerk/elements 0.23.83-snapshot.v20251117204446
@clerk/clerk-expo 2.19.2-snapshot.v20251117204446
@clerk/expo-passkeys 0.4.19-snapshot.v20251117204446
@clerk/express 1.7.50-snapshot.v20251117204446
@clerk/fastify 2.6.2-snapshot.v20251117204446
@clerk/localizations 3.28.1-snapshot.v20251117204446
@clerk/nextjs 6.35.2-snapshot.v20251117204446
@clerk/nuxt 1.13.0-snapshot.v20251117204446
@clerk/clerk-react 5.56.0-snapshot.v20251117204446
@clerk/react-router 2.2.2-snapshot.v20251117204446
@clerk/remix 4.13.17-snapshot.v20251117204446
@clerk/shared 3.35.0-snapshot.v20251117204446
@clerk/tanstack-react-start 0.27.2-snapshot.v20251117204446
@clerk/testing 1.13.16-snapshot.v20251117204446
@clerk/themes 2.4.37-snapshot.v20251117204446
@clerk/types 4.101.0-snapshot.v20251117204446
@clerk/vue 1.16.0-snapshot.v20251117204446

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.2.2-snapshot.v20251117204446 --save-exact

@clerk/astro

npm i @clerk/astro@2.16.0-snapshot.v20251117204446 --save-exact

@clerk/backend

npm i @clerk/backend@2.23.0-snapshot.v20251117204446 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@2.8.2-snapshot.v20251117204446 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.109.0-snapshot.v20251117204446 --save-exact

@clerk/elements

npm i @clerk/elements@0.23.83-snapshot.v20251117204446 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@2.19.2-snapshot.v20251117204446 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@0.4.19-snapshot.v20251117204446 --save-exact

@clerk/express

npm i @clerk/express@1.7.50-snapshot.v20251117204446 --save-exact

@clerk/fastify

npm i @clerk/fastify@2.6.2-snapshot.v20251117204446 --save-exact

@clerk/localizations

npm i @clerk/localizations@3.28.1-snapshot.v20251117204446 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@6.35.2-snapshot.v20251117204446 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@1.13.0-snapshot.v20251117204446 --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.56.0-snapshot.v20251117204446 --save-exact

@clerk/react-router

npm i @clerk/react-router@2.2.2-snapshot.v20251117204446 --save-exact

@clerk/remix

npm i @clerk/remix@4.13.17-snapshot.v20251117204446 --save-exact

@clerk/shared

npm i @clerk/shared@3.35.0-snapshot.v20251117204446 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@0.27.2-snapshot.v20251117204446 --save-exact

@clerk/testing

npm i @clerk/testing@1.13.16-snapshot.v20251117204446 --save-exact

@clerk/themes

npm i @clerk/themes@2.4.37-snapshot.v20251117204446 --save-exact

@clerk/types

npm i @clerk/types@4.101.0-snapshot.v20251117204446 --save-exact

@clerk/vue

npm i @clerk/vue@1.16.0-snapshot.v20251117204446 --save-exact

@alexcarpenter
Copy link
Member Author

!snapshot

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
packages/clerk-js/src/ui/Components.tsx (1)

466-466: Consider removing the type assertion.

The inline type cast (userProfileModal as UserProfileProps & { portal?: boolean | (() => HTMLElement | null) }) duplicates the PortalConfig type definition and may be unnecessary if UserProfileProps already includes the portal field (as shown in the shared types). If the types are properly propagated, you should be able to access userProfileModal?.portal directly without casting.

Apply this diff if the type assertion is unnecessary:

-      portal={(userProfileModal as UserProfileProps & { portal?: boolean | (() => HTMLElement | null) })?.portal}
+      portal={userProfileModal?.portal}
packages/shared/src/types/clerk.ts (1)

66-71: Document the true value behavior in JSDoc.

The JSDoc for PortalConfig documents false (disable portalling) and the function variant (portal to returned element), but doesn't explain what true means. Please add documentation for the true case.

 /**
  * Configuration for portalling components to a specific container.
  * - `false`: Disable portalling (render in place)
+ * - `true`: Portal to the default container (document.body)
  * - `() => HTMLElement | null`: Portal to the returned element
  */
 export type PortalConfig = boolean | (() => HTMLElement | null);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cbaa336 and 03ab666.

📒 Files selected for processing (6)
  • packages/clerk-js/src/ui/Components.tsx (1 hunks)
  • packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx (3 hunks)
  • packages/clerk-js/src/ui/elements/Modal.tsx (3 hunks)
  • packages/clerk-js/src/ui/elements/Popover.tsx (3 hunks)
  • packages/clerk-js/src/ui/lazyModules/providers.tsx (3 hunks)
  • packages/shared/src/types/clerk.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
packages/clerk-js/src/ui/elements/Popover.tsx (1)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx (1)
packages/shared/src/types/clerk.ts (1)
  • UserButtonProps (1683-1752)
packages/clerk-js/src/ui/elements/Modal.tsx (2)
packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
  • withFloatingTree (139-154)
packages/clerk-js/src/ui/hooks/useScrollLock.ts (1)
  • useScrollLock (69-85)
packages/clerk-js/src/ui/Components.tsx (1)
packages/shared/src/types/clerk.ts (1)
  • UserProfileProps (1547-1593)
packages/clerk-js/src/ui/lazyModules/providers.tsx (1)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx (1)

22-22: LGTM! Portal prop properly forwarded through user actions.

The portal prop is correctly added to the picked UserButtonProps and consistently forwarded to openUserProfile in both the manage account and user profile action flows.

Also applies to: 49-52, 68-68

import type { PropsWithChildren } from 'react';
import React from 'react';

type PortalConfig = boolean | (() => HTMLElement | null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Import PortalConfig from shared types instead of redefining it.

This is the fourth duplication of the PortalConfig type across the codebase. Centralizing this by importing from @clerk/shared/types will improve maintainability.

Apply this diff:

 import type { FloatingContext, ReferenceType } from '@floating-ui/react';
 import { FloatingFocusManager, FloatingNode, FloatingPortal } from '@floating-ui/react';
+import type { PortalConfig } from '@clerk/shared/types';
 import type { PropsWithChildren } from 'react';
 import React from 'react';

-type PortalConfig = boolean | (() => HTMLElement | null);
-
 type PopoverProps = PropsWithChildren<{
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type PortalConfig = boolean | (() => HTMLElement | null);
import type { FloatingContext, ReferenceType } from '@floating-ui/react';
import { FloatingFocusManager, FloatingNode, FloatingPortal } from '@floating-ui/react';
import type { PortalConfig } from '@clerk/shared/types';
import type { PropsWithChildren } from 'react';
import React from 'react';
type PopoverProps = PropsWithChildren<{
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/elements/Popover.tsx around line 6, the local type
alias "type PortalConfig = boolean | (() => HTMLElement | null);" duplicates a
shared type; remove this local definition and instead import PortalConfig from
"@clerk/shared/types" at the top of the file, update any usages to reference the
imported name, and ensure the import is correctly added to the existing import
block (or create one) and that TypeScript builds without additional type
conflicts.

Comment on lines +40 to +52
// Resolve portal root
const resolveRoot = (): HTMLElement | null => {
if (typeof portal === 'function') {
return portal();
}
if (typeof root === 'object' && 'current' in root) {
return root.current;
}
return root || null;
};

const portalRoot = portal !== false ? resolveRoot() : null;
const shouldPortal = portal !== false && portalRoot !== null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Verify portal logic consistency with documented defaults.

The portal resolution logic (lines 41-52) results in shouldPortal = false when portal={true} is specified without an explicit root element, because resolveRoot() returns null when only a boolean is passed.

However, the JSDoc for UserProfileProps.portal and UserButtonProps.portal documents @default true, suggesting portalling should be enabled by default. With the current logic, portal={true} without a root disables portalling, which seems inconsistent with the documented behavior.

Please verify:

  1. Should portal={true} use a default container (e.g., document.body)?
  2. Or should the JSDoc be updated to clarify that portal={true} requires an explicit root?

I need to verify the review comment by examining the actual codebase. Let me gather the necessary information.


🏁 Script executed:

#!/bin/bash
# Find and read the Popover.tsx file
fd -t f "Popover.tsx" packages/clerk-js/src/ui/elements

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Search for UserProfileProps and UserButtonProps definitions
rg -n "interface UserProfileProps|interface UserButtonProps|type UserProfileProps|type UserButtonProps" -A 10 packages/clerk-js/src

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Read the Popover.tsx file to see full context
cat packages/clerk-js/src/ui/elements/Popover.tsx

Length of output: 2090


I found the Popover.tsx file. Now I need to search for UserProfileProps and UserButtonProps to verify the JSDoc claims in the review.


🏁 Script executed:

#!/bin/bash
# Search more broadly for UserProfileProps and UserButtonProps
rg -n "UserProfileProps|UserButtonProps" packages/clerk-js/src --type=ts --type=tsx | head -20

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Search for portal prop documentation in prop interfaces
rg -n "portal\?" packages/clerk-js/src -B 5 -A 2 --type=ts --type=tsx | grep -E "(portal|@default)" | head -30

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Find where these props might be defined
fd -t f -e "ts" -e "tsx" packages/clerk-js/src | xargs grep -l "UserProfileProps\|UserButtonProps" | head -5

Length of output: 389


Let me fix the command syntax and search again:


🏁 Script executed:

#!/bin/bash
# Search for UserProfileProps and UserButtonProps without type specification
rg -n "UserProfileProps|UserButtonProps" packages/clerk-js/src | head -20

Length of output: 1856


🏁 Script executed:

#!/bin/bash
# Search for files containing these types
fd . packages/clerk-js/src -type f | xargs grep -l "UserProfileProps\|UserButtonProps" 2>/dev/null | head -10

Length of output: 170


🏁 Script executed:

#!/bin/bash
# Search for portal prop in all files
rg -n "portal\?" packages/clerk-js/src -B 3 -A 1 | head -50

Length of output: 2112


I can see UserProfileProps and UserButtonProps are imported from @clerk/shared/types. Let me find those definitions:


🏁 Script executed:

#!/bin/bash
# Find @clerk/shared package
find . -type d -name "shared" | grep -E "clerk|packages" | head -10

Length of output: 167


🏁 Script executed:

#!/bin/bash
# Search for the actual type definitions in the repo
rg -n "export.*type.*UserProfileProps|export.*interface.*UserProfileProps" . | head -10

Length of output: 292


🏁 Script executed:

#!/bin/bash
# Look for @clerk/shared in package.json files
rg -n "@clerk/shared" packages/clerk-js | head -5

Length of output: 512


Now let me read the UserProfileProps definition and check the portal property documentation:


🏁 Script executed:

#!/bin/bash
# Read the UserProfileProps definition with context
sed -n '1540,1600p' packages/shared/src/types/clerk.ts

Length of output: 1960


🏁 Script executed:

#!/bin/bash
# Search for portal property documentation in the shared types
rg -n "portal" packages/shared/src/types/clerk.ts -B 2 -A 1

Length of output: 3073


🏁 Script executed:

#!/bin/bash
# Find UserButtonProps definition
rg -n "export.*type.*UserButtonProps|export.*interface.*UserButtonProps" packages/shared/src/types/clerk.ts -A 30 | head -60

Length of output: 1249


Portal default behavior in Popover contradicts JSDoc documentation.

The code behavior is inconsistent with the documented defaults. When portal={true} (the default) is used without an explicit root, the shouldPortal flag becomes false, disabling portalling entirely. However, the JSDoc for both UserProfileProps.portal and UserButtonProps.portal in packages/shared/src/types/clerk.ts documents @default true, implying portalling should be enabled by default.

Additionally, the Popover's root prop JSDoc claims @default document.body, but the resolveRoot() function returns null when root is undefined, not using document.body.

Fix required: Either:

  1. Update the implementation to use document.body as the default root when not explicitly provided (matching the JSDoc), or
  2. Update the JSDoc to clarify that portal={true} requires an explicit root element to function.

@alexcarpenter
Copy link
Member Author

!snapshot

@clerk-cookie
Copy link
Collaborator

Hey @alexcarpenter - the snapshot version command generated the following package versions:

Package Version
@clerk/agent-toolkit 0.2.2-snapshot.v20251117213621
@clerk/astro 2.16.0-snapshot.v20251117213621
@clerk/backend 2.23.0-snapshot.v20251117213621
@clerk/chrome-extension 2.8.2-snapshot.v20251117213621
@clerk/clerk-js 5.109.0-snapshot.v20251117213621
@clerk/elements 0.23.83-snapshot.v20251117213621
@clerk/clerk-expo 2.19.2-snapshot.v20251117213621
@clerk/expo-passkeys 0.4.19-snapshot.v20251117213621
@clerk/express 1.7.50-snapshot.v20251117213621
@clerk/fastify 2.6.2-snapshot.v20251117213621
@clerk/localizations 3.28.1-snapshot.v20251117213621
@clerk/nextjs 6.35.2-snapshot.v20251117213621
@clerk/nuxt 1.13.0-snapshot.v20251117213621
@clerk/clerk-react 5.56.0-snapshot.v20251117213621
@clerk/react-router 2.2.2-snapshot.v20251117213621
@clerk/remix 4.13.17-snapshot.v20251117213621
@clerk/shared 3.35.0-snapshot.v20251117213621
@clerk/tanstack-react-start 0.27.2-snapshot.v20251117213621
@clerk/testing 1.13.16-snapshot.v20251117213621
@clerk/themes 2.4.37-snapshot.v20251117213621
@clerk/types 4.101.0-snapshot.v20251117213621
@clerk/vue 1.16.0-snapshot.v20251117213621

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.2.2-snapshot.v20251117213621 --save-exact

@clerk/astro

npm i @clerk/astro@2.16.0-snapshot.v20251117213621 --save-exact

@clerk/backend

npm i @clerk/backend@2.23.0-snapshot.v20251117213621 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@2.8.2-snapshot.v20251117213621 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.109.0-snapshot.v20251117213621 --save-exact

@clerk/elements

npm i @clerk/elements@0.23.83-snapshot.v20251117213621 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@2.19.2-snapshot.v20251117213621 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@0.4.19-snapshot.v20251117213621 --save-exact

@clerk/express

npm i @clerk/express@1.7.50-snapshot.v20251117213621 --save-exact

@clerk/fastify

npm i @clerk/fastify@2.6.2-snapshot.v20251117213621 --save-exact

@clerk/localizations

npm i @clerk/localizations@3.28.1-snapshot.v20251117213621 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@6.35.2-snapshot.v20251117213621 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@1.13.0-snapshot.v20251117213621 --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.56.0-snapshot.v20251117213621 --save-exact

@clerk/react-router

npm i @clerk/react-router@2.2.2-snapshot.v20251117213621 --save-exact

@clerk/remix

npm i @clerk/remix@4.13.17-snapshot.v20251117213621 --save-exact

@clerk/shared

npm i @clerk/shared@3.35.0-snapshot.v20251117213621 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@0.27.2-snapshot.v20251117213621 --save-exact

@clerk/testing

npm i @clerk/testing@1.13.16-snapshot.v20251117213621 --save-exact

@clerk/themes

npm i @clerk/themes@2.4.37-snapshot.v20251117213621 --save-exact

@clerk/types

npm i @clerk/types@4.101.0-snapshot.v20251117213621 --save-exact

@clerk/vue

npm i @clerk/vue@1.16.0-snapshot.v20251117213621 --save-exact

@alexcarpenter
Copy link
Member Author

!snapshot

@clerk-cookie
Copy link
Collaborator

Hey @alexcarpenter - the snapshot version command generated the following package versions:

Package Version
@clerk/agent-toolkit 0.2.2-snapshot.v20251117220746
@clerk/astro 2.16.0-snapshot.v20251117220746
@clerk/backend 2.23.0-snapshot.v20251117220746
@clerk/chrome-extension 2.8.2-snapshot.v20251117220746
@clerk/clerk-js 5.109.0-snapshot.v20251117220746
@clerk/elements 0.23.83-snapshot.v20251117220746
@clerk/clerk-expo 2.19.2-snapshot.v20251117220746
@clerk/expo-passkeys 0.4.19-snapshot.v20251117220746
@clerk/express 1.7.50-snapshot.v20251117220746
@clerk/fastify 2.6.2-snapshot.v20251117220746
@clerk/localizations 3.28.1-snapshot.v20251117220746
@clerk/nextjs 6.35.2-snapshot.v20251117220746
@clerk/nuxt 1.13.0-snapshot.v20251117220746
@clerk/clerk-react 5.56.0-snapshot.v20251117220746
@clerk/react-router 2.2.2-snapshot.v20251117220746
@clerk/remix 4.13.17-snapshot.v20251117220746
@clerk/shared 3.35.0-snapshot.v20251117220746
@clerk/tanstack-react-start 0.27.2-snapshot.v20251117220746
@clerk/testing 1.13.16-snapshot.v20251117220746
@clerk/themes 2.4.37-snapshot.v20251117220746
@clerk/types 4.101.0-snapshot.v20251117220746
@clerk/vue 1.16.0-snapshot.v20251117220746

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.2.2-snapshot.v20251117220746 --save-exact

@clerk/astro

npm i @clerk/astro@2.16.0-snapshot.v20251117220746 --save-exact

@clerk/backend

npm i @clerk/backend@2.23.0-snapshot.v20251117220746 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@2.8.2-snapshot.v20251117220746 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.109.0-snapshot.v20251117220746 --save-exact

@clerk/elements

npm i @clerk/elements@0.23.83-snapshot.v20251117220746 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@2.19.2-snapshot.v20251117220746 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@0.4.19-snapshot.v20251117220746 --save-exact

@clerk/express

npm i @clerk/express@1.7.50-snapshot.v20251117220746 --save-exact

@clerk/fastify

npm i @clerk/fastify@2.6.2-snapshot.v20251117220746 --save-exact

@clerk/localizations

npm i @clerk/localizations@3.28.1-snapshot.v20251117220746 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@6.35.2-snapshot.v20251117220746 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@1.13.0-snapshot.v20251117220746 --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.56.0-snapshot.v20251117220746 --save-exact

@clerk/react-router

npm i @clerk/react-router@2.2.2-snapshot.v20251117220746 --save-exact

@clerk/remix

npm i @clerk/remix@4.13.17-snapshot.v20251117220746 --save-exact

@clerk/shared

npm i @clerk/shared@3.35.0-snapshot.v20251117220746 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@0.27.2-snapshot.v20251117220746 --save-exact

@clerk/testing

npm i @clerk/testing@1.13.16-snapshot.v20251117220746 --save-exact

@clerk/themes

npm i @clerk/themes@2.4.37-snapshot.v20251117220746 --save-exact

@clerk/types

npm i @clerk/types@4.101.0-snapshot.v20251117220746 --save-exact

@clerk/vue

npm i @clerk/vue@1.16.0-snapshot.v20251117220746 --save-exact

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (2)
packages/clerk-js/src/ui/elements/Popover.tsx (1)

1-1: Default portal=true still disables portalling when no root is provided (conflicts with JSDoc).

The new PortalConfig/context wiring is good, but the core behavior problem from the earlier review is still present:

  • Lines 41–44: portal defaults to true when neither prop nor context is provided.
  • Lines 45–53: resolveRoot() returns null when there is no root and portal is not a function.
  • Lines 56–57: portalRoot = portal !== false ? resolveRoot() : null; and shouldPortal = portal !== false && portalRoot !== null;.
  • Lines 74–82: When shouldPortal is false (e.g., portal === true and no root), FloatingPortal is not rendered at all and the content is rendered inline.

This means:

  • The documented default root?: HTMLElement ... @default document.body is never honored, because you never call <FloatingPortal> without an explicit non-null root.
  • A config of portal={true} (or the default true when nothing is provided) with no explicit root actually disables portalling instead of portalling to a default container.

To align behavior with the JSDoc and expected semantics (“portal=true” should portal, and when no root is given it should use a default like document.body), you can:

  • Keep portalRoot as the optional explicit root.
  • Treat portal !== false as “portal enabled”.
  • Let FloatingPortal handle the root === undefined case.

For example:

-  const portalRoot = portal !== false ? resolveRoot() : null;
-  const shouldPortal = portal !== false && portalRoot !== null;
+  const portalRoot = portal !== false ? resolveRoot() : null;
@@
-  if (!isOpen) {
-    return <FloatingNode id={nodeId} />;
-  }
+  if (!isOpen) {
+    return <FloatingNode id={nodeId} />;
+  }
@@
-  if (shouldPortal) {
-    return (
-      <FloatingNode id={nodeId}>
-        <FloatingPortal root={portalRoot}>{content}</FloatingPortal>
-      </FloatingNode>
-    );
-  }
-
-  return <FloatingNode id={nodeId}>{content}</FloatingNode>;
+  if (portal !== false) {
+    return (
+      <FloatingNode id={nodeId}>
+        <FloatingPortal root={portalRoot ?? undefined}>{content}</FloatingPortal>
+      </FloatingNode>
+    );
+  }
+
+  return <FloatingNode id={nodeId}>{content}</FloatingNode>;

Notes:

  • With this change, when portal resolves to true and no root is provided, FloatingPortal is rendered with root={undefined}, allowing it to fall back to its own default (typically document.body).
  • When portal is false, portalling is explicitly disabled.
  • When portal is a function (from prop or context), resolveRoot() will call it and pass the resulting element as root.

The new early if (!isOpen) (lines 59–61) also treats isOpen === undefined as closed; ensure all call sites pass isOpen explicitly via usePopover so uncontrolled usage doesn’t inadvertently render nothing.

Also applies to: 7-8, 20-21, 36-37, 41-58, 59-61, 63-83

packages/clerk-js/src/ui/elements/Modal.tsx (1)

2-2: Boolean portalRoot semantics are inverted: true ends up disabling portal.

The PortalConfig wiring here still has the same boolean handling problem noted previously:

  • Line 42: portalRoot is set to portalRootProp !== undefined ? portalRootProp : (portalFromContext ?? true), so when neither prop nor context is set, portalRoot defaults to true.
  • Lines 45–55: resolvePortalRoot() returns:
    • portalRoot() if it’s a function.
    • null for all booleans (true or false).
    • ref.current for refs, or the element itself otherwise.
  • Line 58: resolvedRoot = resolvePortalRoot();.
  • Lines 90–91: Popover is always called with portal={resolvedRoot ? () => resolvedRoot : false}.

Consequences:

  • For portalRoot === true (including the default case), resolvedRoot is null → Popover receives portal={false} → portalling is disabled.
  • For portalRoot === false, you also get portal={false} (expected).
  • There is currently no way for portalRoot === true to mean “portal is enabled (e.g., to default root)”, which contradicts the intended PortalConfig semantics and the “default portal” behavior you’re introducing elsewhere.

You can preserve boolean semantics by deriving a dedicated portal config from portalRoot and resolvedRoot:

-  const resolvedRoot = resolvePortalRoot();
+  const resolvedRoot = resolvePortalRoot();
+  const portal: PortalConfig =
+    typeof portalRoot === 'boolean' ? portalRoot : resolvedRoot ? () => resolvedRoot : false;
@@
-    <Popover
+    <Popover
       nodeId={nodeId}
       context={context}
       isOpen={isOpen}
       outsideElementsInert
-      portal={resolvedRoot ? () => resolvedRoot : false}
-      root={resolvedRoot || undefined}
+      portal={portal}
+      root={resolvedRoot || undefined}

With this:

  • portalRoot === falseportal === false → portalling disabled.
  • portalRoot === true (including the default) → portal === true → Popover uses its default portalling behavior (with the Popover fix, that means “portal to default root” when root is absent).
  • Function/element/ref cases continue to pass a () => resolvedRoot function, same as before.

This aligns Modal’s portalRoot behavior with the PortalConfig type while preserving existing non-boolean behavior.

Also applies to: 8-9, 24-25, 29-38, 40-56, 58-58, 90-91

🧹 Nitpick comments (5)
packages/clerk-js/src/ui/hooks/usePopover.ts (1)

15-16: Strategy integration in usePopover is correct; consider small cleanup

The new strategy prop is correctly threaded into useFloating and reflected in the returned styles, which should preserve existing behavior by defaulting to 'absolute' while allowing 'fixed' for portalled content.

You can slightly simplify the defaulting by pulling strategy out during destructuring:

-  const {
-    bubbles = false,
-    shoudFlip = true,
-    outsidePress,
-    adjustToReferenceWidth = false,
-    referenceElement,
-    canCloseModal,
-  } = props;
+  const {
+    bubbles = false,
+    shoudFlip = true,
+    outsidePress,
+    adjustToReferenceWidth = false,
+    referenceElement,
+    canCloseModal,
+    strategy = 'absolute',
+  } = props;
@@
-    nodeId,
-    strategy: props.strategy || 'absolute',
+    nodeId,
+    strategy,

Functionally identical, but it keeps the default in one place.

Also applies to: 37-45, 78-87

packages/react/src/components/uiComponents.tsx (1)

50-51: Portal wiring looks correct; consider centralizing the resolve logic

The new usePortalConfig usage and the portal = props.portal !== undefined ? props.portal : portalFromContext pattern in UserProfile, UserButton, and OrganizationSwitcher correctly:

  • Prefer explicit props.portal when provided.
  • Fall back to the PortalProvider context when not.
  • Preserve existing props while ensuring the resolved portal wins due to spread order.

This should make portal behavior predictable and override-friendly for consumers.

Given the identical pattern in multiple components, consider extracting a small helper (e.g. resolvePortalConfig(props.portal) or a hook) so future changes to the resolution rules stay consistent across all components.

Also applies to: 199-235, 248-305, 430-487

packages/clerk-js/src/ui/elements/Tooltip.tsx (1)

20-21: Tooltip portal behavior matches PortalConfig semantics

The tooltip content now correctly derives portalFromContext and interprets PortalConfig as:

  • false → render inline (no FloatingPortal).
  • undefined or true → portal with default root.
  • () => HTMLElement | null → portal with the returned root (falling back to default if it returns null).

The isMounted guard avoids resolving the root prematurely, and the updated Box/Text structure keeps positioning and transitions intact while making styles more explicit.

If you find similar portal resolution logic emerging elsewhere (Modal, Popover, Menu, etc.), it could be worth extracting a shared utility to keep behavior perfectly in sync.

Also applies to: 186-252

packages/clerk-js/src/ui/elements/Menu.tsx (1)

90-103: Update the comment to match prop‑first precedence

The logic correctly prefers asPortal from props over the context value, but the comment currently states the opposite order. Updating it to something like “Prefer asPortal prop, then context, else default to false” will better document the intended API.

packages/react/src/components/PortalProvider.tsx (1)

1-19: React PortalProvider implementation is correct; consider exporting the props type if it’s public API

This React-facing PortalProvider correctly exposes PortalConfig via context and matches how usePortalConfig is consumed in uiComponents.tsx. The minimal context shape keeps it easy to evolve.

If PortalProvider is part of the public React API surface (as suggested by its re-exports), you may want to export interface PortalProviderProps as well so consumers can type their wrappers/HOCs against it. If it’s intentionally internal, the current shape is fine.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2deadca and f806b5f.

📒 Files selected for processing (16)
  • packages/clerk-js/src/ui/components/OrganizationSwitcher/index.tsx (2 hunks)
  • packages/clerk-js/src/ui/components/UserButton/index.tsx (2 hunks)
  • packages/clerk-js/src/ui/contexts/PortalContext.tsx (1 hunks)
  • packages/clerk-js/src/ui/contexts/index.ts (1 hunks)
  • packages/clerk-js/src/ui/elements/Menu.tsx (3 hunks)
  • packages/clerk-js/src/ui/elements/Modal.tsx (3 hunks)
  • packages/clerk-js/src/ui/elements/Popover.tsx (3 hunks)
  • packages/clerk-js/src/ui/elements/Select.tsx (5 hunks)
  • packages/clerk-js/src/ui/elements/Tooltip.tsx (2 hunks)
  • packages/clerk-js/src/ui/hooks/usePopover.ts (2 hunks)
  • packages/clerk-js/src/ui/lazyModules/providers.tsx (3 hunks)
  • packages/nextjs/src/client-boundary/uiComponents.tsx (1 hunks)
  • packages/nextjs/src/index.ts (1 hunks)
  • packages/react/src/components/PortalProvider.tsx (1 hunks)
  • packages/react/src/components/index.ts (1 hunks)
  • packages/react/src/components/uiComponents.tsx (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/ui/lazyModules/providers.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (4)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/nextjs/src/client-boundary/uiComponents.tsx (1)
  • PortalProvider (29-29)
packages/nextjs/src/index.ts (1)
  • PortalProvider (42-42)
packages/react/src/components/PortalProvider.tsx (1)
  • PortalProvider (12-14)
packages/clerk-js/src/ui/components/UserButton/index.tsx (4)
packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
  • withFloatingTree (139-154)
packages/clerk-js/src/ui/contexts/components/UserButton.ts (1)
  • useUserButtonContext (13-68)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/hooks/usePopover.ts (1)
  • usePopover (26-89)
packages/react/src/components/PortalProvider.tsx (2)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • PortalProvider (11-13)
packages/react/src/components/uiComponents.tsx (1)
packages/react/src/components/PortalProvider.tsx (1)
  • usePortalConfig (17-19)
packages/clerk-js/src/ui/elements/Tooltip.tsx (1)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/components/OrganizationSwitcher/index.tsx (4)
packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
  • withFloatingTree (139-154)
packages/clerk-js/src/ui/contexts/components/OrganizationSwitcher.ts (1)
  • useOrganizationSwitcherContext (11-114)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/hooks/usePopover.ts (1)
  • usePopover (26-89)
packages/clerk-js/src/ui/elements/Modal.tsx (2)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/elements/Menu.tsx (3)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/hooks/usePopover.ts (1)
  • usePopover (26-89)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/elements/Popover.tsx (2)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/elements/Select.tsx (3)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/hooks/usePopover.ts (1)
  • usePopover (26-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
packages/react/src/components/index.ts (1)

41-41: PortalProvider barrel export looks correct

Re-exporting PortalProvider from the React components barrel aligns it with other public components and matches the new implementation; no issues from my side.

packages/nextjs/src/index.ts (1)

23-43: Next.js PortalProvider export is consistent with client boundary

Including PortalProvider in the explicit ./client-boundary/uiComponents export list follows the existing pattern and correctly exposes the React PortalProvider through the Next.js entrypoint.

packages/nextjs/src/client-boundary/uiComponents.tsx (1)

14-30: PortalProvider pass-through from @clerk/clerk-react is wired correctly

Re-exporting PortalProvider alongside the other UI components keeps the Next.js client boundary surface in sync with @clerk/clerk-react without altering behavior.

packages/clerk-js/src/ui/contexts/index.ts (1)

1-9: PortalContext barrel export matches existing pattern

Adding export * from './PortalContext'; is consistent with the other context barrel exports and cleanly surfaces the new portal context utilities.

packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)

1-18: PortalContext + hook are simple and sound

The context shape, PortalProvider implementation, and usePortalContext helper are straightforward and correctly model PortalConfig | undefined with a sensible default when no provider is present. This aligns well with the intended API surface.

Comment on lines +15 to 23
const { defaultOpen, portal: portalProp = true } = useOrganizationSwitcherContext();

// Get portal from context, fallback to prop, then default to true
const portalFromContext = usePortalContext();
const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? true);

// Detect if custom portal is used (function or from context indicates intentional custom portal)
const useFixedPosition = typeof portal === 'function' || (portal !== false && portalFromContext !== undefined);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

PortalContext fallback is effectively disabled for OrganizationSwitcher.

  • Line 15 destructures portal with a default (portal: portalProp = true), so portalProp is never undefined.
  • Lines 18–19 then compute portal as portalProp !== undefined ? portalProp : (portalFromContext ?? true), which means the portalFromContext and ?? true fallbacks are dead code.
  • As a result, PortalProvider / usePortalContext cannot influence this Popover at all; only the OrganizationSwitcher context prop can, and even that defaults to true. The comment on line 17 also no longer matches the behavior.

This breaks the intent of wiring global portal configuration through PortalContext into OrganizationSwitcher.

You can fix both the behavior and the comment with something like:

-  const { defaultOpen, portal: portalProp = true } = useOrganizationSwitcherContext();
-
-  // Get portal from context, fallback to prop, then default to true
-  const portalFromContext = usePortalContext();
-  const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? true);
+  const { defaultOpen, portal: portalProp } = useOrganizationSwitcherContext();
+
+  // Get portal from prop, fallback to context, then default to true
+  const portalFromContext = usePortalContext();
+  const portal = portalProp ?? portalFromContext ?? true;

This lets:

  • Per-component portal override the global context when provided.
  • Global PortalProvider portal config apply when the component doesn’t specify portal.
  • The default still be true when neither is set.

Also applies to: 28-29, 46-46

🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/OrganizationSwitcher/index.tsx around
lines 15 to 23, the current destructure sets portalProp default to true which
prevents PortalContext fallback from ever being used; remove the default from
the destructure so portalProp can be undefined, compute portal as portalProp ??
portalFromContext ?? true so per-component prop overrides context and context
overrides the final default, update the comment to reflect that ordering, and
adjust the useFixedPosition detection to check typeof portal === 'function' ||
(portal !== false && portalFromContext !== undefined); make the identical fixes
at the other mentioned locations (lines 28-29 and 46) to restore PortalProvider
influence.

Comment on lines +14 to 28
const { defaultOpen, portal: portalProp = true } = useUserButtonContext();

// Get portal from context, fallback to prop, then default to true
const portalFromContext = usePortalContext();
const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? true);

// Detect if custom portal is used (function or from context indicates intentional custom portal)
const useFixedPosition = typeof portal === 'function' || (portal !== false && portalFromContext !== undefined);

const { floating, reference, styles, toggle, isOpen, nodeId, context } = usePopover({
defaultOpen,
placement: 'bottom-end',
offset: 8,
strategy: useFixedPosition ? 'fixed' : 'absolute',
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Portal precedence currently ignores PortalContext and can mis-handle context false

The current portal resolution has two problems:

  1. portalFromContext is effectively never used for the portal value:

    • portalProp is destructured with a default: portal: portalProp = true, so portalProp is never undefined.
    • const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? true); therefore always picks portalProp, even when a PortalProvider is present.
    • This contradicts the comment (“Get portal from context, fallback to prop, then default to true”) and means PortalContext configuration doesn’t actually influence the portal prop passed to <Popover>.
  2. Context portal: false can’t properly disable portalling for UserButton:

    • With a PortalProvider setting portal: false, portalProp still defaults to true, portal becomes true, and useFixedPosition can still flip to true because portalFromContext !== undefined.

To match the comment and expected semantics (context → prop → default), you can remove the default from the destructuring and explicitly apply precedence:

-  const { defaultOpen, portal: portalProp = true } = useUserButtonContext();
-
-  // Get portal from context, fallback to prop, then default to true
-  const portalFromContext = usePortalContext();
-  const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? true);
-
-  // Detect if custom portal is used (function or from context indicates intentional custom portal)
-  const useFixedPosition = typeof portal === 'function' || (portal !== false && portalFromContext !== undefined);
+  const { defaultOpen, portal: portalProp } = useUserButtonContext();
+
+  // Get portal from context, fallback to prop, then default to true
+  const portalFromContext = usePortalContext();
+  const portal = portalFromContext ?? (portalProp ?? true);
+
+  // Detect if custom portal is used (function or from context indicates intentional custom portal)
+  const useFixedPosition =
+    typeof portal === 'function' || (portal !== false && portalFromContext !== undefined);

This way:

  • PortalProvider (context) takes precedence when present.
  • A portal={...} prop on UserButton still overrides the default when no context is set.
  • Explicit portal={false} from context correctly disables portalling and keeps strategy: 'absolute'.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { defaultOpen, portal: portalProp = true } = useUserButtonContext();
// Get portal from context, fallback to prop, then default to true
const portalFromContext = usePortalContext();
const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? true);
// Detect if custom portal is used (function or from context indicates intentional custom portal)
const useFixedPosition = typeof portal === 'function' || (portal !== false && portalFromContext !== undefined);
const { floating, reference, styles, toggle, isOpen, nodeId, context } = usePopover({
defaultOpen,
placement: 'bottom-end',
offset: 8,
strategy: useFixedPosition ? 'fixed' : 'absolute',
});
const { defaultOpen, portal: portalProp } = useUserButtonContext();
// Get portal from context, fallback to prop, then default to true
const portalFromContext = usePortalContext();
const portal = portalFromContext ?? (portalProp ?? true);
// Detect if custom portal is used (function or from context indicates intentional custom portal)
const useFixedPosition =
typeof portal === 'function' || (portal !== false && portalFromContext !== undefined);
const { floating, reference, styles, toggle, isOpen, nodeId, context } = usePopover({
defaultOpen,
placement: 'bottom-end',
offset: 8,
strategy: useFixedPosition ? 'fixed' : 'absolute',
});
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/UserButton/index.tsx around lines 14 to
28, the portal precedence logic is wrong and context false is ignored; remove
the default value from the portal prop destructuring (don’t set portalProp =
true), then compute portal using context first: const portal = portalFromContext
!== undefined ? portalFromContext : (portalProp !== undefined ? portalProp :
true); finally keep useFixedPosition detection as: typeof portal === 'function'
|| (portal !== false && portalFromContext !== undefined) so that a
PortalProvider can override the prop and context=false correctly disables
portalling.

@@ -1,11 +1,12 @@
import { createContextAndHook } from '@clerk/shared/react';
import type { MenuId } from '@clerk/shared/types';
import type { MenuId, PortalConfig } from '@clerk/shared/types';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align positioning strategy with the actual portal resolution and fix the misleading comment

The new portal integration largely makes sense, but there are two small points worth addressing:

  1. Comment vs. behavior (Lines 100–103)
    The comment says “Get portal from context, fallback to prop, then default to false”, but the code actually does:

    • Prefer asPortal prop when defined.
    • Else fall back to context.
    • Else default to false.

    This is the right behavior; the comment is just inverted. Updating it will avoid confusion.

  2. strategy may not reflect asPortal overrides (Lines 30–41, 90–103)
    useFixedPosition is derived solely from usePortalContext() in Menu, while MenuList can override portalling with its asPortal prop. This creates an edge case where:

    • Context sets portal = true.
    • A consumer passes <MenuList asPortal={false} />.

    In that case, the popover still uses strategy: 'fixed' even though portal={false}, which might be surprising in some layouts.

    Consider basing the positioning strategy on the resolved portal configuration (i.e., after combining prop + context) or allowing MenuList to influence the strategy, so the final portal decision and the positioning mode stay in lockstep.

Also applies to: 30-41, 90-103

🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/elements/Menu.tsx around lines 30-41 and 90-103 (and
update the comment at ~100-103), the comment about portal resolution is inverted
and the positioning strategy is derived only from context which can be out of
sync with MenuList's asPortal prop; fix by (1) updating the comment to reflect
the actual resolution order: prefer the asPortal prop when defined, else fall
back to context, else default false, and (2) compute a single resolvedPortal
value (prop overrides context) and use that resolvedPortal to decide the
positioning strategy (fixed vs absolute) so Menu and MenuList share the same
final portal decision; ensure MenuList either accepts a resolvedPortal prop or
re-computes using the same resolution logic so strategy and portal are always in
lockstep.

@@ -1,8 +1,9 @@
import { createContextAndHook } from '@clerk/shared/react';
import type { SelectId } from '@clerk/shared/types';
import type { PortalConfig, SelectId } from '@clerk/shared/types';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

PortalContext is effectively ignored for Select due to defaulted portal prop.

The Select portal wiring mirrors the OrganizationSwitcher issue:

  • Line 95: portal is destructured as portal: portalProp = false, so portalProp is never undefined.
  • Lines 100–103: const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? false); means portalFromContext and the ?? false fallback are dead code.
  • The comment says “Get portal from context, fallback to prop, then default to false”, but the actual behavior is “use prop (or its default false) and ignore context”.
  • As a result:
    • A global PortalProvider value from usePortalContext cannot configure Select by default.
    • The strategy expression on lines 108–110 only reacts to the local portal prop, not to context, so context-based portal configuration doesn’t influence positioning either.

To let Select honor PortalContext while still allowing per-instance overrides, you can:

   const {
@@
-    children,
-    portal: portalProp = false,
+    children,
+    portal: portalProp,
@@
-  // Get portal from context, fallback to prop, then default to false
-  const portalFromContext = usePortalContext();
-  const portal = portalProp !== undefined ? portalProp : (portalFromContext ?? false);
+  // Get portal from prop, fallback to context, then default to false
+  const portalFromContext = usePortalContext();
+  const portal = portalProp ?? portalFromContext ?? false;

This yields:

  • If a portal prop is provided on Select, it wins (including false to explicitly disable portalling).
  • Otherwise, PortalProvider / PortalContext controls portal behavior.
  • If neither is set, portal falls back to false, preserving the current default of “no portal” for Select.

The existing strategy logic then correctly treats function/context-backed portals as requiring fixed positioning.

Also applies to: 6-7, 34-35, 50-51, 83-99, 100-103, 108-110, 135-153, 248-263, 316-323

🤖 Prompt for AI Agents
packages/clerk-js/src/ui/elements/Select.tsx (apply fixes at ranges: 6-7, 34-35,
50-51, 83-99, 100-103, 108-110, 135-153, 248-263, 316-323): the bug is that the
`portal` prop is destructured with a default (`portal: portalProp = false`)
which makes it never undefined and therefore always overrides PortalContext;
remove the default from the destructure so `portalProp` can be undefined, then
compute: `const portal = portalProp !== undefined ? portalProp :
(portalFromContext ?? false);` (so an explicit prop—true or false—wins,
otherwise context is used, otherwise fallback to false); ensure the existing
`strategy` calculation uses this computed `portal` value and apply the same
pattern consistently wherever portal is handled in the listed ranges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
packages/clerk-js/src/ui/elements/Select.tsx (1)

95-111: Critical: Explicit portal={false} is overridden by context.

The positioning logic doesn't respect an explicit portal={false} prop when context is truthy.

Test case:

  • User sets portal={false} to disable portalling
  • A parent PortalProvider has portal={true}
  • Lines 103-105: Condition 3 evaluates to true because context is true
  • Result: useFixedPosition = true despite explicit false

This contradicts the intent of allowing per-instance overrides.

Apply this diff to prioritize explicit prop values:

-  // Only read from context to determine strategy for fixed positioning
-  const portalFromContext = usePortalContext();
-  const useFixedPosition =
-    typeof portalProp === 'function' ||
-    (portalProp !== false && portalProp !== undefined) ||
-    (portalFromContext !== undefined && portalFromContext !== false);
+  // Get portal from prop, fallback to context, then default to false
+  const portalFromContext = usePortalContext();
+  const effectivePortal = portalProp !== undefined ? portalProp : (portalFromContext ?? false);
+  const useFixedPosition = typeof effectivePortal === 'function' || effectivePortal === true;

This ensures:

  • Explicit portal={false} disables portalling (overrides context)
  • Explicit portal={true} or portal={function} enables portalling (overrides context)
  • When portalProp is undefined, context controls the behavior
  • Default is false when neither prop nor context is set
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/elements/Select.tsx (1)

323-323: Simplify redundant ternary expression.

The expression portal !== undefined ? portal : undefined is equivalent to just portal. This ternary is a no-op.

Apply this diff:

-      portal={portal !== undefined ? portal : undefined}
+      portal={portal}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f806b5f and eaa0c4e.

📒 Files selected for processing (2)
  • packages/clerk-js/src/ui/elements/Menu.tsx (4 hunks)
  • packages/clerk-js/src/ui/elements/Select.tsx (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/clerk-js/src/ui/elements/Menu.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/ui/elements/Select.tsx (3)
packages/shared/src/types/clerk.ts (1)
  • PortalConfig (71-71)
packages/clerk-js/src/ui/contexts/PortalContext.tsx (1)
  • usePortalContext (15-18)
packages/clerk-js/src/ui/hooks/usePopover.ts (1)
  • usePopover (26-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/ui/elements/Select.tsx (1)

2-2: LGTM! Type changes align with the new portal configuration approach.

The imports and type definitions correctly integrate PortalConfig and usePortalContext. Making the portal prop optional (portal?: PortalConfig) enables proper fallback to context when no explicit prop is provided.

Also applies to: 6-6, 34-34, 50-50

@alexcarpenter
Copy link
Member Author

!snapshot

@clerk-cookie
Copy link
Collaborator

Hey @alexcarpenter - the snapshot version command generated the following package versions:

Package Version
@clerk/agent-toolkit 0.2.2-snapshot.v20251117224912
@clerk/astro 2.16.0-snapshot.v20251117224912
@clerk/backend 2.23.0-snapshot.v20251117224912
@clerk/chrome-extension 2.8.2-snapshot.v20251117224912
@clerk/clerk-js 5.109.0-snapshot.v20251117224912
@clerk/elements 0.23.83-snapshot.v20251117224912
@clerk/clerk-expo 2.19.2-snapshot.v20251117224912
@clerk/expo-passkeys 0.4.19-snapshot.v20251117224912
@clerk/express 1.7.50-snapshot.v20251117224912
@clerk/fastify 2.6.2-snapshot.v20251117224912
@clerk/localizations 3.28.1-snapshot.v20251117224912
@clerk/nextjs 6.35.2-snapshot.v20251117224912
@clerk/nuxt 1.13.0-snapshot.v20251117224912
@clerk/clerk-react 5.56.0-snapshot.v20251117224912
@clerk/react-router 2.2.2-snapshot.v20251117224912
@clerk/remix 4.13.17-snapshot.v20251117224912
@clerk/shared 3.35.0-snapshot.v20251117224912
@clerk/tanstack-react-start 0.27.2-snapshot.v20251117224912
@clerk/testing 1.13.16-snapshot.v20251117224912
@clerk/themes 2.4.37-snapshot.v20251117224912
@clerk/types 4.101.0-snapshot.v20251117224912
@clerk/vue 1.16.0-snapshot.v20251117224912

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/agent-toolkit

npm i @clerk/agent-toolkit@0.2.2-snapshot.v20251117224912 --save-exact

@clerk/astro

npm i @clerk/astro@2.16.0-snapshot.v20251117224912 --save-exact

@clerk/backend

npm i @clerk/backend@2.23.0-snapshot.v20251117224912 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@2.8.2-snapshot.v20251117224912 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.109.0-snapshot.v20251117224912 --save-exact

@clerk/elements

npm i @clerk/elements@0.23.83-snapshot.v20251117224912 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@2.19.2-snapshot.v20251117224912 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@0.4.19-snapshot.v20251117224912 --save-exact

@clerk/express

npm i @clerk/express@1.7.50-snapshot.v20251117224912 --save-exact

@clerk/fastify

npm i @clerk/fastify@2.6.2-snapshot.v20251117224912 --save-exact

@clerk/localizations

npm i @clerk/localizations@3.28.1-snapshot.v20251117224912 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@6.35.2-snapshot.v20251117224912 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@1.13.0-snapshot.v20251117224912 --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.56.0-snapshot.v20251117224912 --save-exact

@clerk/react-router

npm i @clerk/react-router@2.2.2-snapshot.v20251117224912 --save-exact

@clerk/remix

npm i @clerk/remix@4.13.17-snapshot.v20251117224912 --save-exact

@clerk/shared

npm i @clerk/shared@3.35.0-snapshot.v20251117224912 --save-exact

@clerk/tanstack-react-start

npm i @clerk/tanstack-react-start@0.27.2-snapshot.v20251117224912 --save-exact

@clerk/testing

npm i @clerk/testing@1.13.16-snapshot.v20251117224912 --save-exact

@clerk/themes

npm i @clerk/themes@2.4.37-snapshot.v20251117224912 --save-exact

@clerk/types

npm i @clerk/types@4.101.0-snapshot.v20251117224912 --save-exact

@clerk/vue

npm i @clerk/vue@1.16.0-snapshot.v20251117224912 --save-exact

@alexcarpenter alexcarpenter deleted the alexcarpenter/portal-options branch November 17, 2025 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants