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/healthy-panthers-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-react': patch
---

Fix an issue where `<UserButton />` wouldn't update when custom menu item props changed
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { UserButton } from '@clerk/clerk-react';
import { useContext } from 'react';
import { PageContext, 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');
const [notifications, setNotifications] = React.useState(false);
const [language, setLanguage] = React.useState('en');

return (
<PageContextProvider>
<UserButton fallback={<>Loading user button</>}>
<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>
</PageContextProvider>
);
}
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 @@ -10,6 +10,7 @@ import SignUp from './sign-up';
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 UserButtonCustomTrigger from './custom-user-button-trigger';
import UserButton from './user-button';
import Waitlist from './waitlist';
Expand Down Expand Up @@ -75,6 +76,10 @@ const router = createBrowserRouter([
path: '/custom-user-button',
element: <UserButtonCustom />,
},
{
path: '/custom-user-button-dynamic-labels',
element: <UserButtonCustomDynamicLabels />,
},
{
path: '/custom-user-button-trigger',
element: <UserButtonCustomTrigger />,
Expand Down
55 changes: 55 additions & 0 deletions integration/tests/custom-pages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { createTestUtils, testAgainstRunningApps } from '../testUtils';
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';

async function waitForMountedComponent(
component: 'UserButton' | 'UserProfile',
Expand Down Expand Up @@ -320,5 +321,59 @@ testAgainstRunningApps({ withPattern: ['react.vite.withEmailCodes'] })(
await pendingDialog;
});
});

test.describe('User Button with dynamic labels', () => {
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_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];

// 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');
});
});
},
);
7 changes: 4 additions & 3 deletions packages/react/src/components/ClerkHostRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ 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 or customMenuItems relies on props getting new references
const prevProps = without(_prevProps.props, 'customPages', 'customMenuItems', 'children');
const newProps = without(this.props.props, 'customPages', 'customMenuItems', 'children');
// and the implementation of customPages relies on props getting new references
const prevProps = without(_prevProps.props, 'customPages', 'children');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Have we checked if this causes infinite re-renders?

Copy link
Contributor Author

@nikospapcom nikospapcom Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper yes I checked different scenarios and I didn't saw any re-render in <UserButton />, <UserProfile /> or <OrganizationProfile />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed a support ticket about this, but noting this did cause infinite re-renders in my app, so we've pinned down to clerk-react@5.22.10 for now. Not doing anything fancy with customMenuItems:

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @aldenquimby could you open an issue with a minimal reproduction ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimal repro here: https://github.com/aldenquimby/clerk-bug-demo/blob/main/src/app/page.tsx

I have an open support ticket. Want me to create a github issue separately?

const newProps = without(this.props.props, 'customPages', '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;
Expand Down