-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: remove accessing elem.ref in renderTransitions #2373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove accessing elem.ref in renderTransitions #2373
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
82482f6
to
fd593f6
Compare
Thank you @robertjarske I can reproduce the error message. Note that it is not really an error. React is a bit dramatic about it, it is just a deprecation. But after further testing this solution is not retro compatible with React 18. And the v10 still supports it. Removing the mechanism in useTransition, as you can see below, breaks the React 18 compatibility, the log show an undefined ref. If you really want to get rid of the warning, maybe a solution could be to have a fallback to |
Thank you for checking @dagatsoin We could get the react version and have a check that solves this issue, what do you think about something like this? //tsconfig.json
{
...
"compilerOptions": {
...
"resolveJsonModule": true
}
}
//useTransition.tsx
import { version as reactVersion } from 'react/package.json'
...
const renderTransitions: TransitionFn = render => (
<>
{transitions.map((t, i) => {
const { springs } = changes.get(t) || t.ctrl
const elem: any = render({ ...springs }, t.item, t, i)
return elem && elem.type ? (
<elem.type
{...elem.props}
key={is.str(t.key) || is.num(t.key) ? t.key : t.ctrl.id}
ref={reactVersion < "19.0.0" ? elem.ref : elem.props.ref}
/>
) : (
elem
)
})}
</>
) |
mmh I am not use to this. I wonder if it works when you use the module file directly, like with https://unpkg.com/ |
both getting the version from package.json and using React.version seems to be working from what I can see Using this
I get this when running 18.3.1 for both react and react-dom And this when running 19.1.0 for both react and react-dom Neither @types/react or @types/react-dom have been updated here and remain at their previous versions |
Does React.version works also when the file comes from unpkg ? If so, I would go for it. @joshuaellis what do you think about this ? |
Seems to be working as intended with unpkg as well |
04d1b80
to
15c5b28
Compare
let @joshuaellis say the final work now |
c37ee49
to
18ace1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is probably fine, it doesn't seem like there's advise on doing it a different way for library maintainers at the moment. We can always tweak in the future.
the tests failed with a "Cannot read properties of null (reading 'props')" |
e236613
to
2c92a8d
Compare
2c92a8d
to
3a62f3e
Compare
Why
Accessing element.ref is deprecated. Refs are now regular props and should be accessed using element.props.ref. Accessing element.ref produces a web console error.
https://react.dev/blog/2024/04/25/react-19-upgrade-guide#deprecated-element-ref
What
Removed accessing elem.ref in useTransition.tsx. Props are already spreaded.
Checklist