fix(ui-motion,ui-alerts): silence "ref is not a prop" warning when wrapping class components in Transition

BaseTransition.renderChildren inspects the child's React type to decide
which ref props to attach:
- host elements (<div>, <span>) receive a plain ref callback
- forwardRef components (including memo-wrapped ones) receive both
  elementRef (the InstUI convention) and ref (for non-InstUI forwardRef
  children)
- any other component shape (class, plain function, Context.Provider,
  etc.) receives elementRef only, since attaching ref to a non-forwardRef
  component would bind to the class instance or trigger a React warning

handleChildRef ignores non-Element values so that a forwardRef chain
landing on a class instance does not clobber a DOM node already captured
via elementRef.

Alert.handleRef forwards the consumer-supplied elementRef so that
parents like Transition can observe Alert's mounted DOM node; without
chaining, Alert silently swallowed the elementRef it received.

Author: Gazuru

packages/ui-alerts/src/Alert/v1/index.tsx

@@ -98,6 +98,12 @@ class Alert extends Component<AlertProps, AlertState> {
 
   handleRef = (el: Element | null) => {
     this.ref = el
+    const { elementRef } = this.props as AlertProps & {
+      elementRef?: (el: Element | null) => void
+    }
+    if (typeof elementRef === 'function') {
+      elementRef(el)
+    }
   }
 
   handleTimeout = () => {

packages/ui-alerts/src/Alert/v2/index.tsx

@@ -97,6 +97,12 @@ class Alert extends Component<AlertProps, AlertState> {
 
   handleRef = (el: Element | null) => {
     this.ref = el
+    const { elementRef } = this.props as AlertProps & {
+      elementRef?: (el: Element | null) => void
+    }
+    if (typeof elementRef === 'function') {
+      elementRef(el)
+    }
   }
 
   handleTimeout = () => {

packages/ui-motion/src/Transition/BaseTransition/index.ts

@@ -24,11 +24,12 @@
 
 import { Component, ReactElement } from 'react'
 
-import { getClassList, findDOMNode } from '@instructure/ui-dom-utils'
+import { getClassList } from '@instructure/ui-dom-utils'
 import {
   ensureSingleChild,
   safeCloneElement
 } from '@instructure/ui-react-utils'
+import { createChainedFunction } from '@instructure/ui-utils'
 
 import { allowedProps } from './props'
 import type {
@@ -38,6 +39,24 @@ import type {
   BaseTransitionStatesType
 } from './props'
 
+const REACT_FORWARD_REF_TYPE = Symbol.for('react.forward_ref')
+const REACT_MEMO_TYPE = Symbol.for('react.memo')
+
+type ElementRefCallback = (el: Element | null) => void
+type ChildElementType = ReactElement['type']
+
+function unwrapMemo(type: ChildElementType): ChildElementType {
+  let unwrapped = type
+  while (
+    unwrapped &&
+    typeof unwrapped !== 'string' &&
+    (unwrapped as { $$typeof?: symbol }).$$typeof === REACT_MEMO_TYPE
+  ) {
+    unwrapped = (unwrapped as unknown as { type: ChildElementType }).type
+  }
+  return unwrapped
+}
+
 const STATES: Record<BaseTransitionStateValue, BaseTransitionStatesType> = {
   EXITED: -2,
   EXITING: -1,
@@ -88,6 +107,16 @@ class BaseTransition extends Component<
     }
   }
 
+  handleChildRef = (el: unknown) => {
+    // The `ref` we attach to a forwardRef child can end up bound to a class
+    // instance (when the forwardRef chain wraps a class component). Ignore
+    // those so they don't clobber a DOM node we got via `elementRef`.
+    if (el != null && !(el instanceof Element)) return
+    const next = el instanceof Element ? el : null
+    if (next === this.ref) return
+    this.handleRef(next)
+  }
+
   componentDidMount() {
     this.startTransition(this.props.in, this.props.transitionOnMount)
     this._unmounted = false
@@ -333,19 +362,45 @@ class BaseTransition extends Component<
   }
 
   renderChildren() {
-    return this.props.children
-      ? safeCloneElement(
-          ensureSingleChild(this.props.children) as ReactElement,
-          {
-            'aria-hidden': !this.props.in ? true : undefined,
-            ref: (el: React.ReactInstance | null) => {
-              const ref = (findDOMNode(el) as Element) || null
-
-              this.handleRef(ref)
-            }
-          }
-        )
-      : null
+    if (!this.props.children) return null
+
+    const child = ensureSingleChild(this.props.children) as ReactElement
+
+    return safeCloneElement(child, {
+      'aria-hidden': !this.props.in ? true : undefined,
+      ...(this.refPropsForChild(child) as Record<string, unknown>)
+    })
+  }
+
+  // Host elements receive a plain `ref`. `forwardRef` components (including
+  // memo-wrapped ones) receive both `ref` (for non-InstUI forwardRef
+  // children) and `elementRef` (the InstUI convention). Any other component
+  // shape (class, function, Context.Provider, etc.) only gets `elementRef` —
+  // attaching `ref` to a non-forwardRef component would either bind to the
+  // class instance (which we can't resolve to a DOM node without
+  // findDOMNode) or trigger a React "Function components cannot be given
+  // refs" warning.
+  // If the child already carries an `elementRef` prop from the consumer,
+  // we chain it with ours so the consumer's callback isn't clobbered.
+  refPropsForChild(child: ReactElement) {
+    if (typeof child.type === 'string') {
+      return { ref: this.handleChildRef }
+    }
+
+    const consumerElementRef = child.props?.elementRef
+    const mergedElementRef: ElementRefCallback =
+      typeof consumerElementRef === 'function'
+        ? (createChainedFunction(
+            consumerElementRef as ElementRefCallback,
+            this.handleChildRef
+          ) as ElementRefCallback)
+        : this.handleChildRef
+
+    const unwrapped = unwrapMemo(child.type) as { $$typeof?: symbol }
+    if (unwrapped?.$$typeof === REACT_FORWARD_REF_TYPE) {
+      return { elementRef: mergedElementRef, ref: this.handleChildRef }
+    }
+    return { elementRef: mergedElementRef }
   }
 
   render() {

packages/ui-motion/src/Transition/__tests__/Transition.test.tsx

@@ -22,7 +22,7 @@
  * SOFTWARE.
  */
 
-import { Component, createRef, RefObject } from 'react'
+import { Component, forwardRef, memo, useCallback } from 'react'
 import {
   render,
   waitFor,
@@ -47,18 +47,83 @@ const getClass = (
   return classNames[phase]
 }
 
-class ExampleComponent extends Component<any, any> {
-  private ref: RefObject<any>
+// A non-InstUI forwardRef component: exposes the DOM node via React's
+// built-in ref forwarding only. Intentionally does not spread all props
+// onto the DOM so unrecognized props (like the InstUI-convention
+// `elementRef` BaseTransition also passes) don't leak as DOM attributes.
+const ExampleComponent = forwardRef<HTMLDivElement>((_props, ref) => {
+  return <div ref={ref}>{COMPONENT_TEXT}</div>
+})
+ExampleComponent.displayName = 'ExampleComponent'
 
-  constructor(props: any) {
-    super(props)
-    this.ref = createRef()
-  }
+// Mirrors the InstUI functional-component pattern: exposes the DOM node via
+// an `elementRef` prop rather than via React's built-in ref forwarding.
+type InstUIStyleComponentProps = {
+  elementRef?: (el: Element | null) => void
+}
+const InstUIStyleComponent = ({ elementRef }: InstUIStyleComponentProps) => {
+  return (
+    <div
+      ref={(el) => {
+        if (elementRef) elementRef(el)
+      }}
+    >
+      {COMPONENT_TEXT}
+    </div>
+  )
+}
+
+// Mirrors an InstUI v1 class component: exposes the DOM node only via
+// `elementRef`, not via React's built-in ref.
+class InstUIStyleClassComponent extends Component<InstUIStyleComponentProps> {
   render() {
-    return <div ref={this.ref}>{COMPONENT_TEXT}</div>
+    const { elementRef } = this.props
+    return (
+      <div
+        ref={(el) => {
+          if (elementRef) elementRef(el)
+        }}
+      >
+        {COMPONENT_TEXT}
+      </div>
+    )
   }
 }
 
+// Honors both `ref` (via forwardRef) and `elementRef` (InstUI convention),
+// letting us verify BaseTransition doesn't fire its ref bookkeeping twice
+// when a child wires up both.
+const DualRefComponent = forwardRef<HTMLDivElement, InstUIStyleComponentProps>(
+  ({ elementRef }, ref) => {
+    // Memoized to avoid a fresh ref callback on every render, which would
+    // cause React to detach/reattach the ref and muddy the dedupe assertion.
+    const setRef = useCallback(
+      (el: HTMLDivElement | null) => {
+        if (typeof ref === 'function') {
+          ref(el)
+        } else if (ref) {
+          // eslint-disable-next-line no-param-reassign
+          ref.current = el
+        }
+        if (elementRef) elementRef(el)
+      },
+      [ref, elementRef]
+    )
+    return <div ref={setRef}>{COMPONENT_TEXT}</div>
+  }
+)
+DualRefComponent.displayName = 'DualRefComponent'
+
+// memo(forwardRef(...)) — a common third-party shape. BaseTransition must
+// unwrap the memo layer to detect the forwardRef underneath, otherwise the
+// DOM node isn't captured.
+const MemoForwardRefComponent = memo(
+  forwardRef<HTMLDivElement>((_props, ref) => {
+    return <div ref={ref}>{COMPONENT_TEXT}</div>
+  })
+)
+MemoForwardRefComponent.displayName = 'MemoForwardRefComponent'
+
 describe('<Transition />', () => {
   let consoleWarningMock: ReturnType<typeof vi.spyOn>
   let consoleErrorMock: ReturnType<typeof vi.spyOn>
@@ -107,6 +172,33 @@ describe('<Transition />', () => {
       )
       const element = getByText(COMPONENT_TEXT)
 
+      expect(element).toHaveClass(getClass(type, 'entered'))
+      // BaseTransition passes both `ref` and `elementRef` to component
+      // children so InstUI and non-InstUI forwardRef components both work.
+      // A well-behaved forwardRef child must not leak the unrecognized
+      // `elementRef` prop onto the DOM (React would warn in dev).
+      expect(consoleErrorMock).not.toHaveBeenCalled()
+    })
+
+    it(`should correctly apply classes for '${type}' with a component that only exposes elementRef`, () => {
+      const { getByText } = render(
+        <Transition type={type} in={true}>
+          <InstUIStyleComponent />
+        </Transition>
+      )
+      const element = getByText(COMPONENT_TEXT)
+
+      expect(element).toHaveClass(getClass(type, 'entered'))
+    })
+
+    it(`should correctly apply classes for '${type}' with a class component that only exposes elementRef`, () => {
+      const { getByText } = render(
+        <Transition type={type} in={true}>
+          <InstUIStyleClassComponent />
+        </Transition>
+      )
+      const element = getByText(COMPONENT_TEXT)
+
       expect(element).toHaveClass(getClass(type, 'entered'))
     })
   }
@@ -275,4 +367,78 @@ describe('<Transition />', () => {
       expect(onExited).toHaveBeenCalled()
     })
   })
+
+  it("should forward the child's rendered DOM element to Transition's elementRef", () => {
+    const transitionElementRef = vi.fn()
+
+    const { getByText } = render(
+      <Transition type="fade" in={true} elementRef={transitionElementRef}>
+        <InstUIStyleComponent />
+      </Transition>
+    )
+    const element = getByText(COMPONENT_TEXT)
+
+    expect(transitionElementRef).toHaveBeenCalledWith(element)
+  })
+
+  it('should not double-fire elementRef when the child honors both ref and elementRef', () => {
+    const transitionElementRef = vi.fn()
+
+    render(
+      <Transition type="fade" in={true} elementRef={transitionElementRef}>
+        <DualRefComponent />
+      </Transition>
+    )
+
+    const calls = transitionElementRef.mock.calls.filter(
+      ([arg]) => arg instanceof Element
+    )
+    expect(calls).toHaveLength(1)
+  })
+
+  it('should capture the DOM of a memo(forwardRef(...)) child', () => {
+    const transitionElementRef = vi.fn()
+
+    const { getByText } = render(
+      <Transition type="fade" in={true} elementRef={transitionElementRef}>
+        <MemoForwardRefComponent />
+      </Transition>
+    )
+    const element = getByText(COMPONENT_TEXT)
+
+    expect(element).toHaveClass(getClass('fade', 'entered'))
+    expect(transitionElementRef).toHaveBeenCalledWith(element)
+  })
+
+  it("should chain the consumer's own elementRef with Transition's", () => {
+    const transitionElementRef = vi.fn()
+    const childElementRef = vi.fn()
+
+    const { getByText } = render(
+      <Transition type="fade" in={true} elementRef={transitionElementRef}>
+        <InstUIStyleComponent elementRef={childElementRef} />
+      </Transition>
+    )
+    const element = getByText(COMPONENT_TEXT)
+
+    expect(transitionElementRef).toHaveBeenCalledWith(element)
+    expect(childElementRef).toHaveBeenCalledWith(element)
+  })
+
+  it('should not throw when the child has a non-callback elementRef prop', () => {
+    // Some consumers may pass a RefObject instead of a callback. InstUI's
+    // convention is callback-style, but a runtime throw would be worse than
+    // quietly ignoring the unsupported shape.
+    const objectRef = { current: null }
+
+    expect(() =>
+      render(
+        <Transition type="fade" in={true}>
+          <InstUIStyleComponent
+            elementRef={objectRef as unknown as (el: Element | null) => void}
+          />
+        </Transition>
+      )
+    ).not.toThrow()
+  })
 })