Skip to content

fix(ui-motion,ui-alerts): capture child DOM via elementRef in Transition to avoid React 'ref is not a prop' warning#2618

Open
joyenjoyer wants to merge 1 commit into
masterfrom
ref-not-a-prop-v2
Open

fix(ui-motion,ui-alerts): capture child DOM via elementRef in Transition to avoid React 'ref is not a prop' warning#2618
joyenjoyer wants to merge 1 commit into
masterfrom
ref-not-a-prop-v2

Conversation

@joyenjoyer

@joyenjoyer joyenjoyer commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • BaseTransition now captures the child DOM node by branching on typeof child.type: forwardRef/withStyle children go through the elementRef convention (chained so a consumer elementRef still fires), so React no longer reads ref as a prop and stops warning.
  • Host elements and plain class/fn children keep a ref callback with findDOMNode as the last-resort fallback.

How it works (reviewer guide)

The lifecycle is unchanged — only how this.ref (the child's DOM node) gets captured in renderChildren() changed. Everything downstream still needs a real DOM Element by the time transition() runs:

render() -> renderChildren()          [clone child + attach ref-capture]
        |
        v
  React attaches ref/elementRef -> handleRef(el) -> this.ref = <DOM node>
        |                                            + chains consumer elementRef
        v
  componentDidMount() -> startTransition() -> transition()
        |                                          |
        +-------- getClassList(this.ref) <---------+   [needs a real Element here]

The one changed decision — how the node is captured, keyed on typeof child.type:

                 ensureSingleChild(children)
                            |
              typeof child.type === 'object' ?
                            |
        YES (forwardRef / withStyle)          NO (host "div" / plain class|fn)
                    |                                       |
   elementRef: chain(child.elementRef,        ref: (el) =>
                     handleRef)                  el instanceof Element
     (primary: InstUI convention;                 ? el
      NOT reading child's `ref` =>                 : findDOMNode(el)   <- last-resort
      no "ref is not a prop" warning)                                     legacy fallback
   ref: elementOnlyRef                                    |
     (fallback for forwardRef kids that                   v
      expose the node via `ref`, not               handleRef(el)
      elementRef; keeps Element only)
                    |
                    v
              handleRef(el) -> this.ref

Reproduction

Confirmed on this repo's React 18.3.1:

Transition child master this PR
real withStyle InstUI component (View, Alert content, …) `ref` is not a prop warning none
bare host <div> / plain forwardRef none none

Two conditions are both required — a static host child, or a mount that never runs a transition, will not show it:

  1. The child is a real InstUI (withStyle-wrapped) component, so safeCloneElement re-clones it.
  2. A transition actually runs — mount with in={false}, or toggle in (a static in={true} mount with transitionOnMount={false} won't).
import { useState } from 'react'
import { Transition } from '@instructure/ui-motion'
import { View } from '@instructure/ui-view'

function Repro() {
  const [open, setOpen] = useState(false)
  return (
    <>
      <button onClick={() => setOpen((o) => !o)}>toggle</button>
      {/* View is a withStyle/forwardRef InstUI component; running the
          transition re-clones it, which trips the warning on master */}
      <Transition in={open} type="fade">
        <View as="div" padding="small">Animated content</View>
      </Transition>
    </>
  )
}

On master, initiating the transition logs Warning: … `ref` is not a prop. Trying to access it will result in `undefined` being returned.; with this PR the warning is gone and the transition still animates.

Test Plan

  • Reproduce per the snippet above and confirm the `ref` is not a prop warning is gone after the change (needs a withStyle child + an actually-run transition).
  • Confirm animations still play for InstUI children (elementRef path) and host-element children (ref path).

Fixes LX-4014 INSTUI-5026

🤖 Generated with Claude Code

…ion to avoid React 'ref is not a prop' warning
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://instructure.design/pr-preview/pr-2618/

Built to branch gh-pages at 2026-07-01 12:55 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

github-actions Bot pushed a commit that referenced this pull request Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Visual regression report

No changes.

Status Count
Unchanged 32
Changed 0
New 0
Removed 0

📊 View full report

Baselines come from the visual-baselines branch. They refresh on every merge to master.

@ToMESSKa ToMESSKa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joyenjoyer This code snipper you attached does not animate:

function Repro() {
  const [open, setOpen] = useState(false)
  return (
    <>
      <button onClick={() => setOpen((o) => !o)}>toggle</button>
      {/* View is a withStyle/forwardRef InstUI component; running the
          transition re-clones it, which trips the warning on master */}
      <Transition in={open} type="fade">
        <View as="div" padding="small">Animated content</View>
      </Transition>
    </>
  )
}
render(<Repro />)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants