Skip to content

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

Merged
merged 6 commits into from
May 27, 2025

Conversation

robertjarske
Copy link
Contributor

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

  • Documentation updated
  • Demo added
  • Ready to be merged

Copy link

vercel bot commented May 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
react-spring ✅ Ready (Inspect) Visit Preview May 24, 2025 6:28pm

Copy link

changeset-bot bot commented May 21, 2025

⚠️ No Changeset found

Latest commit: 3a62f3e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dagatsoin
Copy link
Contributor

Thank you @robertjarske

I can reproduce the error message.

Repro with React 19 ✅
image

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.

QA under react 18 ❌
image

QA under react 19 ✅
image

If you really want to get rid of the warning, maybe a solution could be to have a fallback to element.props.ref somehow.

@robertjarske
Copy link
Contributor Author

robertjarske commented May 21, 2025

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
        )
      })}
    </>
  )

@dagatsoin
Copy link
Contributor

mmh I am not use to this. I wonder if it works when you use the module file directly, like with https://unpkg.com/

@robertjarske
Copy link
Contributor Author

robertjarske commented May 21, 2025

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

diff --git a/dist/react-spring_core.modern.mjs b/dist/react-spring_core.modern.mjs
index 631624fb630980556d47f13cb52a16fa09e22950..017bed34b0b0db1def4be96f7e654380e1bc4876 100644
--- a/dist/react-spring_core.modern.mjs
+++ b/dist/react-spring_core.modern.mjs
@@ -1863,6 +1863,7 @@ function useTrail(length, propsArg, deps) {
 
 // src/hooks/useTransition.tsx
 import * as React2 from "react";
+import {version as reactVersion} from "react/package.json"
 import { useContext as useContext3, useRef as useRef2, useMemo as useMemo3 } from "react";
 import {
   is as is11,
@@ -1874,6 +1875,8 @@ import {
   useIsomorphicLayoutEffect as useIsomorphicLayoutEffect4
 } from "@react-spring/shared";
 function useTransition(data, props, deps) {
+console.log("React.version: ", React2.version)
+console.log("Version from package.json: ", reactVersion)
   const propsFn = is11.fun(props) && props;
   const {
     reset,

I get this when running 18.3.1 for both react and react-dom
image

And this when running 19.1.0 for both react and react-dom
image

Neither @types/react or @types/react-dom have been updated here and remain at their previous versions

@dagatsoin
Copy link
Contributor

dagatsoin commented May 21, 2025

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 ?

@robertjarske
Copy link
Contributor Author

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
https://codesandbox.io/p/sandbox/3hzwf6

image

@dagatsoin
Copy link
Contributor

let @joshuaellis say the final work now

joshuaellis
joshuaellis previously approved these changes May 23, 2025
Copy link
Member

@joshuaellis joshuaellis left a 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.

@robertjarske
Copy link
Contributor Author

the tests failed with a "Cannot read properties of null (reading 'props')"
should be fixed now @joshuaellis

@joshuaellis joshuaellis merged commit 197e0a7 into pmndrs:next May 27, 2025
14 checks passed
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.

3 participants