fix(ui-motion): stop leaking elementRef onto emotion-wrapped DOM children

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

@@ -341,32 +341,38 @@ class BaseTransition extends Component<
 
     const child = ensureSingleChild(this.props.children) as ReactElement
 
-    const elementOnlyRef = (el: ReactInstance | Element | null) => {
-      if (el instanceof Element) {
-        this.handleRef(el)
-      }
-    }
-
-    // `typeof type === 'object'` => forwardRef wrapper (withStyle-decorated InstUI components)
-    const refProps =
-      typeof child.type === 'object'
-        ? {
-            // chain so the child's own elementRef still fires instead of being overwritten
-            elementRef: createChainedFunction(
-              (child.props as { elementRef?: (el: Element | null) => void })
-                ?.elementRef,
-              this.handleRef
-            ),
-            // fallback for forwardRef children that expose their node via `ref`, not elementRef
-            ref: elementOnlyRef
-          }
-        : {
-            // for host el / plain class|fn: findDOMNode is the fallback
-            ref: (el: ReactInstance | Element | null) =>
-              this.handleRef(
-                el instanceof Element ? el : (findDOMNode(el) as Element) ?? null
-              )
+    // InstUI components expose an `elementRef` prop, which hands back the DOM
+    // node without React having to read `ref` off the element (which warns).
+    // Only children that actually declare it can be given one: `typeof
+    // child.type === 'object'` is true of every forwardRef wrapper, including
+    // the one emotion wraps `<div css={...}>` in, and emotion forwards unknown
+    // props straight to the DOM node.
+    const acceptsElementRef = (
+      child.type as { allowedProps?: readonly string[] }
+    )?.allowedProps?.includes('elementRef')
+
+    const refProps = acceptsElementRef
+      ? {
+          // chain so the child's own elementRef still fires instead of being overwritten
+          elementRef: createChainedFunction(
+            (child.props as { elementRef?: (el: Element | null) => void })
+              ?.elementRef,
+            this.handleRef
+          ),
+          // fallback for forwardRef children that expose their node via `ref`, not elementRef
+          ref: (el: ReactInstance | Element | null) => {
+            if (el instanceof Element) {
+              this.handleRef(el)
+            }
           }
+        }
+      : {
+          // host el / plain class|fn: findDOMNode is the fallback
+          ref: (el: ReactInstance | Element | null) =>
+            this.handleRef(
+              el instanceof Element ? el : (findDOMNode(el) as Element) ?? null
+            )
+        }
 
     return safeCloneElement(child, {
       'aria-hidden': !this.props.in ? true : undefined,

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

@@ -23,6 +23,7 @@
  */
 
 import { Component, createRef, RefObject } from 'react'
+import type { ComponentType } from 'react'
 import {
   render,
   waitFor,
@@ -32,6 +33,8 @@ import { vi } from 'vitest'
 import type { MockInstance } from 'vitest'
 import '@testing-library/jest-dom'
 
+import { withStyle } from '@instructure/emotion'
+
 import { Transition } from '../index.js'
 import { getClassNames } from '../styles.js'
 
@@ -59,6 +62,23 @@ class ExampleComponent extends Component<any, any> {
   }
 }
 
+// Stands in for an InstUI component: `withStyle`-decorated, and declaring
+// `elementRef` in `allowedProps` the way real components do. ui-motion can't
+// import one directly — ui-alerts and friends depend on ui-motion.
+type StyledChildProps = { elementRef?: (el: Element | null) => void }
+
+class StyledChildBase extends Component<StyledChildProps> {
+  static allowedProps = ['elementRef']
+  render() {
+    return <div ref={this.props.elementRef}>{COMPONENT_TEXT}</div>
+  }
+}
+
+const StyledChild = withStyle(
+  () => ({}),
+  () => ({})
+)(StyledChildBase) as unknown as ComponentType<StyledChildProps>
+
 describe('<Transition />', () => {
   let consoleWarningMock: ReturnType<typeof vi.spyOn>
   let consoleErrorMock: ReturnType<typeof vi.spyOn>
@@ -275,4 +295,75 @@ describe('<Transition />', () => {
       expect(onExited).toHaveBeenCalled()
     })
   })
+
+  describe('capturing the child node', () => {
+    const warningsMatching = (mock: MockInstance, pattern: RegExp) =>
+      mock.mock.calls.filter((args: unknown[]) => pattern.test(args.join(' ')))
+
+    const elementRefWarnings = (mock: MockInstance) =>
+      warningsMatching(mock, /elementRef/)
+
+    const refIsNotAPropWarnings = (mock: MockInstance) =>
+      warningsMatching(mock, /`?ref`? is not a prop/)
+
+    // The repo compiles JSX with `jsxImportSource: '@emotion/react'`, so a child
+    // written with a `css` prop is rendered through emotion's own forwardRef
+    // wrapper. These children are the realistic case — Tray, DrawerTray, Modal
+    // and Alert all render one.
+    it('does not leak elementRef onto an emotion-wrapped host element', async () => {
+      const { getByText } = render(
+        <Transition type="fade" in={true}>
+          <div css={{ color: 'red' }}>hello</div>
+        </Transition>
+      )
+      const element = getByText('hello')
+
+      expect(element).not.toHaveAttribute('elementref')
+      expect(elementRefWarnings(consoleErrorMock)).toHaveLength(0)
+    })
+
+    it('still captures an emotion-wrapped host element', async () => {
+      const elementRef = vi.fn()
+      const { getByText } = render(
+        <Transition type="fade" in={true} elementRef={elementRef}>
+          <div css={{ color: 'red' }}>hello</div>
+        </Transition>
+      )
+
+      // the node reached handleRef, so the transition classes could be applied
+      expect(getByText('hello')).toHaveClass(getClass('fade', 'entered'))
+      await waitFor(() => {
+        expect(elementRef).toHaveBeenCalledWith(expect.any(Element))
+      })
+    })
+
+    // LX-4014 (#2618): a withStyle child plus an actually-running transition
+    // made React read `ref` off the element. Both conditions are required — a
+    // host child, or a mount that never transitions, won't reproduce it.
+    it('does not read `ref` off a withStyle child mid-transition', async () => {
+      const childElementRef = vi.fn()
+      const transitionElementRef = vi.fn()
+
+      const { getByText } = render(
+        <Transition
+          type="fade"
+          in={false}
+          transitionOnMount
+          elementRef={transitionElementRef}
+        >
+          <StyledChild elementRef={childElementRef} />
+        </Transition>
+      )
+
+      await waitFor(() => {
+        // the child's own elementRef is chained, not overwritten, and
+        // Transition still captured the node
+        expect(childElementRef).toHaveBeenCalledWith(expect.any(Element))
+        expect(transitionElementRef).toHaveBeenCalledWith(expect.any(Element))
+      })
+      expect(getByText(COMPONENT_TEXT)).toBeInTheDocument()
+      expect(refIsNotAPropWarnings(consoleErrorMock)).toHaveLength(0)
+      expect(elementRefWarnings(consoleErrorMock)).toHaveLength(0)
+    })
+  })
 })