remove PortalRootManager usage

Author: alexcarpenter

packages/shared/src/react/PortalProvider.tsx

@@ -1,9 +1,8 @@
 'use client';
 
-import React, { useEffect, useRef } from 'react';
+import React from 'react';
 
 import { createContextAndHook } from './hooks/createContextAndHook';
-import { portalRootManager } from './portal-root-manager';
 
 type PortalProviderProps = React.PropsWithChildren<{
   /**
@@ -19,9 +18,12 @@ const [PortalContext, , usePortalContextWithoutGuarantee] = createContextAndHook
 }>('PortalProvider');
 
 /**
- * PortalProvider allows you to specify a custom container for all Clerk floating UI elements
+ * UNSAFE_PortalProvider allows you to specify a custom container for Clerk floating UI elements
  * (popovers, modals, tooltips, etc.) that use portals.
  *
+ * Only components within this provider will be affected. Components outside the provider
+ * will continue to use the default document.body for portals.
+ *
  * This is particularly useful when using Clerk components inside external UI libraries
  * like Radix Dialog or React Aria Components, where portaled elements need to render
  * within the dialog's container to remain interactable.
@@ -32,46 +34,32 @@ const [PortalContext, , usePortalContextWithoutGuarantee] = createContextAndHook
  *   const containerRef = useRef(null);
  *   return (
  *     <RadixDialog ref={containerRef}>
- *       <PortalProvider getContainer={() => containerRef.current}>
+ *       <UNSAFE_PortalProvider getContainer={() => containerRef.current}>
  *         <UserButton />
- *       </PortalProvider>
+ *       </UNSAFE_PortalProvider>
  *     </RadixDialog>
  *   );
  * }
  * ```
  */
 export const UNSAFE_PortalProvider = ({ children, getContainer }: PortalProviderProps) => {
-  const getContainerRef = useRef(getContainer);
-  getContainerRef.current = getContainer;
-
-  // Register with the manager for cross-tree access (e.g., modals in Components.tsx)
-  useEffect(() => {
-    const getContainerWrapper = () => getContainerRef.current();
-    portalRootManager.push(getContainerWrapper);
-    return () => {
-      portalRootManager.pop();
-    };
-  }, []);
-
-  // Provide context for same-tree access (e.g., UserButton popover)
   const contextValue = React.useMemo(() => ({ value: { getContainer } }), [getContainer]);
 
   return <PortalContext.Provider value={contextValue}>{children}</PortalContext.Provider>;
 };
 
 /**
  * Hook to get the current portal root container.
- * First checks React context (for same-tree components),
- * then falls back to PortalRootManager (for cross-tree like modals).
+ * Returns the getContainer function from context if inside a PortalProvider,
+ * otherwise returns a function that returns null (default behavior).
  */
 export const usePortalRoot = (): (() => HTMLElement | null) => {
-  // Try to get from context first (for components in the same React tree)
   const contextValue = usePortalContextWithoutGuarantee();
 
   if (contextValue && 'getContainer' in contextValue && contextValue.getContainer) {
     return contextValue.getContainer;
   }
 
-  // Fall back to manager (for components in different React trees, like modals)
-  return portalRootManager.getCurrent.bind(portalRootManager);
+  // Return a function that returns null when not inside a PortalProvider
+  return () => null;
 };

packages/shared/src/react/__tests__/PortalProvider.test.tsx

@@ -1,11 +1,10 @@
-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { describe, expect, it } from 'vitest';
 import React from 'react';
 import { render, screen } from '@testing-library/react';
 
 import { UNSAFE_PortalProvider, usePortalRoot } from '../PortalProvider';
-import { portalRootManager } from '../portal-root-manager';
 
-describe('PortalProvider', () => {
+describe('UNSAFE_PortalProvider', () => {
   it('provides getContainer to children via context', () => {
     const container = document.createElement('div');
     const getContainer = () => container;
@@ -24,38 +23,31 @@ describe('PortalProvider', () => {
     expect(screen.getByTestId('test').textContent).toBe('found');
   });
 
-  it('registers with portalRootManager on mount', () => {
+  it('only affects components within the provider', () => {
     const container = document.createElement('div');
     const getContainer = () => container;
-    const pushSpy = vi.spyOn(portalRootManager, 'push');
 
-    const { unmount } = render(
-      <UNSAFE_PortalProvider getContainer={getContainer}>
-        <div>test</div>
-      </UNSAFE_PortalProvider>,
-    );
-
-    expect(pushSpy).toHaveBeenCalledTimes(1);
-    expect(portalRootManager.getCurrent()).toBe(container);
-
-    unmount();
-  });
+    const InsideComponent = () => {
+      const portalRoot = usePortalRoot();
+      return <div data-testid='inside'>{portalRoot() === container ? 'container' : 'null'}</div>;
+    };
 
-  it('unregisters from portalRootManager on unmount', () => {
-    const container = document.createElement('div');
-    const getContainer = () => container;
-    const popSpy = vi.spyOn(portalRootManager, 'pop');
+    const OutsideComponent = () => {
+      const portalRoot = usePortalRoot();
+      return <div data-testid='outside'>{portalRoot() === null ? 'null' : 'container'}</div>;
+    };
 
-    const { unmount } = render(
-      <UNSAFE_PortalProvider getContainer={getContainer}>
-        <div>test</div>
-      </UNSAFE_PortalProvider>,
+    render(
+      <>
+        <OutsideComponent />
+        <UNSAFE_PortalProvider getContainer={getContainer}>
+          <InsideComponent />
+        </UNSAFE_PortalProvider>
+      </>,
     );
 
-    unmount();
-
-    expect(popSpy).toHaveBeenCalledTimes(1);
-    expect(portalRootManager.getCurrent()).toBeNull();
+    expect(screen.getByTestId('inside').textContent).toBe('container');
+    expect(screen.getByTestId('outside').textContent).toBe('null');
   });
 });
 
@@ -78,103 +70,34 @@ describe('usePortalRoot', () => {
     expect(screen.getByTestId('test').textContent).toBe('found');
   });
 
-  it('returns manager.getCurrent when outside PortalProvider', () => {
-    const container = document.createElement('div');
-    portalRootManager.push(() => container);
-
+  it('returns a function that returns null when outside PortalProvider', () => {
     const TestComponent = () => {
       const portalRoot = usePortalRoot();
-      return <div data-testid='test'>{portalRoot() === container ? 'found' : 'not-found'}</div>;
+      return <div data-testid='test'>{portalRoot() === null ? 'null' : 'not-null'}</div>;
     };
 
     render(<TestComponent />);
 
-    expect(screen.getByTestId('test').textContent).toBe('found');
-
-    portalRootManager.pop();
+    expect(screen.getByTestId('test').textContent).toBe('null');
   });
 
-  it('context value takes precedence over manager', () => {
-    const contextContainer = document.createElement('div');
-    const managerContainer = document.createElement('div');
-    const contextGetContainer = () => contextContainer;
-
-    portalRootManager.push(() => managerContainer);
+  it('supports nested providers with innermost taking precedence', () => {
+    const outerContainer = document.createElement('div');
+    const innerContainer = document.createElement('div');
 
     const TestComponent = () => {
       const portalRoot = usePortalRoot();
-      return <div data-testid='test'>{portalRoot() === contextContainer ? 'found' : 'not-found'}</div>;
+      return <div data-testid='test'>{portalRoot() === innerContainer ? 'inner' : 'outer'}</div>;
     };
 
     render(
-      <UNSAFE_PortalProvider getContainer={contextGetContainer}>
-        <TestComponent />
+      <UNSAFE_PortalProvider getContainer={() => outerContainer}>
+        <UNSAFE_PortalProvider getContainer={() => innerContainer}>
+          <TestComponent />
+        </UNSAFE_PortalProvider>
       </UNSAFE_PortalProvider>,
     );
 
-    expect(screen.getByTestId('test').textContent).toBe('found');
-
-    portalRootManager.pop();
-  });
-});
-
-describe('portalRootManager', () => {
-  beforeEach(() => {
-    // Clear the stack before each test
-    while (portalRootManager.getCurrent() !== null) {
-      portalRootManager.pop();
-    }
-  });
-
-  it('maintains stack of portal roots', () => {
-    const container1 = document.createElement('div');
-    const container2 = document.createElement('div');
-    const getContainer1 = () => container1;
-    const getContainer2 = () => container2;
-
-    portalRootManager.push(getContainer1);
-    portalRootManager.push(getContainer2);
-
-    expect(portalRootManager.getCurrent()).toBe(container2);
-
-    portalRootManager.pop();
-    expect(portalRootManager.getCurrent()).toBe(container1);
-
-    portalRootManager.pop();
-  });
-
-  it('getCurrent returns topmost root', () => {
-    const container1 = document.createElement('div');
-    const container2 = document.createElement('div');
-    const getContainer1 = () => container1;
-    const getContainer2 = () => container2;
-
-    portalRootManager.push(getContainer1);
-    portalRootManager.push(getContainer2);
-
-    expect(portalRootManager.getCurrent()).toBe(container2);
-
-    portalRootManager.pop();
-    portalRootManager.pop();
-  });
-
-  it('pop removes topmost root', () => {
-    const container1 = document.createElement('div');
-    const container2 = document.createElement('div');
-    const getContainer1 = () => container1;
-    const getContainer2 = () => container2;
-
-    portalRootManager.push(getContainer1);
-    portalRootManager.push(getContainer2);
-
-    portalRootManager.pop();
-
-    expect(portalRootManager.getCurrent()).toBe(container1);
-
-    portalRootManager.pop();
-  });
-
-  it('getCurrent returns null when stack is empty', () => {
-    expect(portalRootManager.getCurrent()).toBeNull();
+    expect(screen.getByTestId('test').textContent).toBe('inner');
   });
 });