Provide nodeRef to Transition to fix StrictMode warning (#3865)

* Update react-transition-group

* Pass nodeRef into Transition

* WIP

* fix docs build

* Remove a few ignores and explain the last remaining one

* fixing some overlay refs passed to Transition

got rid of clone children in tooltip also since it would break if wrapper existed around the tooltip component

* fixing transition for Modal and Tray

needed to provide an actual DOM ref to OpenTransition nodeRef

* actually use wrapper refs

* fixing actionbar ref to Transition

* adding wrapper to attach transition nodeRef

technically we could attach it to any DOM node in the Modal/Tray/Popover since we dont have a transition on enter, but for correctness with reflow/.scrollTop we want it as the wrapping element

* fixing rebase

* fix lint

* fix yarn.lock

* fix Tray and Modal types

* fix TS strict

For some reason, Ref doesnt break TS strict linter but RefObject does...

* adding comments

* remove ref from Underlay

Co-authored-by: Niklas Mischkulnig <[email protected]>
Co-authored-by: Rob Snow <[email protected]>

Author: 3 people

packages/@react-spectrum/actionbar/src/ActionBar.tsx

@@ -21,7 +21,7 @@ import {FocusScope} from '@react-aria/focus';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {OpenTransition} from '@react-spectrum/overlays';
-import React, {ReactElement, useEffect, useRef} from 'react';
+import React, {ReactElement, Ref, useEffect, useRef} from 'react';
 import {SpectrumActionBarProps} from '@react-types/actionbar';
 import styles from './actionbar.css';
 import {Text} from '@react-spectrum/text';
@@ -31,13 +31,15 @@ import {useProviderProps} from '@react-spectrum/provider';
 
 function ActionBar<T extends object>(props: SpectrumActionBarProps<T>, ref: DOMRef<HTMLDivElement>) {
   let isOpen = props.selectedItemCount !== 0;
+  let domRef = useDOMRef(ref);
 
   return (
     <OpenTransition
+      nodeRef={domRef}
       in={isOpen}
       mountOnEnter
       unmountOnExit>
-      <ActionBarInnerWithRef {...props} ref={ref} />
+      <ActionBarInnerWithRef {...props} ref={domRef} />
     </OpenTransition>
   );
 }
@@ -46,7 +48,7 @@ interface ActionBarInnerProps<T> extends SpectrumActionBarProps<T> {
   isOpen?: boolean
 }
 
-function ActionBarInner<T>(props: ActionBarInnerProps<T>, ref: DOMRef<HTMLDivElement>) {
+function ActionBarInner<T>(props: ActionBarInnerProps<T>, ref: Ref<HTMLDivElement>) {
   props = useProviderProps(props);
 
   let {
@@ -59,7 +61,6 @@ function ActionBarInner<T>(props: ActionBarInnerProps<T>, ref: DOMRef<HTMLDivEle
   } = props;
 
   let {styleProps} = useStyleProps(props);
-  let domRef = useDOMRef(ref);
   let stringFormatter = useLocalizedStringFormatter(intlMessages);
 
   // Store the last count greater than zero in a ref so that we can retain it while rendering the fade-out animation.
@@ -88,7 +89,7 @@ function ActionBarInner<T>(props: ActionBarInnerProps<T>, ref: DOMRef<HTMLDivEle
         {...filterDOMProps(props)}
         {...styleProps}
         {...keyboardProps}
-        ref={domRef}
+        ref={ref}
         className={classNames(
           styles,
           'react-spectrum-ActionBar', {
@@ -127,7 +128,7 @@ function ActionBarInner<T>(props: ActionBarInnerProps<T>, ref: DOMRef<HTMLDivEle
   );
 }
 
-const ActionBarInnerWithRef = React.forwardRef(ActionBarInner) as <T>(props: SpectrumActionBarProps<T> & {ref?: DOMRef<HTMLDivElement>}) => ReturnType<typeof ActionBarInner>;
+const ActionBarInnerWithRef = React.forwardRef(ActionBarInner) as <T>(props: ActionBarInnerProps<T> & {ref?: Ref<HTMLDivElement>}) => ReactElement;
 
 /**
  * TODO: Add description of component here.

packages/@react-spectrum/overlays/src/Modal.tsx

@@ -18,27 +18,29 @@ import {Overlay} from './Overlay';
 import {OverlayProps} from '@react-types/overlays';
 import {OverlayTriggerState} from '@react-stately/overlays';
 import overrideStyles from './overlays.css';
-import React, {forwardRef, ReactNode, RefObject} from 'react';
+import React, {forwardRef, MutableRefObject, ReactNode, RefObject, useRef} from 'react';
 import {Underlay} from './Underlay';
 import {useViewportSize} from '@react-aria/utils';
 
-interface ModalProps extends AriaModalOverlayProps, StyleProps, OverlayProps {
+interface ModalProps extends AriaModalOverlayProps, StyleProps, Omit<OverlayProps, 'nodeRef'> {
   children: ReactNode,
   state: OverlayTriggerState,
   type?: 'modal' | 'fullscreen' | 'fullscreenTakeover'
 }
 
 interface ModalWrapperProps extends ModalProps {
-  isOpen?: boolean
+  isOpen?: boolean,
+  wrapperRef: MutableRefObject<HTMLDivElement>
 }
 
 function Modal(props: ModalProps, ref: DOMRef<HTMLDivElement>) {
   let {children, state, ...otherProps} = props;
   let domRef = useDOMRef(ref);
+  let wrapperRef = useRef<HTMLDivElement>(null);
 
   return (
-    <Overlay {...otherProps} isOpen={state.isOpen}>
-      <ModalWrapper {...props} ref={domRef}>
+    <Overlay {...otherProps} isOpen={state.isOpen} nodeRef={wrapperRef}>
+      <ModalWrapper {...props} wrapperRef={wrapperRef} ref={domRef}>
         {children}
       </ModalWrapper>
     </Overlay>
@@ -51,10 +53,9 @@ let typeMap = {
 };
 
 let ModalWrapper = forwardRef(function (props: ModalWrapperProps, ref: RefObject<HTMLDivElement>) {
-  let {type, children, state, isOpen} = props;
+  let {type, children, state, isOpen, wrapperRef} = props;
   let typeVariant = typeMap[type];
   let {styleProps} = useStyleProps(props);
-
   let {modalProps, underlayProps} = useModalOverlay(props, state, ref);
 
   let wrapperClassName = classNames(
@@ -87,8 +88,9 @@ let ModalWrapper = forwardRef(function (props: ModalWrapperProps, ref: RefObject
     '--spectrum-visual-viewport-height': viewport.height + 'px'
   };
 
+  // Attach Transition's nodeRef to outer most wrapper for node.reflow: https://github.com/reactjs/react-transition-group/blob/c89f807067b32eea6f68fd6c622190d88ced82e2/src/Transition.js#L231
   return (
-    <>
+    <div ref={wrapperRef}>
       <Underlay {...underlayProps} isOpen={isOpen} />
       <div className={wrapperClassName} style={style}>
         <div
@@ -100,7 +102,7 @@ let ModalWrapper = forwardRef(function (props: ModalWrapperProps, ref: RefObject
           {children}
         </div>
       </div>
-    </>
+    </div>
   );
 });
 

packages/@react-spectrum/overlays/src/Overlay.tsx

@@ -18,7 +18,7 @@ import React, {useCallback, useState} from 'react';
 import {Overlay as ReactAriaOverlay} from '@react-aria/overlays';
 
 function Overlay(props: OverlayProps, ref: DOMRef<HTMLDivElement>) {
-  let {children, isOpen, container, onEnter, onEntering, onEntered, onExit, onExiting, onExited} = props;
+  let {children, isOpen, container, onEnter, onEntering, onEntered, onExit, onExiting, onExited, nodeRef} = props;
   let [exited, setExited] = useState(!isOpen);
 
   let handleEntered = useCallback(() => {
@@ -53,7 +53,8 @@ function Overlay(props: OverlayProps, ref: DOMRef<HTMLDivElement>) {
           onExited={handleExited}
           onEnter={onEnter}
           onEntering={onEntering}
-          onEntered={handleEntered}>
+          onEntered={handleEntered}
+          nodeRef={nodeRef}>
           {children}
         </OpenTransition>
       </Provider>

packages/@react-spectrum/overlays/src/Popover.tsx

@@ -16,7 +16,7 @@ import {DOMRef, StyleProps} from '@react-types/shared';
 import {Overlay} from './Overlay';
 import {OverlayTriggerState} from '@react-stately/overlays';
 import overrideStyles from './overlays.css';
-import React, {forwardRef, ReactNode, RefObject, useRef, useState} from 'react';
+import React, {forwardRef, MutableRefObject, ReactNode, RefObject, useRef, useState} from 'react';
 import styles from '@adobe/spectrum-css-temp/components/popover/vars.css';
 import {Underlay} from './Underlay';
 import {useLayoutEffect} from '@react-aria/utils';
@@ -28,7 +28,8 @@ interface PopoverProps extends Omit<AriaPopoverProps, 'popoverRef' | 'maxHeight'
 }
 
 interface PopoverWrapperProps extends PopoverProps {
-  isOpen?: boolean
+  isOpen?: boolean,
+  wrapperRef: MutableRefObject<HTMLDivElement>
 }
 
 /**
@@ -52,10 +53,11 @@ function Popover(props: PopoverProps, ref: DOMRef<HTMLDivElement>) {
     ...otherProps
   } = props;
   let domRef = useDOMRef(ref);
+  let wrapperRef = useRef<HTMLDivElement>(null);
 
   return (
-    <Overlay {...otherProps} isOpen={state.isOpen}>
-      <PopoverWrapper ref={domRef} {...props}>
+    <Overlay {...otherProps} isOpen={state.isOpen} nodeRef={wrapperRef}>
+      <PopoverWrapper ref={domRef} {...props} wrapperRef={wrapperRef}>
         {children}
       </PopoverWrapper>
     </Overlay>
@@ -68,7 +70,8 @@ const PopoverWrapper = forwardRef((props: PopoverWrapperProps, ref: RefObject<HT
     isOpen,
     hideArrow,
     isNonModal,
-    state
+    state,
+    wrapperRef
   } = props;
   let {styleProps} = useStyleProps(props);
 
@@ -78,8 +81,9 @@ const PopoverWrapper = forwardRef((props: PopoverWrapperProps, ref: RefObject<HT
     maxHeight: null
   }, state);
 
+  // Attach Transition's nodeRef to outer most wrapper for node.reflow: https://github.com/reactjs/react-transition-group/blob/c89f807067b32eea6f68fd6c622190d88ced82e2/src/Transition.js#L231
   return (
-    <>
+    <div ref={wrapperRef}>
       {!isNonModal && <Underlay isTransparent {...underlayProps} isOpen={isOpen} /> }
       <div
         {...styleProps}
@@ -115,7 +119,7 @@ const PopoverWrapper = forwardRef((props: PopoverWrapperProps, ref: RefObject<HT
         )}
         <DismissButton onDismiss={state.close} />
       </div>
-    </>
+    </div>
   );
 });
 

packages/@react-spectrum/overlays/src/Tray.tsx

@@ -17,28 +17,30 @@ import {Overlay} from './Overlay';
 import {OverlayProps} from '@react-types/overlays';
 import {OverlayTriggerState} from '@react-stately/overlays';
 import overrideStyles from './overlays.css';
-import React, {forwardRef, ReactNode, RefObject} from 'react';
+import React, {forwardRef, MutableRefObject, ReactNode, RefObject, useRef} from 'react';
 import trayStyles from '@adobe/spectrum-css-temp/components/tray/vars.css';
 import {Underlay} from './Underlay';
 import {useViewportSize} from '@react-aria/utils';
 
-interface TrayProps extends AriaModalOverlayProps, StyleProps, OverlayProps {
+interface TrayProps extends AriaModalOverlayProps, StyleProps, Omit<OverlayProps, 'nodeRef'> {
   children: ReactNode,
   state: OverlayTriggerState,
   isFixedHeight?: boolean
 }
 
 interface TrayWrapperProps extends TrayProps {
-  isOpen?: boolean
+  isOpen?: boolean,
+  wrapperRef: MutableRefObject<HTMLDivElement>
 }
 
 function Tray(props: TrayProps, ref: DOMRef<HTMLDivElement>) {
   let {children, state, ...otherProps} = props;
   let domRef = useDOMRef(ref);
+  let wrapperRef = useRef<HTMLDivElement>(null);
 
   return (
-    <Overlay {...otherProps} isOpen={state.isOpen}>
-      <TrayWrapper {...props} ref={domRef}>
+    <Overlay {...otherProps} isOpen={state.isOpen} nodeRef={wrapperRef}>
+      <TrayWrapper {...props} wrapperRef={wrapperRef} ref={domRef}>
         {children}
       </TrayWrapper>
     </Overlay>
@@ -50,7 +52,8 @@ let TrayWrapper = forwardRef(function (props: TrayWrapperProps, ref: RefObject<H
     children,
     isOpen,
     isFixedHeight,
-    state
+    state,
+    wrapperRef
   } = props;
   let {styleProps} = useStyleProps(props);
 
@@ -91,8 +94,9 @@ let TrayWrapper = forwardRef(function (props: TrayWrapperProps, ref: RefObject<H
     styleProps.className
   );
 
+  // Attach Transition's nodeRef to outer most wrapper for node.reflow: https://github.com/reactjs/react-transition-group/blob/c89f807067b32eea6f68fd6c622190d88ced82e2/src/Transition.js#L231
   return (
-    <>
+    <div ref={wrapperRef}>
       <Underlay {...underlayProps} isOpen={isOpen} />
       <div className={wrapperClassName} style={wrapperStyle}>
         <div
@@ -106,7 +110,7 @@ let TrayWrapper = forwardRef(function (props: TrayWrapperProps, ref: RefObject<H
           <DismissButton onDismiss={state.close} />
         </div>
       </div>
-    </>
+    </div>
   );
 });
 

packages/@react-spectrum/tooltip/src/TooltipTrigger.tsx

@@ -31,8 +31,7 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
     trigger: triggerAction
   } = props;
 
-  let [trigger, tooltip] = React.Children.toArray(children);
-
+  let [trigger, tooltip] = React.Children.toArray(children) as [ReactElement, ReactElement];
   let state = useTooltipTriggerState(props);
 
   let tooltipTriggerRef = useRef<HTMLElement>();
@@ -68,7 +67,7 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
           arrowProps,
           ...tooltipProps
         }}>
-        <Overlay isOpen={state.isOpen}>
+        <Overlay isOpen={state.isOpen} nodeRef={overlayRef}>
           {tooltip}
         </Overlay>
       </TooltipContext.Provider>

packages/@react-types/overlays/src/index.d.ts

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {HTMLAttributes, ReactElement, ReactNode} from 'react';
+import {HTMLAttributes, MutableRefObject, ReactElement, ReactNode} from 'react';
 import {StyleProps} from '@react-types/shared';
 
 export type Placement = 'bottom' | 'bottom left' | 'bottom right' | 'bottom start' | 'bottom end' |
@@ -71,18 +71,19 @@ export interface OverlayProps {
   onEntered?: () => void,
   onExit?: () => void,
   onExiting?: () => void,
-  onExited?: () => void
+  onExited?: () => void,
+  nodeRef: MutableRefObject<HTMLElement>
 }
 
-export interface ModalProps extends StyleProps, OverlayProps {
+export interface ModalProps extends StyleProps, Omit<OverlayProps, 'nodeRef'> {
   children: ReactElement,
   isOpen?: boolean,
   onClose?: () => void,
   type?: 'modal' | 'fullscreen' | 'fullscreenTakeover',
   isDismissable?: boolean
 }
 
-export interface PopoverProps extends StyleProps, OverlayProps {
+export interface PopoverProps extends StyleProps, Omit<OverlayProps, 'nodeRef'>  {
   children: ReactNode,
   placement?: PlacementAxis,
   arrowProps?: HTMLAttributes<HTMLElement>,
@@ -94,7 +95,7 @@ export interface PopoverProps extends StyleProps, OverlayProps {
   isDismissable?: boolean
 }
 
-export interface TrayProps extends StyleProps, OverlayProps {
+export interface TrayProps extends StyleProps, Omit<OverlayProps, 'nodeRef'>  {
   children: ReactElement,
   isOpen?: boolean,
   onClose?: () => void,