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

[v9] chore: replace forwardRef with props.ref #3282

Merged
merged 3 commits into from
Jan 10, 2025

Conversation

alexandernanberg
Copy link
Contributor


React.useImperativeHandle(ref, () => props)
// React.useImperativeHandle(ref, () => props)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idk what to do with this? It didn't work before either

Copy link

codesandbox-ci bot commented Jun 7, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 36a6774:

Sandbox Source
example Configuration

@alexandernanberg alexandernanberg changed the title Migrate to props.ref Replace forwardRef with props.ref Jun 20, 2024
Copy link

codesandbox-ci bot commented Sep 10, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8abd59d:

Sandbox Source
example Configuration

@alexandernanberg alexandernanberg changed the title Replace forwardRef with props.ref v9 Replace forwardRef with props.ref Sep 15, 2024
@alexandernanberg alexandernanberg changed the title v9 Replace forwardRef with props.ref [v9] chore: replace forwardRef with props.ref Sep 15, 2024
@borisghidaglia
Copy link

Hey @alexandernanberg 👋

Do you know if this could cause problems with extend ?
I'm having troubles on this PR.

@alexandernanberg
Copy link
Contributor Author

@borisghidaglia No, this is not included in the v9 release atm. And I see no reason to why it would affect extend

@borisghidaglia
Copy link

borisghidaglia commented Jan 4, 2025

Yes I know it's not included!
But I removed the forwardRef functions in my linked PR as well. That's why what you did is relevant to me.

And about why it might affect extend, I've read that r3f "reconciler" used refs to do its job (which I couldn't explain as it's unclear to me atm).

In theory I believe removing forwardRef in favor of ref props shouldn't change anything at runtime, but I figured there might be things I didn't think about.

@CodyJasonBennett
Copy link
Member

CodyJasonBennett commented Jan 4, 2025

I missed this PR, but the changes are indeed unrelated. You likely have a broken build or duplicated installs of R3F itself. We're working on getting RC channels of libraries out (chiefly R3F and Drei), and then we'll start rolling them out to stable. That should make things easier for your package manager as well as testing.

@borisghidaglia
Copy link

borisghidaglia commented Jan 4, 2025

Thanks for the answer @CodyJasonBennett.

I'm actually using "@react-three/fiber": "^9.0.0-rc.1" in the PR.

My goal was to get @react-three-csg ready for when R3F will be rolled out as stable.

I'm going to try to update Drei to the RC channel and see if it solves the problem I'm having with extend, thanks!

@CodyJasonBennett CodyJasonBennett merged commit b3e4ee4 into pmndrs:v9 Jan 10, 2025
2 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