fix(react): prevent custom page portal remounts

Author: jescalan

.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.

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

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<string, unknown>, ...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('<ClerkHostRenderer />', () => {
+  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(
+      <ClerkHostRenderer
+        mount={mount}
+        props={{ customPages: [{ label: 'General' }] }}
+        unmount={unmount}
+        updateProps={updateProps}
+      />,
+    );
+
+    rerender(
+      <ClerkHostRenderer
+        mount={mount}
+        props={{ customPages: [{ label: 'General' }, { label: 'Permissions' }] }}
+        unmount={unmount}
+        updateProps={updateProps}
+      />,
+    );
+
+    expect(updateProps).toHaveBeenCalledTimes(1);
+    expect(updateProps).toHaveBeenCalledWith({
+      node: expect.any(HTMLDivElement),
+      props: { customPages: [{ label: 'General' }, { label: 'Permissions' }] },
+    });
+  });
+});

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 <div>{label}</div>;
+    };
+
+    const TestComponent = ({ label }: { label: string }) => {
+      const [{ mount, portal: Portal, unmount }] = useCustomElementPortal([
+        { component: <CustomContent label={label} />, id: 0 },
+      ]);
+
+      useEffect(() => {
+        mount(portalRoot);
+        return unmount;
+      }, [mount, unmount]);
+
+      return <Portal />;
+    };
+
+    portalRoot = document.createElement('div');
+    document.body.appendChild(portalRoot);
+
+    const { rerender } = render(<TestComponent label='first render' />);
+
+    await screen.findByText('first render');
+    expect(mountTracker).toHaveBeenCalledTimes(1);
+
+    rerender(<TestComponent label='second render' />);
+
+    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 <Portal />;
+    };
+
+    portalRoot = document.createElement('div');
+    document.body.appendChild(portalRoot);
+
+    render(<TestComponent />);
+
+    await screen.findByText('0');
+    expect(portalRoot.textContent).toBe('0');
+  });
+});

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<Map<string, Element | null>>(new Map());
+  const nodeMapRef = useRef(nodeMap);
+  const elementsRef = useRef<Map<string, React.ReactNode>>(new Map());
+  const portalsRef = useRef<Map<string, UseCustomElementPortalReturn>>(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;
+  });
 };