fix(react): Prevent custom page remounts in profile components#8604
fix(react): Prevent custom page remounts in profile components#8604jescalan wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 93d045e The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
|
@codex review |
jescalan
changed the title
|
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:
📝 WalkthroughWalkthroughThis PR refactors useCustomElementPortal to use ref-backed portal caching and widens portal id types and portal API to a React.ComponentType. It adds stable portalId generation for custom menu items and custom pages, updates ClerkHostRenderer to compare previous/current customPages/customMenuItems lengths with null-safe props handling, and introduces tests that verify portal persistence, identity across insertions, and rendering of falsy content. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 592c4eee6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/react/src/utils/useCustomMenuItems.tsx`:
- Around line 199-210: getCustomMenuItemPortalId is currently generating
non-keyed fallback IDs from label+target which can collide; change its contract
so fallback IDs are collision-safe: either require the caller to pass a stable
fallback identifier (e.g., an index or generated uid) and include that in the
returned string, or make getCustomMenuItemPortalId append a stable per-item uid
(not just label/target) so two items with identical label/target get distinct
IDs; if you opt to require explicit keys, make the function throw/console.error
when no stable id/key is supplied so callers must fix duplication. Update all
call sites that invoke getCustomMenuItemPortalId (where lookup happens) to pass
the new stable identifier or handle the thrown error so portal lookups (the
places using the id) never alias different menu items.
🪄 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: e9065694-5486-4e33-9310-c73644931340
📒 Files selected for processing (5)
packages/react/src/utils/__tests__/useCustomMenuItems.test.tsxpackages/react/src/utils/__tests__/useCustomPages.test.tsxpackages/react/src/utils/useCustomElementPortal.tsxpackages/react/src/utils/useCustomMenuItems.tsxpackages/react/src/utils/useCustomPages.tsx
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@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: |
jescalan
force-pushed
the
je/orgprofile-custom-pages-rendering-bug
branch
from
c85645f to
6bde33b
Compare
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/react/src/utils/useCustomElementPortal.tsx`:
- Around line 20-23: The hook useCustomElementPortal currently coerces all
portal IDs to strings (String(id)) causing numeric and string IDs to collide;
update the internal maps (nodeMap, nodeMapRef, elementsRef, portalsRef) to key
by the original id type (string | number) instead of stringified keys and change
any places that call String(id) (around the blocks indicated at lines 25-36 and
37-61) to use the raw id value; also update the Map generic types from
Map<string, ...> to Map<string | number, ...> and ensure all lookups, sets and
refs consistently use the id as the key so numeric and string IDs remain
distinct.
🪄 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: acb48737-0ce2-4089-953a-9fc73361a24c
📒 Files selected for processing (9)
.changeset/fix-org-profile-custom-page-remounts.mdpackages/react/src/components/ClerkHostRenderer.tsxpackages/react/src/components/__tests__/ClerkHostRenderer.test.tsxpackages/react/src/utils/__tests__/useCustomElementPortal.test.tsxpackages/react/src/utils/__tests__/useCustomMenuItems.test.tsxpackages/react/src/utils/__tests__/useCustomPages.test.tsxpackages/react/src/utils/useCustomElementPortal.tsxpackages/react/src/utils/useCustomMenuItems.tsxpackages/react/src/utils/useCustomPages.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/react/src/utils/tests/useCustomMenuItems.test.tsx
- .changeset/fix-org-profile-custom-page-remounts.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bde33bc81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jescalan
force-pushed
the
je/orgprofile-custom-pages-rendering-bug
branch
from
6bde33b to
f82b0ab
Compare
|
@codex review |
|
@wobsoriano @anagstef any chance you might be able to take a look here? 🙏 |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Break Check: no API changes detected across the tracked packages. |
jacekradko
left a comment
jacekradko
left a comment
There was a problem hiding this comment.
Verified this addresses the remounts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
integration/templates/react-vite/src/custom-user-profile/index.tsx (1)
15-27: ⚡ Quick winAdd explicit
type="button"to the new controls.These buttons currently rely on the implicit default of
submit, which makes the template fragile if it ever ends up inside a form.♻️ Suggested fix
<button + type='button' data-page={1} onClick={() => setCounter(a => a + 1)} > Update </button> @@ <button + type='button' data-local-counter={1} onClick={() => setLocalCounter(a => a + 1)} > Increment local </button> @@ <button + type='button' data-testid='rerender-parent' onClick={() => setParentTick(t => t + 1)} > Rerender parent: {parentTick} </button>As per coding guidelines,
Use proper button types in React components.Also applies to: 40-45
🤖 Prompt for 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. In `@integration/templates/react-vite/src/custom-user-profile/index.tsx` around lines 15 - 27, The two new button elements that call setCounter and setLocalCounter (and the similar buttons around the setLocalCounter at lines ~40-45) currently rely on the default submit behavior; update each <button> used as a control (the ones invoking setCounter and setLocalCounter in this component) to include an explicit type="button" attribute so they do not submit enclosing forms unexpectedly.
🤖 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.
Nitpick comments:
In `@integration/templates/react-vite/src/custom-user-profile/index.tsx`:
- Around line 15-27: The two new button elements that call setCounter and
setLocalCounter (and the similar buttons around the setLocalCounter at lines
~40-45) currently rely on the default submit behavior; update each <button> used
as a control (the ones invoking setCounter and setLocalCounter in this
component) to include an explicit type="button" attribute so they do not submit
enclosing forms unexpectedly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5f6d5efb-b086-466a-afa0-d3bc85011d45
📒 Files selected for processing (9)
.changeset/custom-page-portal-stable-keys.mdintegration/templates/react-vite/src/custom-user-profile/index.tsxintegration/tests/custom-pages.test.tspackages/react/src/components/uiComponents.tsxpackages/react/src/utils/__tests__/customPages.remount.integration.test.tsxpackages/react/src/utils/__tests__/useCustomMenuItems.test.tsxpackages/react/src/utils/__tests__/useCustomPages.test.tsxpackages/react/src/utils/useCustomMenuItems.tsxpackages/react/src/utils/useCustomPages.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/react/src/components/uiComponents.tsx
- .changeset/custom-page-portal-stable-keys.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/react/src/utils/tests/useCustomMenuItems.test.tsx
- packages/react/src/utils/tests/useCustomPages.test.tsx
- packages/react/src/utils/useCustomPages.tsx
- packages/react/src/utils/useCustomMenuItems.tsx
2 participants
Summary
Prevents custom pages rendered through React profile components from remounting on parent rerenders. The portal wrapper now keeps stable portal component identities while still rendering the latest React node into the Clerk-owned mount node, so customer custom page trees do not unmount/remount during expected Clerk rerenders such as focus-driven data refreshes. Also fixes custom page/menu length change detection in the host renderer.
Changes in this repo
Stabilizes custom element portal identities in @clerk/react, fixes customPages/customMenuItems length change detection in ClerkHostRenderer, and adds focused regressions for rerender-without-remount, falsy ReactNode portal content, and custom page length updates.