diff --git a/.changeset/custom-page-portal-stable-keys.md b/.changeset/custom-page-portal-stable-keys.md new file mode 100644 index 00000000000..258c9320a0b --- /dev/null +++ b/.changeset/custom-page-portal-stable-keys.md @@ -0,0 +1,5 @@ +--- +'@clerk/react': patch +--- + +Keep custom pages and menu items mounted when sibling pages are added, removed, or reordered. Portals are now keyed by a stable id rather than their array index, so a surviving page is reconciled as an update instead of being remounted. 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/integration/templates/react-vite/src/custom-user-profile/index.tsx b/integration/templates/react-vite/src/custom-user-profile/index.tsx index f44e25f0e90..d1d94711aee 100644 --- a/integration/templates/react-vite/src/custom-user-profile/index.tsx +++ b/integration/templates/react-vite/src/custom-user-profile/index.tsx @@ -1,9 +1,12 @@ import { UserProfile } from '@clerk/react'; -import { useContext } from 'react'; +import { useContext, useState } from 'react'; import { PageContext, PageContextProvider } from '../PageContext.tsx'; function Page1() { const { counter, setCounter } = useContext(PageContext); + // Local state lives INSIDE the portaled custom page. It only resets if the + // page is remounted, so it is our instrument for detecting remounts. + const [localCounter, setLocalCounter] = useState(0); return ( <> @@ -15,13 +18,31 @@ function Page1() { > Update +

Local counter: {localCounter}

+ ); } export default function Page() { + // Bumping parent state recreates the element, forcing the + // profile component (and useCustomPages) to rerender. The custom page content + // must survive this without remounting. + const [parentTick, setParentTick] = useState(0); + return ( + Loading user profile} path={'/custom-user-profile'} diff --git a/integration/tests/custom-pages.test.ts b/integration/tests/custom-pages.test.ts index aa7892332f3..0636e964c71 100644 --- a/integration/tests/custom-pages.test.ts +++ b/integration/tests/custom-pages.test.ts @@ -171,6 +171,37 @@ testAgainstRunningApps({ withPattern: ['react.vite.withEmailCodes'] })( }); }); + test('custom profile page survives a parent rerender without remounting', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + await u.po.signIn.goTo(); + await u.po.signIn.waitForMounted(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + + await u.page.goToRelative(CUSTOM_PROFILE_PAGE); + await u.po.userProfile.waitForMounted(); + + // Open the custom page (Page 1) + const [profilePage] = await u.page.locator('button.cl-navbarButton__custom-page-0').all(); + await profilePage.click(); + + // Local state lives inside the portaled custom page and starts at 0. + await u.page.waitForSelector('p[data-local-counter="1"]', { state: 'attached' }); + await expect(u.page.locator('p[data-local-counter="1"]')).toHaveText('Local counter: 0'); + + // Mutate the local state to 2. + await u.page.locator('button[data-local-counter="1"]').click(); + await u.page.locator('button[data-local-counter="1"]').click(); + await expect(u.page.locator('p[data-local-counter="1"]')).toHaveText('Local counter: 2'); + + // Force a parent rerender: this re-creates the element and reruns useCustomPages. + await u.page.locator('button[data-testid="rerender-parent"]').click(); + await expect(u.page.locator('button[data-testid="rerender-parent"]')).toHaveText('Rerender parent: 1'); + + // The custom page must NOT remount, so its local state is preserved. + await expect(u.page.locator('p[data-local-counter="1"]')).toHaveText('Local counter: 2'); + }); + test.describe('User Button with experimental asStandalone and asProvider', () => { test('items at the specified order', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); diff --git a/packages/react/src/components/ClerkHostRenderer.tsx b/packages/react/src/components/ClerkHostRenderer.tsx index 3e1304caebc..886d7057a8d 100644 --- a/packages/react/src/components/ClerkHostRenderer.tsx +++ b/packages/react/src/components/ClerkHostRenderer.tsx @@ -75,17 +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.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 - 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 new file mode 100644 index 00000000000..02f06a08237 --- /dev/null +++ b/packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx @@ -0,0 +1,74 @@ +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('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(); + 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/components/uiComponents.tsx b/packages/react/src/components/uiComponents.tsx index da0ab7f8fbb..c4ca99c04f4 100644 --- a/packages/react/src/components/uiComponents.tsx +++ b/packages/react/src/components/uiComponents.tsx @@ -127,8 +127,8 @@ type OrganizationSwitcherPropsWithoutCustomPages = Without< const CustomPortalsRenderer = (props: CustomPortalsRendererProps) => { return ( <> - {props?.customPagesPortals?.map((portal, index) => createElement(portal, { key: index }))} - {props?.customMenuItemsPortals?.map((portal, index) => createElement(portal, { key: index }))} + {props?.customPagesPortals?.map(({ key, portal }) => createElement(portal, { key }))} + {props?.customMenuItemsPortals?.map(({ key, portal }) => createElement(portal, { key }))} ); }; diff --git a/packages/react/src/utils/__tests__/customPages.remount.integration.test.tsx b/packages/react/src/utils/__tests__/customPages.remount.integration.test.tsx new file mode 100644 index 00000000000..848a8b4d75d --- /dev/null +++ b/packages/react/src/utils/__tests__/customPages.remount.integration.test.tsx @@ -0,0 +1,120 @@ +import { render, screen } from '@testing-library/react'; +import React, { createElement, useEffect, useRef } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { OrganizationProfilePage } from '../../components/uiComponents'; +import { useOrganizationProfileCustomPages } from '../useCustomPages'; + +vi.mock('@clerk/shared/utils', () => ({ + logErrorInDevMode: vi.fn(), +})); + +// Per-page mount/unmount counters. A remount re-runs the mount effect. +const mounts: Record = {}; +const unmounts: Record = {}; + +// Stable component type, defined once. If it remounts across a rerender it is +// because the portal wrapping it changed identity or render key. +const TrackedContent = ({ id, text }: { id: string; text: string }) => { + useEffect(() => { + mounts[id] = (mounts[id] ?? 0) + 1; + return () => { + unmounts[id] = (unmounts[id] ?? 0) + 1; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- mount-once instrument; id is stable per instance + }, []); + return
{text}
; +}; + +/** + * Faithfully reproduces the production render path for custom pages: + * - useOrganizationProfileCustomPages parses children into { customPages, customPagesPortals } + * - clerk-js calls customPages[i].mount(node) once per logical page (by identity; here keyed by url) + * - CustomPortalsRenderer renders each portal via createElement(portal, { key }) using the STABLE key + */ +const Harness = ({ children, tick }: { children: React.ReactNode; tick: number }) => { + const { customPages, customPagesPortals } = useOrganizationProfileCustomPages(children); + const hostRef = useRef(null); + const mountedUrls = useRef>(new Set()); + + useEffect(() => { + customPages.forEach(page => { + if (page.mount && page.url && !mountedUrls.current.has(page.url)) { + mountedUrls.current.add(page.url); + const node = document.createElement('div'); + hostRef.current?.appendChild(node); + page.mount(node); + } + }); + }); + + return ( + <> +
+ {customPagesPortals.map(({ key, portal }) => createElement(portal, { key }))} + + ); +}; + +const makePage = (id: string, label: string, url: string, text: string) => ( + i} + url={url} + > + + +); + +afterEach(() => { + for (const k of Object.keys(mounts)) { + delete mounts[k]; + } + for (const k of Object.keys(unmounts)) { + delete unmounts[k]; + } +}); + +describe('custom pages remount behavior (integration through CustomPortalsRenderer path)', () => { + it('does not remount custom page content when the parent rerenders', async () => { + const { rerender } = render({[makePage('p1', 'Page 1', 'page-1', 'first')]}); + + await screen.findByText('first'); + expect(mounts['p1']).toBe(1); + + // Parent rerenders for an unrelated reason; the page content prop changes but the + // logical page (key/label/url) is identical. + rerender({[makePage('p1', 'Page 1', 'page-1', 'second')]}); + + await screen.findByText('second'); + expect(mounts['p1']).toBe(1); + expect(unmounts['p1'] ?? 0).toBe(0); + }); + + it('does not remount a surviving custom page when another page is inserted before it', async () => { + const second = makePage('second', 'Second', 'second', 'second-content'); + const first = makePage('first', 'First', 'first', 'first-content'); + + const { rerender } = render({[second]}); + await screen.findByText('second-content'); + expect(mounts['second']).toBe(1); + + // Insert a new page BEFORE the existing one. + rerender({[first, second]}); + await screen.findByText('first-content'); + + // The surviving page keeps its stable key + portal identity, so React reconciles it as an + // update rather than a remount. + expect(mounts['second']).toBe(1); + expect(unmounts['second'] ?? 0).toBe(0); + // The newly inserted page mounts exactly once. + expect(mounts['first']).toBe(1); + }); +}); 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..4e0bf2504c7 --- /dev/null +++ b/packages/react/src/utils/__tests__/useCustomElementPortal.test.tsx @@ -0,0 +1,115 @@ +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'); + }); + + 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/__tests__/useCustomMenuItems.test.tsx b/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx index b748226b250..b947905d8c3 100644 --- a/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx +++ b/packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx @@ -135,4 +135,67 @@ describe('useUserButtonCustomMenuItems', () => { expect(mockOnClick).toHaveBeenCalledTimes(1); } }); + + 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].portal).not.toBe(result.current.customMenuItemsPortals[1].portal); + expect(result.current.customMenuItemsPortals[0].key).not.toBe(result.current.customMenuItemsPortals[1].key); + }); + + 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].portal; + const secondItemIconKey = result.current.customMenuItemsPortals[0].key; + + rerender({ includeFirstItem: true }); + + expect(result.current.customMenuItems).toHaveLength(2); + expect(result.current.customMenuItemsPortals[0].portal).not.toBe(secondItemIconPortal); + expect(result.current.customMenuItemsPortals[1].portal).toBe(secondItemIconPortal); + expect(result.current.customMenuItemsPortals[1].key).toBe(secondItemIconKey); + }); }); 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..b07e6081fde --- /dev/null +++ b/packages/react/src/utils/__tests__/useCustomPages.test.tsx @@ -0,0 +1,94 @@ +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('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); + // Duplicate (same label+url, no key) pages get distinct portal identities... + expect(result.current.customPagesPortals[0].portal).not.toBe(result.current.customPagesPortals[2].portal); + expect(result.current.customPagesPortals[1].portal).not.toBe(result.current.customPagesPortals[3].portal); + // ...and distinct stable render keys. + const keys = result.current.customPagesPortals.map(p => p.key); + expect(new Set(keys).size).toBe(keys.length); + }); + + 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].portal; + const secondPageContentKey = result.current.customPagesPortals[0].key; + const secondPageIconPortal = result.current.customPagesPortals[1].portal; + const secondPageIconKey = result.current.customPagesPortals[1].key; + + rerender({ includeFirstPage: true }); + + expect(result.current.customPages).toHaveLength(2); + expect(result.current.customPagesPortals[0].portal).not.toBe(secondPageContentPortal); + expect(result.current.customPagesPortals[1].portal).not.toBe(secondPageIconPortal); + // The second page keeps BOTH its portal identity and its stable render key when it moves + // position, so CustomPortalsRenderer reconciles it as an update instead of a remount. + expect(result.current.customPagesPortals[2].portal).toBe(secondPageContentPortal); + expect(result.current.customPagesPortals[2].key).toBe(secondPageContentKey); + expect(result.current.customPagesPortals[3].portal).toBe(secondPageIconPortal); + expect(result.current.customPagesPortals[3].key).toBe(secondPageIconKey); + }); +}); diff --git a/packages/react/src/utils/useCustomElementPortal.tsx b/packages/react/src/utils/useCustomElementPortal.tsx index c0bf7e39b23..0ac9d20b9a3 100644 --- a/packages/react/src/utils/useCustomElementPortal.tsx +++ b/packages/react/src/utils/useCustomElementPortal.tsx @@ -1,36 +1,66 @@ import type React from 'react'; -import { useState } from 'react'; +import { useRef, useState } from 'react'; import { createPortal } from 'react-dom'; export type UseCustomElementPortalParams = { component: React.ReactNode; - id: number; + id: string | number; }; export type UseCustomElementPortalReturn = { - portal: () => React.JSX.Element; + portal: React.ComponentType; mount: (node: Element) => void; unmount: () => void; - id: number; + 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()); - - 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; - }, - })); + const [nodeMap, setNodeMap] = useState>(new Map()); + const nodeMapRef = useRef(nodeMap); + const elementsRef = useRef>(new Map()); + const portalsRef = useRef>(new Map()); + + nodeMapRef.current = nodeMap; + elementsRef.current = new Map(elements.map(el => [el.id, el.component])); + + const elementIds = new Set(elements.map(el => el.id)); + portalsRef.current.forEach((_, id) => { + if (!elementIds.has(id)) { + portalsRef.current.delete(id); + } + }); + + return elements.map(el => { + const id = 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; + }); }; diff --git a/packages/react/src/utils/useCustomMenuItems.tsx b/packages/react/src/utils/useCustomMenuItems.tsx index 59385ea423d..250b07d8918 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, @@ -57,7 +57,8 @@ const useCustomMenuItems = ({ }: UseCustomMenuItemsParams) => { const validChildren: CustomMenuItemType[] = []; const customMenuItems: CustomMenuItem[] = []; - const customMenuItemsPortals: React.ComponentType[] = []; + const customMenuItemsPortals: Array<{ key: string; portal: React.ComponentType }> = []; + const portalIdCounts = new Map(); React.Children.forEach(children, child => { if ( @@ -89,6 +90,7 @@ const useCustomMenuItems = ({ } const { props } = child as ReactElement; + const childKey = (child as ReactElement).key; const { label, labelIcon, href, onClick, open } = props; @@ -106,11 +108,13 @@ const useCustomMenuItems = ({ validChildren.push({ ...baseItem, onClick, + portalId: getCustomMenuItemPortalId('action', props, childKey, portalIdCounts), }); } else if (open !== undefined) { validChildren.push({ ...baseItem, open: open.startsWith('/') ? open : `/${open}`, + portalId: getCustomMenuItemPortalId('action', props, childKey, portalIdCounts), }); } else { // Handle the case where neither onClick nor open is defined @@ -125,7 +129,12 @@ const useCustomMenuItems = ({ if (isThatComponent(child, MenuLinkComponent)) { if (isExternalLink(props)) { - validChildren.push({ label, labelIcon, href }); + validChildren.push({ + label, + labelIcon, + href, + portalId: getCustomMenuItemPortalId('link', props, childKey, portalIdCounts), + }); } else { logErrorInDevMode(userButtonMenuItemLinkWrongProps); return; @@ -138,10 +147,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 +168,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, @@ -172,27 +181,44 @@ const useCustomMenuItems = ({ menuItem.open = mi.open; } customMenuItems.push(menuItem); - customMenuItemsPortals.push(iconPortal); + customMenuItemsPortals.push({ key: `icon:${mi.portalId || index}`, portal: iconPortal }); } if (isExternalLink(mi)) { const { 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, mountIcon, unmountIcon, }); - customMenuItemsPortals.push(iconPortal); + customMenuItemsPortals.push({ key: `icon:${mi.portalId || index}`, portal: iconPortal }); } }); return { customMenuItems, customMenuItemsPortals }; }; +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 || ''; + 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 => { 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..4ddea7936fc 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. @@ -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 ( @@ -121,13 +122,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, portalIdCounts), + }); } else { logErrorInDevMode(customPageWrongProps(componentName)); return; @@ -137,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 }); + validChildren.push({ + label, + labelIcon, + url, + portalId: getCustomPagePortalId('link', props, childKey, portalIdCounts), + }); } else { logErrorInDevMode(customLinkWrongProps(componentName)); return; @@ -151,12 +165,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 }); } }); @@ -165,7 +179,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp const customLinkLabelIconsPortals = useCustomElementPortal(customLinkLabelIcons); const customPages: CustomPage[] = []; - const customPagesPortals: React.ComponentType[] = []; + const customPagesPortals: Array<{ key: string; portal: React.ComponentType }> = []; validChildren.forEach((cp, index) => { if (isReorderItem(cp, reorderItemsLabels)) { @@ -177,15 +191,15 @@ 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); + customPagesPortals.push({ key: `content:${cp.portalId || index}`, portal: contentPortal }); + customPagesPortals.push({ key: `label:${cp.portalId || index}`, portal: labelPortal }); return; } if (isExternalLink(cp)) { @@ -193,9 +207,9 @@ 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); + customPagesPortals.push({ key: `label:${cp.portalId || index}`, portal: labelPortal }); return; } }); @@ -203,6 +217,22 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp return { customPages, customPagesPortals }; }; +const getCustomPagePortalId = ( + type: 'page' | 'link', + props: Pick, + key: React.Key | null, + portalIdCounts: Map, +) => { + 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 => { const { children, label, url, labelIcon } = childProps; return !children && !url && !labelIcon && validItems.some(v => v === label);