fix(v0): make Portal safe in CM

packages/fluentui/react-northstar/src/components/Portal/Portal.tsx

@@ -85,7 +85,7 @@ export interface PortalProps extends ChildrenComponentProps, ContentComponentPro
 export const Portal: React.FC<PortalProps> & FluentComponentStaticProps<PortalProps> = props => {
   const context = useFluentContext();
 
-  const { children, content, trapFocus, trigger, triggerAccessibility } = props;
+  const { children, content, trapFocus, trigger, triggerAccessibility = {} } = props;
   const portalRef = React.useRef<HTMLElement>();
   const triggerRef = React.useRef<HTMLElement>();
 
@@ -193,7 +193,3 @@ Portal.handledProps = Object.keys(Portal.propTypes) as any;
 Portal.create = createShorthandFactory({
   Component: Portal,
 });
-
-Portal.defaultProps = {
-  triggerAccessibility: {},
-};

packages/fluentui/react-northstar/src/components/Portal/PortalInner.tsx

@@ -41,7 +41,7 @@ export const PortalInner: React.FC<PortalInnerProps> = props => {
   const { target, rtl } = useFluentContext();
   const registerPortalEl = usePortalCompat();
 
-  const box = usePortalBox({ className, target, rtl });
+  const box = usePortalBox({ className, targetNode: target?.body, rtl });
   // PortalInner should render elements even without a context
   // eslint-disable-next-line
   const container: HTMLElement | null = isBrowser() ? mountNode || box || document.body : null;

packages/fluentui/react-northstar/src/components/Portal/usePortalBox.ts

@@ -1,58 +1,146 @@
-import { useIsomorphicLayoutEffect } from '@fluentui/react-bindings';
 import * as React from 'react';
 
-import { isBrowser } from '../../utils/isBrowser';
+const useInsertionEffect =
+  // eslint-disable-next-line no-useless-concat
+  ((React as never)['useInsertion' + 'Effect'] as typeof React.useLayoutEffect | undefined) ?? React.useLayoutEffect;
 
-type UsePortalBoxOptions = {
-  className: string;
+export type UsePortalBoxOptions = {
+  className?: string;
   rtl: boolean;
-  target: Document | undefined;
+  targetNode: HTMLElement | undefined;
 };
 
-export const usePortalBox = (options: UsePortalBoxOptions): HTMLDivElement | null => {
-  const { className, rtl, target } = options;
+const initializeElementFactory = () => {
+  let currentElement: HTMLDivElement | undefined = undefined;
 
-  const element: HTMLDivElement | null = React.useMemo(() => {
-    const newElement = isBrowser() && target ? target.createElement('div') : null;
+  function get(targetRoot: HTMLElement, forceCreation: boolean): HTMLDivElement | undefined {
+    if (currentElement) {
+      return currentElement;
+    }
 
-    // Element should be attached to DOM during render to make elements that will be rendered
-    // inside accessible in effects of child components
-    if (newElement) {
-      target.body.appendChild(newElement);
+    if (forceCreation) {
+      currentElement = targetRoot.ownerDocument.createElement('div');
+      targetRoot.appendChild(currentElement);
     }
 
-    return newElement;
-  }, [target]);
+    return currentElement;
+  }
+
+  function dispose() {
+    if (currentElement) {
+      currentElement.remove();
+      currentElement = undefined;
+    }
+  }
 
-  useIsomorphicLayoutEffect(() => {
-    const classes = className.split(' ').filter(Boolean);
+  return {
+    get,
+    dispose,
+  };
+};
 
-    if (element) {
-      element.classList.add(...classes);
+/**
+ * This is a modern element factory for React 18 and above. It is safe for concurrent rendering.
+ *
+ * It abuses the fact that React will mount DOM once (unlike hooks), so by using a proxy we can intercept:
+ * - the `remove()` method (we call it in `useEffect()`) and remove the element only when the portal is unmounted
+ * - all other methods (and properties) will be called by React once a portal is mounted
+ */
+export const usePortalBox = (options: UsePortalBoxOptions): HTMLDivElement | null => {
+  const { className, rtl, targetNode } = options;
+  const [elementFactory] = React.useState(initializeElementFactory);
 
-      if (rtl) {
-        element.setAttribute('dir', 'rtl');
-      } else {
-        element.removeAttribute('dir');
-      }
+  const elementProxy = React.useMemo(() => {
+    if (targetNode === undefined) {
+      return null;
     }
 
+    return new Proxy({} as HTMLDivElement, {
+      get(_, property: keyof HTMLDivElement) {
+        // Heads up!
+        // `createPortal()` performs a check for `nodeType` property to determine if the mount node is a valid DOM node
+        // before mounting the portal. We hardcode the value to `Node.ELEMENT_NODE` to pass this check and avoid
+        // premature node creation
+        if (property === 'nodeType') {
+          return Node.ELEMENT_NODE;
+        }
+
+        // Heads up!
+        // We intercept the `remove()` method to remove the mount node only when portal has been unmounted already.
+        if (property === 'remove') {
+          const targetElement = elementFactory.get(targetNode, false);
+
+          if (targetElement) {
+            // If the mountElement has children, the portal is still mounted, otherwise we can dispose of it
+            const portalHasNoChildren = targetElement.childNodes.length === 0;
+
+            if (portalHasNoChildren) {
+              elementFactory.dispose();
+            }
+          }
+
+          return () => {
+            // Always return a no-op function to avoid errors in the code
+          };
+        }
+
+        const targetElement = elementFactory.get(targetNode, true);
+        const targetProperty = targetElement ? targetElement[property] : undefined;
+
+        if (typeof targetProperty === 'function') {
+          return targetProperty.bind(targetElement);
+        }
+
+        return targetProperty;
+      },
+
+      set(_, property: keyof HTMLDivElement | '_virtual' | 'focusVisible', value) {
+        const ignoredProperty = property === '_virtual' || property === 'focusVisible';
+
+        // We should use the `elementFactory.get(targetNode, !ignoredProperty)`,
+        // but TypeScript requires a literal `true` or `false` for the overload signature.
+        // This workaround ensures the correct overload is called and avoids TypeScript errors.
+        const targetElement = ignoredProperty
+          ? elementFactory.get(targetNode, false)
+          : elementFactory.get(targetNode, true);
+
+        if (ignoredProperty && !targetElement) {
+          // We ignore the `_virtual` and `focusVisible` properties to avoid conflicts with the proxy
+          return true;
+        }
+
+        if (targetElement) {
+          Object.assign(targetElement, { [property]: value });
+          return true;
+        }
+
+        return false;
+      },
+    });
+  }, [elementFactory, targetNode]);
+
+  useInsertionEffect!(() => {
+    if (!elementProxy) {
+      return () => {};
+    }
+
+    const classesToApply = className.split(' ').filter(Boolean);
+
+    elementProxy.classList.add(...classesToApply);
+    elementProxy.setAttribute('dir', rtl ? 'rtl' : 'ltr');
+    elementProxy.setAttribute('data-portal-node', 'true');
+
     return () => {
-      if (element) {
-        element.classList.remove(...classes);
-        element.removeAttribute('dir');
-      }
+      elementProxy.classList.remove(...classesToApply);
+      elementProxy.removeAttribute('dir');
     };
-  }, [className, element, rtl]);
+  }, [className, elementProxy, rtl]);
 
-  // This effect should always last as it removes element from HTML tree
   React.useEffect(() => {
     return () => {
-      if (element) {
-        target.body.removeChild(element);
-      }
+      elementProxy?.remove();
     };
-  }, [element, target]);
+  }, [elementProxy]);
 
-  return element;
+  return elementProxy;
 };

packages/fluentui/react-northstar/test/specs/components/Portal/PortalInner-test.tsx

@@ -97,7 +97,7 @@ describe('PortalInner', () => {
       );
 
       expect(register).toHaveBeenCalledTimes(1);
-      expect(register).toHaveBeenCalledWith(expect.any(HTMLElement));
+      expect(register).toHaveBeenCalledWith(expect.any(Object));
 
       wrapper.unmount();
       expect(unregister).toHaveBeenCalledTimes(1);