Address custom portal review edge cases

Author: jescalan

packages/react/src/components/ClerkHostRenderer.tsx

@@ -75,18 +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.props.customPages?.length !== this.props.props.customPages?.length;
+    const customPagesChanged = _prevProps.props?.customPages?.length !== this.props.props?.customPages?.length;
     const customMenuItemsChanged =
-      _prevProps.props.customMenuItems?.length !== this.props.props.customMenuItems?.length;
+      _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) ||

packages/react/src/components/__tests__/ClerkHostRenderer.test.tsx

@@ -14,6 +14,34 @@ vi.mock('@clerk/shared/react', () => ({
 }));
 
 describe('<ClerkHostRenderer />', () => {
+  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(
+      <ClerkHostRenderer
+        component='UserProfile'
+        mount={mount}
+        unmount={unmount}
+        updateProps={updateProps}
+      />,
+    );
+
+    expect(() =>
+      rerender(
+        <ClerkHostRenderer
+          component='OrganizationProfile'
+          mount={mount}
+          unmount={unmount}
+          updateProps={updateProps}
+        />,
+      ),
+    ).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();

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

@@ -74,4 +74,42 @@ describe('useCustomElementPortal', () => {
     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: <div>number id</div>, id: 1 },
+        { component: <div>string id</div>, id: '1' },
+      ]);
+
+      useEffect(() => {
+        mountNumber(portalRoot);
+        mountString(portalRoot);
+
+        return () => {
+          unmountNumber();
+          unmountString();
+        };
+      }, [mountNumber, mountString, unmountNumber, unmountString]);
+
+      return (
+        <>
+          <NumberPortal />
+          <StringPortal />
+        </>
+      );
+    };
+
+    portalRoot = document.createElement('div');
+    document.body.appendChild(portalRoot);
+
+    render(<TestComponent />);
+
+    await screen.findByText('number id');
+    await screen.findByText('string id');
+    expect(portalRoot.textContent).toBe('number idstring id');
+  });
 });

packages/react/src/utils/useCustomElementPortal.tsx

@@ -14,26 +14,28 @@ export type UseCustomElementPortalReturn = {
   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<Map<string, Element | null>>(new Map());
+  const [nodeMap, setNodeMap] = useState<Map<PortalKey, 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());
+  const elementsRef = useRef<Map<PortalKey, React.ReactNode>>(new Map());
+  const portalsRef = useRef<Map<PortalKey, UseCustomElementPortalReturn>>(new Map());
 
   nodeMapRef.current = nodeMap;
-  elementsRef.current = new Map(elements.map(el => [String(el.id), el.component]));
+  elementsRef.current = new Map(elements.map(el => [el.id, el.component]));
 
-  const elementIds = new Set(elements.map(el => String(el.id)));
+  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 = String(el.id);
+    const id = el.id;
     const existingPortal = portalsRef.current.get(id);
 
     if (existingPortal) {