Skip to content
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 composeRefs in React 19 #3283

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

dmitri-gb
Copy link
Contributor

@dmitri-gb dmitri-gb commented Dec 12, 2024

Description

Ref callbacks can now return cleanup functions in React 19. Currently it's not always possible to use the new cleanup-returning ref callbacks with Radix components. For instance, the following example with TextField from Radix Themes isn't working properly.

<TextField.Root
  ref={(el) => {
    console.log('created', el);
    return () => console.log('removed');
  }}
/>

Instead of logging removed when unmounting, this logs created null.

This PR fixes the issue. I added a single test for the Switch component to test the behaviour.

I also updated the main workspace React version to 19 and fixed a handful of type-checking issues. However, all packages should still be compatible with earlier React versions.

Storybook also needed an update, as the previous version was not compatible with React 19. I performed the update by simply running their codemod.

Callback refs can now return cleanup functions. A composed ref also
needs to invoke those functions upon cleanup.

I also updated the main workspace React version to 19 and fixed a
handful of type-checking issues. However, all packages should still be
compatible with earlier React versions.

Storybook also needed an update, as the previous version was not
compatible with React 19.
@chaance chaance merged commit 6d803c0 into radix-ui:main Dec 12, 2024
3 checks passed
@chaance
Copy link
Member

chaance commented Dec 12, 2024

Important to note that React 17/18 will log an error in the console when returning a function from a callback ref, but afaict it doesn't actually break anything. We'll need to add some annoying hack check for React 19 before returning the function.

@dmitri-gb
Copy link
Contributor Author

Important to note that React 17/18 will log an error in the console when returning a function from a callback ref

Hmm, you're right. I missed that.

I think an easy fix would be to only return our own clean-up function if any of the passed refs also returned one. Then the error would only (and rightfully) show up, if someone were to actually return a function from their ref callback in React <19.

@chaance
Copy link
Member

chaance commented Dec 12, 2024

@dmitri-gb I like that solution much better than trying to trick compilers 👍 Mind making that adjustment to the PR and I can merge?

@dmitri-gb
Copy link
Contributor Author

Since this is already merged, here's a new PR: #3285

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