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

React 18 support ? #358

Closed
periman2 opened this issue Jul 7, 2022 · 19 comments · Fixed by #399
Closed

React 18 support ? #358

periman2 opened this issue Jul 7, 2022 · 19 comments · Fixed by #399

Comments

@periman2
Copy link

periman2 commented Jul 7, 2022

Greetings! This is an amazing tool! I'd like to ask when is the plan to add support for react 18.

@drcmda
Copy link
Member

drcmda commented Jul 7, 2022

@dbismut @gsimone is it a big change or really just createRoot and that's it?

@dbismut
Copy link
Collaborator

dbismut commented Jul 7, 2022

Yes. I'm not sure how to keep retro compatibility with React 17 though.

@periman2
Copy link
Author

periman2 commented Jul 8, 2022

@dbismut Maybe you could have a build into a @leva/next version or something like that? I imagine withing this year tons of projects will be migrating from 17 to 18. People with older versions of react could just use older versions of leva too.

@dbismut
Copy link
Collaborator

dbismut commented Jul 8, 2022

Just to clarify: Leva is compatible with React 18. It just throws a warning and behaves like React 17, that's all AFAIK. So there's no urgency in making this move.

@periman2
Copy link
Author

periman2 commented Jul 8, 2022

@dbismut That's right, and the application I'm currently using it in doesn't benefit from react 18s features a whole lot. However, it this does make me unable to put leva into other applications I'm working on that do benefit from react 18's features. The way I understand it is that this warning makes the whole app behave like react 17 and not just the parts that use leva. Let me know if I am misunderstanding something here though. It's indeed not urgent, I was just curious if there's a schedule for this change to happen this year or not.

@dbismut
Copy link
Collaborator

dbismut commented Jul 8, 2022

@periman2 no, Leva won't have any impact on the rest of your app. Let me clarify how this works.

We wanted Leva to be as seamless as possible, meaning we wanted the useControls hook to automagically mount the Leva panel without implicitly asking the user to add <Leva /> in its app tree.

This is done at the moment using the ReactDOM.render(element, <Leva />) api from React 17, which is replaced in favor of createRoot(element).render(<Leva />) in React 18. This is what's throwing a warning, but it only affects the Leva panel.

However, Leva also supports passing options to the <Leva /> panel, which requires the user to explicitly render <Leva /> somewhere in the app. In that case, we don't need to run ReactDOM.render, the warning should go away, and <Leva /> will be working as the rest of the app.

In other words, you can do:

function MyAppWithLeva() {
  return (
    <>
      <Leva />
      <App />
    </>
  )
}

The plan is still to move to React 18 at some point, but I don't think it's worth it until we benefit from the transition hooks in order to keep Leva's inputs responsive without blocking the render (a Leva input increasing the number of objects in a 3D scene would probably result in a very laggy experience at the moment). And that requires more work than just replacing the ReactDOM render api.

@periman2
Copy link
Author

periman2 commented Jul 8, 2022

@dbismut Thanks so much for the response and the detailed solution! I will most certainly use it this way then. Best regards!

@controversial
Copy link

Can’t leva release a new major version requiring React 18 (and using the new createRoot API), and leave the previous major version stay for React 17 compatibility? That’s my understanding of how these things are usually handled. Alternatively, there might be a way to detect React version and choose which API to use at runtime.

@dbismut dbismut reopened this Jul 30, 2022
@dbismut
Copy link
Collaborator

dbismut commented Jul 30, 2022

@controversial I'm reopening the issue for more visibility. But again, what is the problem with having a warning in the console that has zero impact on your app whatsoever? I've already made my point on React 18 above and once again, we won't release a new major version which only point would be to suppress a harmless warning!

@drcmda
Copy link
Member

drcmda commented Jul 30, 2022

these warnings do annoy me, a little :-D im sunk in debugging half the day and im a console guy, always having to scroll up every time to see my logs ... i've also had clients complain about "crashes in the logs", it's not the best impression at least.

imo a major updating to react, leaving the previous one reserved for react 16 and 17 could be reasonable, and peerDependencies will inform on install. or a release tag @ stable (18) and @ pre18 ? changing render to createroot and deprecating render like that was a mean decision by react :/

@dbismut
Copy link
Collaborator

dbismut commented Jul 30, 2022

I think this warning only happens in dev mode, and only at mount.

I'll end up doing this if you insist.

const log = console.log
console.log = () => {}
render()
console.log = log

More seriously though: there's an open PR for React 18 but I don't want to merge it until we make good use of startTransition / useDeferredValue.

@drcmda
Copy link
Member

drcmda commented Jul 30, 2022

ah yes, i've experimented with these for a while. that would indeed make it worthwhile.

@apecollector
Copy link

There's an open PR for this: #330

@CodyJasonBennett
Copy link
Member

This isn't harmless to robots and can block CI, so I have a PR with a work-around in #399 until a depreciation is necessary.

@geyang
Copy link

geyang commented Dec 19, 2023

I want to bump this up. Having a new version released would be nice.

@DennisSmolek
Copy link
Contributor

I want to bump this up. Having a new version released would be nice.

This was fixed in #399 and works fine in React 18, it was just using a older method to maintain backwards compatibility which caused it to trigger a warning in the console. The warning is no longer be triggered.

@GabrielPedroza
Copy link

I get this error when importing and attempting to use useControls in my Next.js 14 with React 18 project:

Error: (0 , react__WEBPACK_IMPORTED_MODULE_1__.createContext) is not a function

Is there any support for this or am I just missing something else?

@CodyJasonBennett
Copy link
Member

No, you should mark that as 'use client' and report upstream that they're breaking basic React API by altering the bundle (compiler error) instead of exporting stubs (no-op or runtime warning/error).

@GabrielPedroza
Copy link

No, you should mark that as 'use client' and report upstream that they're breaking basic React API by altering the bundle (compiler error) instead of exporting stubs (no-op or runtime warning/error).

Awesome it works! Thank you so much!

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 a pull request may close this issue.

9 participants