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: support for react 19 and next15 #2731

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Viraj-10
Copy link

@Viraj-10 Viraj-10 commented Oct 28, 2024

Changelog

Copy link

codesandbox-ci bot commented Oct 28, 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 da016a5:

Sandbox Source
react-native-web-examples Configuration

@javascripter javascripter mentioned this pull request Nov 22, 2024
1 task
@fbartho
Copy link

fbartho commented Jan 30, 2025

This PR looks surprisingly straightforward to me. (Great work!)

Do we think it's feature-complete? I'm unfamiliar with next15 and if there's more work to be done here, but it'd be great to be React-19-ready.

@javascripter
Copy link
Contributor

javascripter commented Feb 10, 2025

This PR looks surprisingly straightforward to me. (Great work!)

Do we think it's feature-complete? I'm unfamiliar with next15 and if there's more work to be done here, but it'd be great to be React-19-ready.

I believe it's feature-complete. We updated our production website to Next.js v15 + React 19 with this and so far it's been working well. cc @necolas

One caveat is that I had to make findNodeHandle noop instead of removing it entirely like in this PR.
Many of our dependencies still imported it for backward compatibility even though they no longer require it for functioning correctly in React 19 🤷‍♂️. It makes finding bugs harder so the current PR is fine and more correct though.

// our workaround
const findNodeHandle = (component) => {
  throw new Error(
    'findNodeHandle has been removed in React 19. ' +
      'Use the ref property on the component instead.'
  );
};

I suggest trying out this PR and report any issues you encounter to get this merged.

@JulesPatry
Copy link

Looking forward to seeing this be merged in!

@akash3gtm
Copy link

Please let us know if this needs any changes or details/description before you decide to merge.
We're using this internally and are shipping with our library for next15 with react 19.

The person who raised the PR is also a part of our team, hence we will do the changes if required.

@akash3gtm
Copy link

Apart from the linting, prettier changes that @FezVrasta reviewed, Are there any other changes that needs addressing?

@sacru2red
Copy link

ping @necolas
Could you please consider merging this PR?

@Viraj-10
Copy link
Author

@sacru2red, I would suggest you should use patch-package with this PR changes. I don't see these changes getting merge since this will require version upgrade to 0.20 and involves breaking changes and seeing most of meta efforts are toward react-strict-dom react native web will be replaced by it or expo might use it internally to replace it (no official words but from the commits you can see the focus).

@sacru2red
Copy link

ah... I've seen necolas often commit to react-strict-dom, but I've never really paid attention to what project it is. Now I know a little bit what the situation is.

@fbartho
Copy link

fbartho commented Feb 21, 2025

@necolas would you mind sharing your thoughts?

If react-native-web is to be deprecated in favor of react-strict-dom or expo it would be incredibly timely to learn this now rather than later as my startup is just starting to build things out!

(I specifically didn’t go with expo, as I appreciate your approach and the core)

@Viraj-10
Copy link
Author

@fbartho, You can find your answer here.

@fbartho
Copy link

fbartho commented Feb 21, 2025

Thanks @Viraj-10.

Admittedly that discussion leaves me more stuck than expected. Oof.

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.

7 participants