Skip to content

Commit 6bde33b

Browse files
committed
Ensure custom portal fallback IDs are unique
1 parent c0f8780 commit 6bde33b

4 files changed

Lines changed: 84 additions & 7 deletions

File tree

packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,29 @@ describe('useUserButtonCustomMenuItems', () => {
136136
}
137137
});
138138

139+
it('uses separate portals for duplicate non-keyed menu items', () => {
140+
const children = (
141+
<MenuItems>
142+
<MenuAction
143+
label='Duplicate'
144+
labelIcon={<div>First icon</div>}
145+
open='/duplicate'
146+
/>
147+
<MenuAction
148+
label='Duplicate'
149+
labelIcon={<div>Second icon</div>}
150+
open='/duplicate'
151+
/>
152+
</MenuItems>
153+
);
154+
155+
const { result } = renderHook(() => useUserButtonCustomMenuItems(children));
156+
157+
expect(result.current.customMenuItems).toHaveLength(2);
158+
expect(result.current.customMenuItemsPortals).toHaveLength(2);
159+
expect(result.current.customMenuItemsPortals[0]).not.toBe(result.current.customMenuItemsPortals[1]);
160+
});
161+
139162
it('keeps portal identity with the logical menu item when inserting before it', () => {
140163
const firstItem = (
141164
<MenuAction

packages/react/src/utils/__tests__/useCustomPages.test.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,36 @@ vi.mock('@clerk/shared', () => ({
1010
}));
1111

1212
describe('useOrganizationProfileCustomPages', () => {
13+
it('uses separate portals for duplicate non-keyed custom pages', () => {
14+
const children = [
15+
React.createElement(
16+
OrganizationProfilePage,
17+
{
18+
label: 'Duplicate',
19+
labelIcon: <div>First icon</div>,
20+
url: 'duplicate',
21+
},
22+
<div>First content</div>,
23+
),
24+
React.createElement(
25+
OrganizationProfilePage,
26+
{
27+
label: 'Duplicate',
28+
labelIcon: <div>Second icon</div>,
29+
url: 'duplicate',
30+
},
31+
<div>Second content</div>,
32+
),
33+
];
34+
35+
const { result } = renderHook(() => useOrganizationProfileCustomPages(children));
36+
37+
expect(result.current.customPages).toHaveLength(2);
38+
expect(result.current.customPagesPortals).toHaveLength(4);
39+
expect(result.current.customPagesPortals[0]).not.toBe(result.current.customPagesPortals[2]);
40+
expect(result.current.customPagesPortals[1]).not.toBe(result.current.customPagesPortals[3]);
41+
});
42+
1343
it('keeps portal identity with the logical custom page when inserting before it', () => {
1444
const firstPage = (
1545
<OrganizationProfilePage

packages/react/src/utils/useCustomMenuItems.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const useCustomMenuItems = ({
5858
const validChildren: CustomMenuItemType[] = [];
5959
const customMenuItems: CustomMenuItem[] = [];
6060
const customMenuItemsPortals: React.ComponentType[] = [];
61+
const portalIdCounts = new Map<string, number>();
6162

6263
React.Children.forEach(children, child => {
6364
if (
@@ -107,13 +108,13 @@ const useCustomMenuItems = ({
107108
validChildren.push({
108109
...baseItem,
109110
onClick,
110-
portalId: getCustomMenuItemPortalId('action', props, childKey),
111+
portalId: getCustomMenuItemPortalId('action', props, childKey, portalIdCounts),
111112
});
112113
} else if (open !== undefined) {
113114
validChildren.push({
114115
...baseItem,
115116
open: open.startsWith('/') ? open : `/${open}`,
116-
portalId: getCustomMenuItemPortalId('action', props, childKey),
117+
portalId: getCustomMenuItemPortalId('action', props, childKey, portalIdCounts),
117118
});
118119
} else {
119120
// Handle the case where neither onClick nor open is defined
@@ -128,7 +129,12 @@ const useCustomMenuItems = ({
128129

129130
if (isThatComponent(child, MenuLinkComponent)) {
130131
if (isExternalLink(props)) {
131-
validChildren.push({ label, labelIcon, href, portalId: getCustomMenuItemPortalId('link', props, childKey) });
132+
validChildren.push({
133+
label,
134+
labelIcon,
135+
href,
136+
portalId: getCustomMenuItemPortalId('link', props, childKey, portalIdCounts),
137+
});
132138
} else {
133139
logErrorInDevMode(userButtonMenuItemLinkWrongProps);
134140
return;
@@ -200,13 +206,17 @@ const getCustomMenuItemPortalId = (
200206
type: 'action' | 'link',
201207
props: Pick<CustomMenuItemType, 'label'> & { href?: string; open?: string },
202208
key: React.Key | null,
209+
portalIdCounts: Map<string, number>,
203210
) => {
204211
if (key != null) {
205212
return `${type}:key:${key}`;
206213
}
207214

208215
const target = props.href || props.open || '';
209-
return `${type}:${props.label}:${target}`;
216+
const baseId = `${type}:${props.label}:${target}`;
217+
const occurrence = portalIdCounts.get(baseId) ?? 0;
218+
portalIdCounts.set(baseId, occurrence + 1);
219+
return `${baseId}:${occurrence}`;
210220
};
211221

212222
const isReorderItem = (childProps: any, validItems: string[]): boolean => {

packages/react/src/utils/useCustomPages.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
104104
const { children, LinkComponent, PageComponent, MenuItemsComponent, reorderItemsLabels, componentName } = params;
105105
const { allowForAnyChildren = false } = options || {};
106106
const validChildren: CustomPageWithIdType[] = [];
107+
const portalIdCounts = new Map<string, number>();
107108

108109
React.Children.forEach(children, child => {
109110
if (
@@ -134,7 +135,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
134135
labelIcon,
135136
children,
136137
url,
137-
portalId: getCustomPagePortalId('page', props, childKey),
138+
portalId: getCustomPagePortalId('page', props, childKey, portalIdCounts),
138139
});
139140
} else {
140141
logErrorInDevMode(customPageWrongProps(componentName));
@@ -145,7 +146,12 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
145146
if (isThatComponent(child, LinkComponent)) {
146147
if (isExternalLink(props)) {
147148
// This is an external link
148-
validChildren.push({ label, labelIcon, url, portalId: getCustomPagePortalId('link', props, childKey) });
149+
validChildren.push({
150+
label,
151+
labelIcon,
152+
url,
153+
portalId: getCustomPagePortalId('link', props, childKey, portalIdCounts),
154+
});
149155
} else {
150156
logErrorInDevMode(customLinkWrongProps(componentName));
151157
return;
@@ -215,8 +221,16 @@ const getCustomPagePortalId = (
215221
type: 'page' | 'link',
216222
props: Pick<CustomPageWithIdType, 'label' | 'url'>,
217223
key: React.Key | null,
224+
portalIdCounts: Map<string, number>,
218225
) => {
219-
return key == null ? `${type}:${props.label}:${props.url}` : `${type}:key:${key}`;
226+
if (key != null) {
227+
return `${type}:key:${key}`;
228+
}
229+
230+
const baseId = `${type}:${props.label}:${props.url}`;
231+
const occurrence = portalIdCounts.get(baseId) ?? 0;
232+
portalIdCounts.set(baseId, occurrence + 1);
233+
return `${baseId}:${occurrence}`;
220234
};
221235

222236
const isReorderItem = (childProps: any, validItems: string[]): boolean => {

0 commit comments

Comments
 (0)