feat: positioning should happen out of React lifecycle

change/@fluentui-react-portal-323fba1d-79e4-4b55-af4f-cfa9444a2bcf.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "refactor: usePortalMount node updates mount node attributes in memo",
+  "packageName": "@fluentui/react-portal",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "feat: position updates are handled out of react lifecycle",
+  "packageName": "@fluentui/react-positioning",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/react-components/react-portal/src/components/Portal/usePortalMountNode.ts

@@ -1,5 +1,4 @@
 import * as React from 'react';
-import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities';
 import {
   useThemeClassName_unstable as useThemeClassName,
   useFluent_unstable as useFluent,
@@ -44,7 +43,11 @@ export const usePortalMountNode = (options: UsePortalMountNodeOptions): HTMLElem
     return newElement;
   }, [targetDocument, options.disabled]);
 
-  useIsomorphicLayoutEffect(() => {
+  // This useMemo call is intentional
+  // We don't want to re-create the portal element when its attributes change.
+  // This also should not be done in an effect because, changing the value of css variables
+  // after initial mount can trigger interesting CSS side effects like transitions.
+  React.useMemo(() => {
     if (element) {
       const classesToApply = className.split(' ').filter(Boolean);
 

packages/react-components/react-positioning/etc/react-positioning.api.md

@@ -68,13 +68,13 @@ export type Position = 'above' | 'below' | 'before' | 'after';
 // @public (undocumented)
 export type PositioningImperativeRef = {
     updatePosition: () => void;
-    setTarget: (target: HTMLElement | PositioningVirtualElement) => void;
+    setTarget: (target: TargetElement) => void;
 };
 
 // @public (undocumented)
 export interface PositioningProps extends Omit<PositioningOptions, 'positionFixed' | 'unstable_disableTether'> {
     positioningRef?: React_2.Ref<PositioningImperativeRef>;
-    target?: HTMLElement | PositioningVirtualElement | null;
+    target?: TargetElement | null;
 }
 
 // @public (undocumented)
@@ -105,11 +105,7 @@ export function resolvePositioningShorthand(shorthand: PositioningShorthand | un
 export type SetVirtualMouseTarget = (event: React_2.MouseEvent | MouseEvent | undefined | null) => void;
 
 // @internal (undocumented)
-export function usePositioning(options: UsePositioningOptions): {
-    targetRef: React_2.MutableRefObject<any>;
-    containerRef: React_2.MutableRefObject<any>;
-    arrowRef: React_2.MutableRefObject<any>;
-};
+export function usePositioning(options: UsePositioningOptions): UsePositioningReturn;
 
 // @internal
 export const usePositioningMouseTarget: (initialState?: PositioningVirtualElement | (() => PositioningVirtualElement) | undefined) => readonly [PositioningVirtualElement | undefined, SetVirtualMouseTarget];

packages/react-components/react-positioning/src/createPositionManager.ts

@@ -0,0 +1,122 @@
+import { computePosition } from '@floating-ui/dom';
+import type { Middleware, Placement, Strategy } from '@floating-ui/dom';
+import type { PositionManager, TargetElement } from './types';
+import { debounce, writeArrowUpdates, writeContainerUpdates, getScrollParent } from './utils';
+
+interface PositionManagerOptions {
+  /**
+   * The positioned element
+   */
+  container: HTMLElement;
+  /**
+   * Element that the container will be anchored to
+   */
+  target: TargetElement;
+  /**
+   * Arrow that points from the container to the target
+   */
+  arrow: HTMLElement | null;
+  /**
+   * The value of the css `position` property
+   * @default absolute
+   */
+  strategy: Strategy;
+  /**
+   * [Floating UI middleware](https://floating-ui.com/docs/middleware)
+   */
+  middleware: Middleware[];
+  /**
+   * [Floating UI placement](https://floating-ui.com/docs/computePosition#placement)
+   */
+  placement?: Placement;
+}
+
+/**
+ * @internal
+ * @returns manager that handles positioning out of the react lifecycle
+ */
+export function createPositionManager(options: PositionManagerOptions): PositionManager {
+  const { container, target, arrow, strategy, middleware, placement } = options;
+  if (!target || !container) {
+    return {
+      updatePosition: () => undefined,
+      dispose: () => undefined,
+    };
+  }
+
+  let isFirstUpdate = true;
+  const scrollParents: Set<HTMLElement> = new Set<HTMLElement>();
+  const targetWindow = container.ownerDocument.defaultView;
+
+  // When the container is first resolved, set position `fixed` to avoid scroll jumps.
+  // Without this scroll jumps can occur when the element is rendered initially and receives focus
+  Object.assign(container.style, { position: 'fixed', left: 0, top: 0, margin: 0 });
+
+  const forceUpdate = () => {
+    if (isFirstUpdate) {
+      scrollParents.add(getScrollParent(container));
+      if (target instanceof HTMLElement) {
+        scrollParents.add(getScrollParent(target));
+      }
+
+      scrollParents.forEach(scrollParent => {
+        scrollParent.addEventListener('scroll', updatePosition);
+      });
+
+      isFirstUpdate = false;
+    }
+
+    Object.assign(container.style, { position: strategy });
+    computePosition(target, container, { placement, middleware, strategy })
+      .then(({ x, y, middlewareData, placement: computedPlacement }) => {
+        writeArrowUpdates({ arrow, middlewareData });
+        writeContainerUpdates({
+          container,
+          middlewareData,
+          placement: computedPlacement,
+          coordinates: { x, y },
+          lowPPI: (targetWindow?.devicePixelRatio || 1) <= 1,
+          strategy,
+        });
+      })
+      .catch(err => {
+        // https://github.com/floating-ui/floating-ui/issues/1845
+        // FIXME for node > 14
+        // node 15 introduces promise rejection which means that any components
+        // tests need to be `it('', async () => {})` otherwise there can be race conditions with
+        // JSDOM being torn down before this promise is resolved so globals like `window` and `document` don't exist
+        // Unless all tests that ever use `usePositioning` are turned into async tests, any logging during testing
+        // will actually be counter productive
+        if (process.env.NODE_ENV === 'development') {
+          // eslint-disable-next-line no-console
+          console.error('[usePositioning]: Failed to calculate position', err);
+        }
+      });
+  };
+
+  const updatePosition = debounce(() => forceUpdate());
+
+  const dispose = () => {
+    if (targetWindow) {
+      targetWindow.removeEventListener('scroll', updatePosition);
+      targetWindow.removeEventListener('resize', updatePosition);
+    }
+
+    scrollParents.forEach(scrollParent => {
+      scrollParent.removeEventListener('scroll', updatePosition);
+    });
+  };
+
+  if (targetWindow) {
+    targetWindow.addEventListener('scroll', updatePosition);
+    targetWindow.addEventListener('resize', updatePosition);
+  }
+
+  // Update the position on initialization
+  updatePosition();
+
+  return {
+    updatePosition,
+    dispose,
+  };
+}

packages/react-components/react-positioning/src/types.ts

@@ -14,6 +14,39 @@ export type OffsetFunctionParam = {
   alignment?: Alignment;
 };
 
+export type TargetElement = HTMLElement | PositioningVirtualElement;
+
+/**
+ * @internal
+ */
+export interface UsePositioningOptions extends PositioningProps {
+  /**
+   * If false, does not position anything
+   */
+  enabled?: boolean;
+}
+
+/**
+ * @internal
+ */
+export interface PositionManager {
+  updatePosition: () => void;
+  dispose: () => void;
+}
+
+export interface UsePositioningReturn {
+  // React refs are supposed to be contravariant
+  // (allows a more general type to be passed rather than a more specific one)
+  // However, Typescript currently can't infer that fact for refs
+  // See https://github.com/microsoft/TypeScript/issues/30748 for more information
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  targetRef: React.MutableRefObject<any>;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  containerRef: React.MutableRefObject<any>;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  arrowRef: React.MutableRefObject<any>;
+}
+
 export type OffsetObject = { crossAxis?: number; mainAxis: number };
 
 export type OffsetShorthand = number;
@@ -40,7 +73,7 @@ export type PositioningImperativeRef = {
    * Sets the target and updates positioning imperatively.
    * Useful for avoiding double renders with the target option.
    */
-  setTarget: (target: HTMLElement | PositioningVirtualElement) => void;
+  setTarget: (target: TargetElement) => void;
 };
 
 export type PositioningVirtualElement = {
@@ -133,7 +166,7 @@ export interface PositioningProps
   /**
    * Manual override for the target element. Useful for scenarios where a component accepts user prop to override target
    */
-  target?: HTMLElement | PositioningVirtualElement | null;
+  target?: TargetElement | null;
 }
 
 export type PositioningShorthandValue =