Skip to content

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

Open
balzss wants to merge 1 commit into
masterfrom
fix/ui-motion-elementref-emotion
Open

fix(ui-motion): stop leaking elementRef onto emotion-wrapped DOM children#2672
balzss wants to merge 1 commit into
masterfrom
fix/ui-motion-elementref-emotion

Conversation

@balzss

@balzss balzss commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Any <Transition> child written with a css prop logs:

Warning: React does not recognize the `elementRef` prop on a DOM element.
    at span
    at BaseTransition
    at Transition -> DrawerTray -> DrawerLayout

BaseTransition treated typeof child.type === 'object' as "withStyle-decorated InstUI component". That's true of every forwardRef wrapper, including the one emotion's jsx runtime wraps around any element carrying a css prop — and emotion forwards unknown props straight to the DOM node. Introduced in aaa4a58 (#2618), shipped in v11.7.4.

Affects Tray, DrawerTray, RatingIcon v2 and Modal (constrain="parent") — each renders a css child that resolves to a host element.

Changes

  • Narrow the check to children that actually declare elementRef in allowedProps. Emotion's wrapper doesn't, so <div css={...}> takes the plain ref path; a withStyle child keeps the chained elementRef.

This is deliberately not a revert. #2618 fixed LX-4014, where a withStyle child plus a running transition made React read ref off the element and warn. I tried the straight revert of renderChildren first — it brings LX-4014 back, confirmed rather than assumed.

Test Plan

Two tests bracket the fix, so neither behaviour can regress into the other:

old typeof check plain revert this PR
elementRef leak on a css host child
LX-4014 ref is not a prop on a withStyle child
  • Both failure modes verified by temporarily swapping the implementation, not by reasoning.
  • The LX-4014 test uses a real withStyle-decorated child and an actually-running transition — both conditions are required to reproduce, per fix(ui-motion,ui-alerts): capture child DOM via elementRef in Transition to avoid React 'ref is not a prop' warning #2618.
  • ui-motion 21 passed. ui-tray ui-drawer-layout ui-modal ui-rating ui-overlays ui-toggle-details ui-tabs ui-popover 229 passed. Browser project (incl. ui-alerts) 190 passed. build:types clean.
  • Docs app with devtools open: Alert dismiss/re-add and all three DrawerLayout trays — clean console, no elementref attributes in the DOM.

Fixes INSTUI-5139

Reported by @drakeaharper in #instui — canvas-rce escalates console.error to a failure, so this alone failed 380 tests across 23 files. Supersedes #2667, which proposed the same allowedProps gate.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

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

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

BaseTransition treated `typeof child.type === 'object'` as "this is a
withStyle-decorated InstUI component". That's true of every forwardRef
wrapper, including the one emotion's jsx runtime wraps around any element
carrying a `css` prop — and emotion forwards unknown props straight to the
DOM node:

    React does not recognize the `elementRef` prop on a DOM element.

Tray, DrawerTray, RatingIcon v2 and Modal (constrain="parent") all render
such a child, so every tray and dialog built on them logged this.
Introduced in aaa4a58 (#2618), first shipped in v11.7.4; reported by
canvas-lms, where it failed 380 canvas-rce tests.

Narrow the check to children that actually declare `elementRef` in
`allowedProps`. Emotion's wrapper doesn't, so `<div css={...}>` falls back
to the plain `ref` path, while a withStyle child keeps the chained
`elementRef` that #2618 added.

Note this is deliberately not a revert. #2618 fixed LX-4014, where a
withStyle child plus a running transition made React read `ref` off the
element and warn. Reverting `renderChildren` brings that back — verified,
not assumed. Both behaviours are now pinned by tests that bracket the fix:
the leak test fails under the old `typeof` check, and the LX-4014 test
fails under a plain revert.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@balzss
balzss force-pushed the fix/ui-motion-elementref-emotion branch from 7069332 to 56fa2c9 Compare July 31, 2026 11:56
@github-actions

Copy link
Copy Markdown
Contributor

Visual regression report

⚠️ Changes detected.

Status Count
Unchanged 0
Changed 1
New 96
Removed 32

📊 View full report

Diff images (33)

alert.png — baseline no longer produced

avatar.png — baseline no longer produced

badge.png — baseline no longer produced

billboard.png — baseline no longer produced

breadcrumb.png — baseline no longer produced

button-and-derivatives.png — baseline no longer produced

byline.png — baseline no longer produced

calendar.png — baseline no longer produced

checkbox.png — baseline no longer produced

checkboxgroup.png — baseline no longer produced

colorpicker.png — baseline no longer produced

contextview.png — baseline no longer produced

custom-and-lucide-icons.png — baseline no longer produced

dateinput-dateinput2.png — baseline no longer produced

datetimeinput.png — baseline no longer produced

diff-demo.png — 6324 pixels differ

drilldown.png — baseline no longer produced

filedrop.png — baseline no longer produced

form-errors.png — baseline no longer produced

heading.png — baseline no longer produced

img.png — baseline no longer produced

link.png — baseline no longer produced

menu.png — baseline no longer produced

metric-pill-tag-timeselect-text.png — baseline no longer produced

options.png — baseline no longer produced

pagination.png — baseline no longer produced

progressbar.png — baseline no longer produced

select-simpleselect.png — baseline no longer produced

table.png — baseline no longer produced

tabs.png — baseline no longer produced

tooltip.png — baseline no longer produced

treebrowser.png — baseline no longer produced

view.png — baseline no longer produced

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

github-actions Bot pushed a commit that referenced this pull request Jul 31, 2026
github-actions Bot pushed a commit that referenced this pull request Jul 31, 2026
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.

1 participant