Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sharp-owls-live.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-react': patch
---

Fix an infinity re-render issue in `UserProfileModal` when we pass `userProfileProps` in `<UserButton />` and we have `customMenuItems` and `customPages`
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { UserButton } from '@clerk/clerk-react';
import { PageContextProvider } from '../PageContext.tsx';
import React from 'react';

export default function Page() {
const [open, setIsOpen] = React.useState(false);
const [theme, setTheme] = React.useState('light');
const [notifications, setNotifications] = React.useState(false);
const [language, setLanguage] = React.useState('en');

return (
<PageContextProvider>
<UserButton
fallback={<>Loading user button</>}
userProfileProps={{ appearance: { elements: { modalBackdrop: { zIndex: '100' } } } }}
>
<UserButton.MenuItems>
<UserButton.Action
label={`Chat is ${open ? 'ON' : 'OFF'}`}
labelIcon={<span>🌐</span>}
onClick={() => setIsOpen(!open)}
/>
<UserButton.Action
label={`Theme: ${theme === 'light' ? '☀️ Light' : '🌙 Dark'}`}
labelIcon={<span>🌐</span>}
onClick={() => setTheme(t => (t === 'light' ? 'dark' : 'light'))}
/>
<UserButton.Action
label={`Notifications ${notifications ? '🔔 ON' : '🔕 OFF'}`}
labelIcon={<span>🌐</span>}
onClick={() => setNotifications(n => !n)}
/>
<UserButton.Action
label={`Language: ${language.toUpperCase()}`}
labelIcon={<span>🌍</span>}
onClick={() => setLanguage(l => (l === 'en' ? 'es' : 'en'))}
/>
<UserButton.Action label={'manageAccount'} />
<UserButton.Action label={'signOut'} />
<UserButton.Link
href={'http://clerk.com'}
label={'Visit Clerk'}
labelIcon={<span>🌐</span>}
/>

<UserButton.Link
href={'/user'}
label={'Visit User page'}
labelIcon={<span>🌐</span>}
/>

<UserButton.Action
label={'Custom Alert'}
labelIcon={<span>🔔</span>}
onClick={() => alert('custom-alert')}
/>
</UserButton.MenuItems>
<UserButton.UserProfilePage
label='Notifications Page'
url='notifications'
labelIcon={<span>🔔</span>}
>
<h1 data-page='notifications-page'>Notifications page</h1>
</UserButton.UserProfilePage>
</UserButton>
</PageContextProvider>
);
}
Original file line number Diff line number Diff line change
@@ -1,25 +1,7 @@
import { UserButton } from '@clerk/clerk-react';
import { useContext } from 'react';
import { PageContext, PageContextProvider } from '../PageContext.tsx';
import { PageContextProvider } from '../PageContext.tsx';
import React from 'react';

function Page1() {
const { counter, setCounter } = useContext(PageContext);

return (
<>
<h1 data-page={1}>Page 1</h1>
<p data-page={1}>Counter: {counter}</p>
<button
data-page={1}
onClick={() => setCounter(a => a + 1)}
>
Update
</button>
</>
);
}

export default function Page() {
const [open, setIsOpen] = React.useState(false);
const [theme, setTheme] = React.useState('light');
Expand Down
5 changes: 5 additions & 0 deletions integration/templates/react-vite/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import UserProfile from './user';
import UserProfileCustom from './custom-user-profile';
import UserButtonCustom from './custom-user-button';
import UserButtonCustomDynamicLabels from './custom-user-button/with-dynamic-labels.tsx';
import UserButtonCustomDynamicLabelsAndCustomPages from './custom-user-button/with-dynamic-label-and-custom-pages.tsx';
import UserButtonCustomTrigger from './custom-user-button-trigger';
import UserButton from './user-button';
import Waitlist from './waitlist';
Expand Down Expand Up @@ -85,6 +86,10 @@ const router = createBrowserRouter([
path: '/custom-user-button-dynamic-labels',
element: <UserButtonCustomDynamicLabels />,
},
{
path: '/custom-user-button-dynamic-labels-and-custom-pages',
element: <UserButtonCustomDynamicLabelsAndCustomPages />,
},
{
path: '/custom-user-button-trigger',
element: <UserButtonCustomTrigger />,
Expand Down
68 changes: 68 additions & 0 deletions integration/tests/custom-pages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const CUSTOM_PROFILE_PAGE = '/custom-user-profile';
const CUSTOM_BUTTON_PAGE = '/custom-user-button';
const CUSTOM_BUTTON_TRIGGER_PAGE = '/custom-user-button-trigger';
const CUSTOM_BUTTON_DYNAMIC_LABELS_PAGE = '/custom-user-button-dynamic-labels';
const CUSTOM_BUTTON_DYNAMIC_LABELS_AND_CUSTOM_PAGES_PAGE = '/custom-user-button-dynamic-labels-and-custom-pages';

async function waitForMountedComponent(
component: 'UserButton' | 'UserProfile',
Expand Down Expand Up @@ -375,5 +376,72 @@ testAgainstRunningApps({ withPattern: ['react.vite.withEmailCodes'] })(
await expect(languageButton).toHaveText('🌍Language: EN');
});
});

test.describe('User Button with dynamic labels and custom page', () => {
test('click Chat is OFF and ensure that state has been changed', 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_BUTTON_DYNAMIC_LABELS_AND_CUSTOM_PAGES_PAGE);
await u.po.userButton.waitForMounted();
await u.po.userButton.toggleTrigger();
await u.po.userButton.waitForPopover();

const pagesContainer = u.page.locator('div.cl-userButtonPopoverActions__multiSession').first();
const buttons = await pagesContainer.locator('button').all();

expect(buttons.length).toBe(9);

const expectedTexts = [
'🌐Chat is OFF',
'🌐Theme: ☀️ Light',
'🌐Notifications 🔕 OFF',
'🌍Language: EN',
'Manage account',
'Sign out',
'🌐Visit Clerk',
'🌐Visit User page',
'🔔Custom Alert',
];

for (let i = 0; i < buttons.length; i++) {
await expect(buttons[i]).toHaveText(expectedTexts[i]);
}

const chatButton = buttons[0];
const notificationsButton = buttons[2];
const languageButton = buttons[3];
const manageAccountButton = buttons[4];

// Test chat toggle
await chatButton.click();
await u.po.userButton.toggleTrigger();
await u.po.userButton.waitForPopover();
await expect(chatButton).toHaveText('🌐Chat is ON');
await expect(languageButton).toHaveText('🌍Language: EN');

await notificationsButton.click();
await u.po.userButton.toggleTrigger();
await u.po.userButton.waitForPopover();
await expect(notificationsButton).toHaveText('🌐Notifications 🔔 ON');
await expect(chatButton).toHaveText('🌐Chat is ON');
await expect(languageButton).toHaveText('🌍Language: EN');

await manageAccountButton.click();
await u.po.userProfile.waitForMounted();

const userProfilePageButtons = await u.page.locator('button.cl-navbarButton__custom-page-0').all();
const [notificationsPage] = userProfilePageButtons;
await expect(notificationsPage.locator('div.cl-navbarButtonIcon__custom-page-0')).toHaveText('🔔');

await notificationsPage.click();

const orderSent = page.locator('h1[data-page="notifications-page"]');
await orderSent.waitFor({ state: 'attached' });
});
});
},
);
27 changes: 24 additions & 3 deletions packages/react/src/components/ClerkHostRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ const isMountProps = (props: any): props is MountProps => {
const isOpenProps = (props: any): props is OpenProps => {
return 'open' in props;
};

const stripMenuItemIconHandlers = (
menuItems?: Array<{
mountIcon?: (el: HTMLDivElement) => void;
unmountIcon?: (el: HTMLDivElement) => void;
[key: string]: any;
}>,
) => {
return menuItems?.map(({ mountIcon, unmountIcon, ...rest }) => rest);
};

// README: <ClerkHostRenderer/> should be a class pure component in order for mount and unmount
// lifecycle props to be invoked correctly. Replacing the class component with a
// functional component wrapped with a React.memo is not identical to the original
Expand Down Expand Up @@ -64,14 +75,24 @@ 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', 'children');
const newProps = without(this.props.props, 'customPages', '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;

if (!isDeeplyEqual(prevProps, newProps) || customPagesChanged || customMenuItemsChanged) {
// 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);

if (
!isDeeplyEqual(prevProps, newProps) ||
!isDeeplyEqual(prevMenuItemsWithoutHandlers, newMenuItemsWithoutHandlers) ||
customPagesChanged ||
customMenuItemsChanged
) {
if (this.rootRef.current) {
this.props.updateProps({ node: this.rootRef.current, props: this.props.props });
}
Expand Down