From 5f88603f4bf11f26819e9f457b21f9b81bac89dd Mon Sep 17 00:00:00 2001 From: Jeff Escalante Date: Wed, 20 May 2026 13:38:44 -0400 Subject: [PATCH 1/4] fix(react): prevent custom page portal remounts --- .../fix-org-profile-custom-page-remounts.md | 5 ++ .../src/components/ClerkHostRenderer.tsx | 5 +- .../__tests__/ClerkHostRenderer.test.tsx | 46 +++++++++++ .../__tests__/useCustomElementPortal.test.tsx | 77 +++++++++++++++++++ .../src/utils/useCustomElementPortal.tsx | 60 +++++++++++---- 5 files changed, 175 insertions(+), 18 deletions(-) create mode 100644 .changeset/fix-org-profile-custom-page-remounts.md create mode 100644 packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx create mode 100644 packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx diff --git a/.changeset/fix-org-profile-custom-page-remounts.md b/.changeset/fix-org-profile-custom-page-remounts.md new file mode 100644 index 00000000000..07a51925d95 --- /dev/null +++ b/.changeset/fix-org-profile-custom-page-remounts.md @@ -0,0 +1,5 @@ +--- +'@clerk/react': patch +--- + +Prevent custom pages in profile components from remounting during parent rerenders. diff --git a/packages/react/src/components/ClerkHostRenderer.tsx b/packages/react/src/components/ClerkHostRenderer.tsx index 3e1304caebc..3c67f9f8937 100644 --- a/packages/react/src/components/ClerkHostRenderer.tsx +++ b/packages/react/src/components/ClerkHostRenderer.tsx @@ -79,8 +79,9 @@ export class ClerkHostRenderer extends React.PureComponent< const newProps = without(this.props.props, 'customPages', 'customMenuItems', 'children'); // instead, we simply use the length of customPages to determine if it changed or not - const customPagesChanged = prevProps.customPages?.length !== newProps.customPages?.length; - const customMenuItemsChanged = prevProps.customMenuItems?.length !== newProps.customMenuItems?.length; + const customPagesChanged = _prevProps.props.customPages?.length !== this.props.props.customPages?.length; + const customMenuItemsChanged = + _prevProps.props.customMenuItems?.length !== this.props.props.customMenuItems?.length; // Strip out mountIcon and unmountIcon handlers since they're always generated as new function references, // which would cause unnecessary re-renders in deep equality checks diff --git a/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx b/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx new file mode 100644 index 00000000000..eca68080205 --- /dev/null +++ b/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx @@ -0,0 +1,46 @@ +import { render } from '@testing-library/react'; +import React from 'react'; +import { describe, expect, it, vi } from 'vitest'; + +import { ClerkHostRenderer } from '../ClerkHostRenderer'; + +vi.mock('@clerk/shared/object', () => ({ + without: (obj: Record, ...keys: string[]) => + Object.fromEntries(Object.entries(obj).filter(([key]) => !keys.includes(key))), +})); + +vi.mock('@clerk/shared/react', () => ({ + isDeeplyEqual: (a: unknown, b: unknown) => JSON.stringify(a) === JSON.stringify(b), +})); + +describe('', () => { + it('updates mounted component props when custom pages are added or removed', () => { + const mount = vi.fn(); + const unmount = vi.fn(); + const updateProps = vi.fn(); + + const { rerender } = render( + , + ); + + rerender( + , + ); + + expect(updateProps).toHaveBeenCalledTimes(1); + expect(updateProps).toHaveBeenCalledWith({ + node: expect.any(HTMLDivElement), + props: { customPages: [{ label: 'General' }, { label: 'Permissions' }] }, + }); + }); +}); diff --git a/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx b/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx new file mode 100644 index 00000000000..e754c124be7 --- /dev/null +++ b/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx @@ -0,0 +1,77 @@ +import { render, screen } from '@testing-library/react'; +import React, { useEffect } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { useCustomElementPortal } from '../useCustomElementPortal'; + +describe('useCustomElementPortal', () => { + let portalRoot: HTMLDivElement; + + afterEach(() => { + portalRoot?.remove(); + }); + + it('does not remount portal content when the parent rerenders', async () => { + const mountTracker = vi.fn(); + const unmountTracker = vi.fn(); + + const CustomContent = ({ label }: { label: string }) => { + useEffect(() => { + mountTracker(); + return unmountTracker; + }, []); + + return
{label}
; + }; + + const TestComponent = ({ label }: { label: string }) => { + const [{ mount, portal: Portal, unmount }] = useCustomElementPortal([ + { component: , id: 0 }, + ]); + + useEffect(() => { + mount(portalRoot); + return unmount; + }, [mount, unmount]); + + return ; + }; + + portalRoot = document.createElement('div'); + document.body.appendChild(portalRoot); + + const { rerender } = render(); + + await screen.findByText('first render'); + expect(mountTracker).toHaveBeenCalledTimes(1); + + rerender(); + + await screen.findByText('second render'); + expect(mountTracker).toHaveBeenCalledTimes(1); + expect(unmountTracker).not.toHaveBeenCalled(); + + expect(portalRoot.textContent).toBe('second render'); + }); + + it('renders falsy ReactNode values after mounting', async () => { + const TestComponent = () => { + const [{ mount, portal: Portal, unmount }] = useCustomElementPortal([{ component: 0, id: 0 }]); + + useEffect(() => { + mount(portalRoot); + return unmount; + }, [mount, unmount]); + + return ; + }; + + portalRoot = document.createElement('div'); + document.body.appendChild(portalRoot); + + render(); + + await screen.findByText('0'); + expect(portalRoot.textContent).toBe('0'); + }); +}); diff --git a/packages/react/src/utils/useCustomElementPortal.tsx b/packages/react/src/utils/useCustomElementPortal.tsx index c0bf7e39b23..193714dddd5 100644 --- a/packages/react/src/utils/useCustomElementPortal.tsx +++ b/packages/react/src/utils/useCustomElementPortal.tsx @@ -1,5 +1,5 @@ import type React from 'react'; -import { useState } from 'react'; +import { useRef, useState } from 'react'; import { createPortal } from 'react-dom'; export type UseCustomElementPortalParams = { @@ -8,7 +8,7 @@ export type UseCustomElementPortalParams = { }; export type UseCustomElementPortalReturn = { - portal: () => React.JSX.Element; + portal: React.ComponentType; mount: (node: Element) => void; unmount: () => void; id: number; @@ -18,19 +18,47 @@ export type UseCustomElementPortalReturn = { // the given component into a given node export const useCustomElementPortal = (elements: UseCustomElementPortalParams[]) => { const [nodeMap, setNodeMap] = useState>(new Map()); + const nodeMapRef = useRef(nodeMap); + const elementsRef = useRef>(new Map()); + const portalsRef = useRef>(new Map()); - return elements.map(el => ({ - id: el.id, - mount: (node: Element) => setNodeMap(prev => new Map(prev).set(String(el.id), node)), - unmount: () => - setNodeMap(prev => { - const newMap = new Map(prev); - newMap.set(String(el.id), null); - return newMap; - }), - portal: () => { - const node = nodeMap.get(String(el.id)); - return node ? createPortal(el.component, node) : null; - }, - })); + nodeMapRef.current = nodeMap; + elementsRef.current = new Map(elements.map(el => [String(el.id), el.component])); + + const elementIds = new Set(elements.map(el => String(el.id))); + portalsRef.current.forEach((_, id) => { + if (!elementIds.has(id)) { + portalsRef.current.delete(id); + } + }); + + return elements.map(el => { + const id = String(el.id); + const existingPortal = portalsRef.current.get(id); + + if (existingPortal) { + return existingPortal; + } + + const portal: React.ComponentType = () => { + const node = nodeMapRef.current.get(id); + const component = elementsRef.current.get(id); + return node ? createPortal(component, node) : null; + }; + + const customElementPortal = { + id: el.id, + mount: (node: Element) => setNodeMap(prev => new Map(prev).set(id, node)), + unmount: () => + setNodeMap(prev => { + const newMap = new Map(prev); + newMap.set(id, null); + return newMap; + }), + portal, + }; + + portalsRef.current.set(id, customElementPortal); + return customElementPortal; + }); }; From 8d3a5c9f88c8763c1ff5720578a78c48b58bf82b Mon Sep 17 00:00:00 2001 From: Jeff Escalante Date: Wed, 20 May 2026 14:05:35 -0400 Subject: [PATCH 2/4] Address custom portal review feedback --- .../__tests__/useCustomMenuItems.test.tsx | 37 +++++++++++++ .../utils/__tests__/useCustomPages.test.tsx | 54 +++++++++++++++++++ .../src/utils/useCustomElementPortal.tsx | 4 +- .../react/src/utils/useCustomMenuItems.tsx | 28 +++++++--- packages/react/src/utils/useCustomPages.tsx | 34 ++++++++---- 5 files changed, 140 insertions(+), 17 deletions(-) create mode 100644 packages/react/src/utils/__tests__/useCustomPages.test.tsx diff --git a/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx b/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx index b748226b250..686c9d8188c 100644 --- a/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx +++ b/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx @@ -135,4 +135,41 @@ describe('useUserButtonCustomMenuItems', () => { expect(mockOnClick).toHaveBeenCalledTimes(1); } }); + + it('keeps portal identity with the logical menu item when inserting before it', () => { + const firstItem = ( + First icon} + onClick={() => {}} + /> + ); + const secondItem = ( + Second icon} + onClick={() => {}} + /> + ); + const makeChildren = (includeFirstItem: boolean) => ( + {includeFirstItem ? [firstItem, secondItem] : [secondItem]} + ); + + const { result, rerender } = renderHook( + ({ includeFirstItem }) => useUserButtonCustomMenuItems(makeChildren(includeFirstItem)), + { + initialProps: { includeFirstItem: false }, + }, + ); + + const secondItemIconPortal = result.current.customMenuItemsPortals[0]; + + rerender({ includeFirstItem: true }); + + expect(result.current.customMenuItems).toHaveLength(2); + expect(result.current.customMenuItemsPortals[0]).not.toBe(secondItemIconPortal); + expect(result.current.customMenuItemsPortals[1]).toBe(secondItemIconPortal); + }); }); diff --git a/packages/react/src/utils/__tests__/useCustomPages.test.tsx b/packages/react/src/utils/__tests__/useCustomPages.test.tsx new file mode 100644 index 00000000000..16853881cde --- /dev/null +++ b/packages/react/src/utils/__tests__/useCustomPages.test.tsx @@ -0,0 +1,54 @@ +import { renderHook } from '@testing-library/react'; +import React from 'react'; +import { describe, expect, it, vi } from 'vitest'; + +import { OrganizationProfilePage } from '../../components/uiComponents'; +import { useOrganizationProfileCustomPages } from '../useCustomPages'; + +vi.mock('@clerk/shared', () => ({ + logErrorInDevMode: vi.fn(), +})); + +describe('useOrganizationProfileCustomPages', () => { + it('keeps portal identity with the logical custom page when inserting before it', () => { + const firstPage = ( + First icon} + url='first' + > +
First content
+
+ ); + const secondPage = ( + Second icon} + url='second' + > +
Second content
+
+ ); + const makeChildren = (includeFirstPage: boolean) => (includeFirstPage ? [firstPage, secondPage] : [secondPage]); + + const { result, rerender } = renderHook( + ({ includeFirstPage }) => useOrganizationProfileCustomPages(makeChildren(includeFirstPage)), + { + initialProps: { includeFirstPage: false }, + }, + ); + + const secondPageContentPortal = result.current.customPagesPortals[0]; + const secondPageIconPortal = result.current.customPagesPortals[1]; + + rerender({ includeFirstPage: true }); + + expect(result.current.customPages).toHaveLength(2); + expect(result.current.customPagesPortals[0]).not.toBe(secondPageContentPortal); + expect(result.current.customPagesPortals[1]).not.toBe(secondPageIconPortal); + expect(result.current.customPagesPortals[2]).toBe(secondPageContentPortal); + expect(result.current.customPagesPortals[3]).toBe(secondPageIconPortal); + }); +}); diff --git a/packages/react/src/utils/useCustomElementPortal.tsx b/packages/react/src/utils/useCustomElementPortal.tsx index 193714dddd5..24e9764f3f3 100644 --- a/packages/react/src/utils/useCustomElementPortal.tsx +++ b/packages/react/src/utils/useCustomElementPortal.tsx @@ -4,14 +4,14 @@ import { createPortal } from 'react-dom'; export type UseCustomElementPortalParams = { component: React.ReactNode; - id: number; + id: string | number; }; export type UseCustomElementPortalReturn = { portal: React.ComponentType; mount: (node: Element) => void; unmount: () => void; - id: number; + id: string | number; }; // This function takes a component as prop, and returns functions that mount and unmount diff --git a/packages/react/src/utils/useCustomMenuItems.tsx b/packages/react/src/utils/useCustomMenuItems.tsx index 59385ea423d..a7641962b1b 100644 --- a/packages/react/src/utils/useCustomMenuItems.tsx +++ b/packages/react/src/utils/useCustomMenuItems.tsx @@ -43,7 +43,7 @@ type UseCustomMenuItemsParams = { allowForAnyChildren?: boolean; }; -type CustomMenuItemType = UserButtonActionProps | UserButtonLinkProps; +type CustomMenuItemType = (UserButtonActionProps | UserButtonLinkProps) & { portalId?: string }; const useCustomMenuItems = ({ children, @@ -89,6 +89,7 @@ const useCustomMenuItems = ({ } const { props } = child as ReactElement; + const childKey = (child as ReactElement).key; const { label, labelIcon, href, onClick, open } = props; @@ -106,11 +107,13 @@ const useCustomMenuItems = ({ validChildren.push({ ...baseItem, onClick, + portalId: getCustomMenuItemPortalId('action', props, childKey), }); } else if (open !== undefined) { validChildren.push({ ...baseItem, open: open.startsWith('/') ? open : `/${open}`, + portalId: getCustomMenuItemPortalId('action', props, childKey), }); } else { // Handle the case where neither onClick nor open is defined @@ -125,7 +128,7 @@ const useCustomMenuItems = ({ if (isThatComponent(child, MenuLinkComponent)) { if (isExternalLink(props)) { - validChildren.push({ label, labelIcon, href }); + validChildren.push({ label, labelIcon, href, portalId: getCustomMenuItemPortalId('link', props, childKey) }); } else { logErrorInDevMode(userButtonMenuItemLinkWrongProps); return; @@ -138,10 +141,10 @@ const useCustomMenuItems = ({ const customLinkLabelIcons: UseCustomElementPortalParams[] = []; validChildren.forEach((mi, index) => { if (isCustomMenuItem(mi)) { - customMenuItemLabelIcons.push({ component: mi.labelIcon, id: index }); + customMenuItemLabelIcons.push({ component: mi.labelIcon, id: mi.portalId || index }); } if (isExternalLink(mi)) { - customLinkLabelIcons.push({ component: mi.labelIcon, id: index }); + customLinkLabelIcons.push({ component: mi.labelIcon, id: mi.portalId || index }); } }); @@ -159,7 +162,7 @@ const useCustomMenuItems = ({ portal: iconPortal, mount: mountIcon, unmount: unmountIcon, - } = customMenuItemLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn; + } = customMenuItemLabelIconsPortals.find(p => p.id === (mi.portalId || index)) as UseCustomElementPortalReturn; const menuItem: CustomMenuItem = { label: mi.label, mountIcon, @@ -179,7 +182,7 @@ const useCustomMenuItems = ({ portal: iconPortal, mount: mountIcon, unmount: unmountIcon, - } = customLinkLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn; + } = customLinkLabelIconsPortals.find(p => p.id === (mi.portalId || index)) as UseCustomElementPortalReturn; customMenuItems.push({ label: mi.label, href: mi.href, @@ -193,6 +196,19 @@ const useCustomMenuItems = ({ return { customMenuItems, customMenuItemsPortals }; }; +const getCustomMenuItemPortalId = ( + type: 'action' | 'link', + props: Pick & { href?: string; open?: string }, + key: React.Key | null, +) => { + if (key != null) { + return `${type}:key:${key}`; + } + + const target = props.href || props.open || ''; + return `${type}:${props.label}:${target}`; +}; + const isReorderItem = (childProps: any, validItems: string[]): boolean => { const { children, label, onClick, labelIcon } = childProps; return !children && !onClick && !labelIcon && validItems.some(v => v === label); diff --git a/packages/react/src/utils/useCustomPages.tsx b/packages/react/src/utils/useCustomPages.tsx index 6a271188616..340e180c144 100644 --- a/packages/react/src/utils/useCustomPages.tsx +++ b/packages/react/src/utils/useCustomPages.tsx @@ -64,7 +64,7 @@ type UseCustomPagesOptions = { allowForAnyChildren: boolean; }; -type CustomPageWithIdType = UserProfilePageProps & { children?: React.ReactNode }; +type CustomPageWithIdType = UserProfilePageProps & { children?: React.ReactNode; portalId?: string }; /** * Exclude any children that is used for identifying Custom Pages or Custom Items. @@ -121,13 +121,21 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp const { children, label, url, labelIcon } = props; + const childKey = (child as ReactElement).key; + if (isThatComponent(child, PageComponent)) { if (isReorderItem(props, reorderItemsLabels)) { // This is a reordering item validChildren.push({ label }); } else if (isCustomPage(props)) { // this is a custom page - validChildren.push({ label, labelIcon, children, url }); + validChildren.push({ + label, + labelIcon, + children, + url, + portalId: getCustomPagePortalId('page', props, childKey), + }); } else { logErrorInDevMode(customPageWrongProps(componentName)); return; @@ -137,7 +145,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp if (isThatComponent(child, LinkComponent)) { if (isExternalLink(props)) { // This is an external link - validChildren.push({ label, labelIcon, url }); + validChildren.push({ label, labelIcon, url, portalId: getCustomPagePortalId('link', props, childKey) }); } else { logErrorInDevMode(customLinkWrongProps(componentName)); return; @@ -151,12 +159,12 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp validChildren.forEach((cp, index) => { if (isCustomPage(cp)) { - customPageContents.push({ component: cp.children, id: index }); - customPageLabelIcons.push({ component: cp.labelIcon, id: index }); + customPageContents.push({ component: cp.children, id: cp.portalId || index }); + customPageLabelIcons.push({ component: cp.labelIcon, id: cp.portalId || index }); return; } if (isExternalLink(cp)) { - customLinkLabelIcons.push({ component: cp.labelIcon, id: index }); + customLinkLabelIcons.push({ component: cp.labelIcon, id: cp.portalId || index }); } }); @@ -177,12 +185,12 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp portal: contentPortal, mount, unmount, - } = customPageContentsPortals.find(p => p.id === index) as UseCustomElementPortalReturn; + } = customPageContentsPortals.find(p => p.id === (cp.portalId || index)) as UseCustomElementPortalReturn; const { portal: labelPortal, mount: mountIcon, unmount: unmountIcon, - } = customPageLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn; + } = customPageLabelIconsPortals.find(p => p.id === (cp.portalId || index)) as UseCustomElementPortalReturn; customPages.push({ label: cp.label, url: cp.url, mount, unmount, mountIcon, unmountIcon }); customPagesPortals.push(contentPortal); customPagesPortals.push(labelPortal); @@ -193,7 +201,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp portal: labelPortal, mount: mountIcon, unmount: unmountIcon, - } = customLinkLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn; + } = customLinkLabelIconsPortals.find(p => p.id === (cp.portalId || index)) as UseCustomElementPortalReturn; customPages.push({ label: cp.label, url: cp.url, mountIcon, unmountIcon }); customPagesPortals.push(labelPortal); return; @@ -203,6 +211,14 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp return { customPages, customPagesPortals }; }; +const getCustomPagePortalId = ( + type: 'page' | 'link', + props: Pick, + key: React.Key | null, +) => { + return key == null ? `${type}:${props.label}:${props.url}` : `${type}:key:${key}`; +}; + const isReorderItem = (childProps: any, validItems: string[]): boolean => { const { children, label, url, labelIcon } = childProps; return !children && !url && !labelIcon && validItems.some(v => v === label); From 7a56740a05d66ff3e817608dc44060cb37a3a80d Mon Sep 17 00:00:00 2001 From: Jeff Escalante Date: Thu, 21 May 2026 14:28:24 -0400 Subject: [PATCH 3/4] Ensure custom portal fallback IDs are unique --- .../__tests__/useCustomMenuItems.test.tsx | 23 ++++++++++++++ .../utils/__tests__/useCustomPages.test.tsx | 30 +++++++++++++++++++ .../react/src/utils/useCustomMenuItems.tsx | 18 ++++++++--- packages/react/src/utils/useCustomPages.tsx | 20 +++++++++++-- 4 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx b/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx index 686c9d8188c..ba6821158b2 100644 --- a/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx +++ b/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx @@ -136,6 +136,29 @@ describe('useUserButtonCustomMenuItems', () => { } }); + it('uses separate portals for duplicate non-keyed menu items', () => { + const children = ( + + First icon} + open='/duplicate' + /> + Second icon} + open='/duplicate' + /> + + ); + + const { result } = renderHook(() => useUserButtonCustomMenuItems(children)); + + expect(result.current.customMenuItems).toHaveLength(2); + expect(result.current.customMenuItemsPortals).toHaveLength(2); + expect(result.current.customMenuItemsPortals[0]).not.toBe(result.current.customMenuItemsPortals[1]); + }); + it('keeps portal identity with the logical menu item when inserting before it', () => { const firstItem = ( ({ })); describe('useOrganizationProfileCustomPages', () => { + it('uses separate portals for duplicate non-keyed custom pages', () => { + const children = [ + React.createElement( + OrganizationProfilePage, + { + label: 'Duplicate', + labelIcon:
First icon
, + url: 'duplicate', + }, +
First content
, + ), + React.createElement( + OrganizationProfilePage, + { + label: 'Duplicate', + labelIcon:
Second icon
, + url: 'duplicate', + }, +
Second content
, + ), + ]; + + const { result } = renderHook(() => useOrganizationProfileCustomPages(children)); + + expect(result.current.customPages).toHaveLength(2); + expect(result.current.customPagesPortals).toHaveLength(4); + expect(result.current.customPagesPortals[0]).not.toBe(result.current.customPagesPortals[2]); + expect(result.current.customPagesPortals[1]).not.toBe(result.current.customPagesPortals[3]); + }); + it('keeps portal identity with the logical custom page when inserting before it', () => { const firstPage = ( (); React.Children.forEach(children, child => { if ( @@ -107,13 +108,13 @@ const useCustomMenuItems = ({ validChildren.push({ ...baseItem, onClick, - portalId: getCustomMenuItemPortalId('action', props, childKey), + portalId: getCustomMenuItemPortalId('action', props, childKey, portalIdCounts), }); } else if (open !== undefined) { validChildren.push({ ...baseItem, open: open.startsWith('/') ? open : `/${open}`, - portalId: getCustomMenuItemPortalId('action', props, childKey), + portalId: getCustomMenuItemPortalId('action', props, childKey, portalIdCounts), }); } else { // Handle the case where neither onClick nor open is defined @@ -128,7 +129,12 @@ const useCustomMenuItems = ({ if (isThatComponent(child, MenuLinkComponent)) { if (isExternalLink(props)) { - validChildren.push({ label, labelIcon, href, portalId: getCustomMenuItemPortalId('link', props, childKey) }); + validChildren.push({ + label, + labelIcon, + href, + portalId: getCustomMenuItemPortalId('link', props, childKey, portalIdCounts), + }); } else { logErrorInDevMode(userButtonMenuItemLinkWrongProps); return; @@ -200,13 +206,17 @@ const getCustomMenuItemPortalId = ( type: 'action' | 'link', props: Pick & { href?: string; open?: string }, key: React.Key | null, + portalIdCounts: Map, ) => { if (key != null) { return `${type}:key:${key}`; } const target = props.href || props.open || ''; - return `${type}:${props.label}:${target}`; + const baseId = `${type}:${props.label}:${target}`; + const occurrence = portalIdCounts.get(baseId) ?? 0; + portalIdCounts.set(baseId, occurrence + 1); + return `${baseId}:${occurrence}`; }; const isReorderItem = (childProps: any, validItems: string[]): boolean => { diff --git a/packages/react/src/utils/useCustomPages.tsx b/packages/react/src/utils/useCustomPages.tsx index 340e180c144..e0a153338d6 100644 --- a/packages/react/src/utils/useCustomPages.tsx +++ b/packages/react/src/utils/useCustomPages.tsx @@ -104,6 +104,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp const { children, LinkComponent, PageComponent, MenuItemsComponent, reorderItemsLabels, componentName } = params; const { allowForAnyChildren = false } = options || {}; const validChildren: CustomPageWithIdType[] = []; + const portalIdCounts = new Map(); React.Children.forEach(children, child => { if ( @@ -134,7 +135,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp labelIcon, children, url, - portalId: getCustomPagePortalId('page', props, childKey), + portalId: getCustomPagePortalId('page', props, childKey, portalIdCounts), }); } else { logErrorInDevMode(customPageWrongProps(componentName)); @@ -145,7 +146,12 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp if (isThatComponent(child, LinkComponent)) { if (isExternalLink(props)) { // This is an external link - validChildren.push({ label, labelIcon, url, portalId: getCustomPagePortalId('link', props, childKey) }); + validChildren.push({ + label, + labelIcon, + url, + portalId: getCustomPagePortalId('link', props, childKey, portalIdCounts), + }); } else { logErrorInDevMode(customLinkWrongProps(componentName)); return; @@ -215,8 +221,16 @@ const getCustomPagePortalId = ( type: 'page' | 'link', props: Pick, key: React.Key | null, + portalIdCounts: Map, ) => { - return key == null ? `${type}:${props.label}:${props.url}` : `${type}:key:${key}`; + if (key != null) { + return `${type}:key:${key}`; + } + + const baseId = `${type}:${props.label}:${props.url}`; + const occurrence = portalIdCounts.get(baseId) ?? 0; + portalIdCounts.set(baseId, occurrence + 1); + return `${baseId}:${occurrence}`; }; const isReorderItem = (childProps: any, validItems: string[]): boolean => { From f82b0ab26bc182d82e673a4382793d95f666ea09 Mon Sep 17 00:00:00 2001 From: Jeff Escalante Date: Fri, 22 May 2026 16:54:15 -0400 Subject: [PATCH 4/4] Address custom portal review edge cases --- .../src/components/ClerkHostRenderer.tsx | 12 +++--- .../__tests__/ClerkHostRenderer.test.tsx | 28 ++++++++++++++ .../__tests__/useCustomElementPortal.test.tsx | 38 +++++++++++++++++++ .../src/utils/useCustomElementPortal.tsx | 14 ++++--- 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/packages/react/src/components/ClerkHostRenderer.tsx b/packages/react/src/components/ClerkHostRenderer.tsx index 3c67f9f8937..886d7057a8d 100644 --- a/packages/react/src/components/ClerkHostRenderer.tsx +++ b/packages/react/src/components/ClerkHostRenderer.tsx @@ -75,18 +75,18 @@ export class ClerkHostRenderer extends React.PureComponent< // Remove children and customPages from props before comparing // children might hold circular references which deepEqual can't handle // and the implementation of customPages relies on props getting new references - const prevProps = without(_prevProps.props, 'customPages', 'customMenuItems', 'children'); - const newProps = without(this.props.props, 'customPages', 'customMenuItems', 'children'); + const prevProps = without(_prevProps.props || {}, 'customPages', 'customMenuItems', 'children'); + const newProps = without(this.props.props || {}, 'customPages', 'customMenuItems', 'children'); // instead, we simply use the length of customPages to determine if it changed or not - const customPagesChanged = _prevProps.props.customPages?.length !== this.props.props.customPages?.length; + const customPagesChanged = _prevProps.props?.customPages?.length !== this.props.props?.customPages?.length; const customMenuItemsChanged = - _prevProps.props.customMenuItems?.length !== this.props.props.customMenuItems?.length; + _prevProps.props?.customMenuItems?.length !== this.props.props?.customMenuItems?.length; // Strip out mountIcon and unmountIcon handlers since they're always generated as new function references, // which would cause unnecessary re-renders in deep equality checks - const prevMenuItemsWithoutHandlers = stripMenuItemIconHandlers(_prevProps.props.customMenuItems); - const newMenuItemsWithoutHandlers = stripMenuItemIconHandlers(this.props.props.customMenuItems); + const prevMenuItemsWithoutHandlers = stripMenuItemIconHandlers(_prevProps.props?.customMenuItems); + const newMenuItemsWithoutHandlers = stripMenuItemIconHandlers(this.props.props?.customMenuItems); if ( !isDeeplyEqual(prevProps, newProps) || diff --git a/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx b/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx index eca68080205..02f06a08237 100644 --- a/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx +++ b/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx @@ -14,6 +14,34 @@ vi.mock('@clerk/shared/react', () => ({ })); describe('', () => { + it('does not throw when mounted component props are omitted during updates', () => { + const mount = vi.fn(); + const unmount = vi.fn(); + const updateProps = vi.fn(); + + const { rerender } = render( + , + ); + + expect(() => + rerender( + , + ), + ).not.toThrow(); + + expect(updateProps).not.toHaveBeenCalled(); + }); + it('updates mounted component props when custom pages are added or removed', () => { const mount = vi.fn(); const unmount = vi.fn(); diff --git a/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx b/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx index e754c124be7..4e0bf2504c7 100644 --- a/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx +++ b/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx @@ -74,4 +74,42 @@ describe('useCustomElementPortal', () => { await screen.findByText('0'); expect(portalRoot.textContent).toBe('0'); }); + + it('keeps string and number ids in separate portal caches', async () => { + const TestComponent = () => { + const [ + { mount: mountNumber, portal: NumberPortal, unmount: unmountNumber }, + { mount: mountString, portal: StringPortal, unmount: unmountString }, + ] = useCustomElementPortal([ + { component:
number id
, id: 1 }, + { component:
string id
, id: '1' }, + ]); + + useEffect(() => { + mountNumber(portalRoot); + mountString(portalRoot); + + return () => { + unmountNumber(); + unmountString(); + }; + }, [mountNumber, mountString, unmountNumber, unmountString]); + + return ( + <> + + + + ); + }; + + portalRoot = document.createElement('div'); + document.body.appendChild(portalRoot); + + render(); + + await screen.findByText('number id'); + await screen.findByText('string id'); + expect(portalRoot.textContent).toBe('number idstring id'); + }); }); diff --git a/packages/react/src/utils/useCustomElementPortal.tsx b/packages/react/src/utils/useCustomElementPortal.tsx index 24e9764f3f3..0ac9d20b9a3 100644 --- a/packages/react/src/utils/useCustomElementPortal.tsx +++ b/packages/react/src/utils/useCustomElementPortal.tsx @@ -14,18 +14,20 @@ export type UseCustomElementPortalReturn = { id: string | number; }; +type PortalKey = string | number; + // This function takes a component as prop, and returns functions that mount and unmount // the given component into a given node export const useCustomElementPortal = (elements: UseCustomElementPortalParams[]) => { - const [nodeMap, setNodeMap] = useState>(new Map()); + const [nodeMap, setNodeMap] = useState>(new Map()); const nodeMapRef = useRef(nodeMap); - const elementsRef = useRef>(new Map()); - const portalsRef = useRef>(new Map()); + const elementsRef = useRef>(new Map()); + const portalsRef = useRef>(new Map()); nodeMapRef.current = nodeMap; - elementsRef.current = new Map(elements.map(el => [String(el.id), el.component])); + elementsRef.current = new Map(elements.map(el => [el.id, el.component])); - const elementIds = new Set(elements.map(el => String(el.id))); + const elementIds = new Set(elements.map(el => el.id)); portalsRef.current.forEach((_, id) => { if (!elementIds.has(id)) { portalsRef.current.delete(id); @@ -33,7 +35,7 @@ export const useCustomElementPortal = (elements: UseCustomElementPortalParams[]) }); return elements.map(el => { - const id = String(el.id); + const id = el.id; const existingPortal = portalsRef.current.get(id); if (existingPortal) {