search

purescript-react / purescript-react-basic-dom

https://pursuit.purescript.org/packages/purescript-react-basic-dom
Apache License 2.0
11 stars 18 forks source link

createRoot #42

Closed andys8 closed 2 years ago

andys8 commented 2 years ago

I'm trying to figure out details about createRoot.

It is the new default function to initialize a react app (v18).

https://github.com/lumihq/purescript-react-basic-dom/blob/53dfc605a1dd91dbc12df160b445156e6a6626af/src/React/Basic/DOM/Concurrent.js#L2-L8

createRoot functionality

Is this createRoot the same function as the ones referred to in the docs imported from react-dom/client? In any case a more prominent export of the function (similar to render) could be helpful.

createRoot name conflict issue

And Concurrent.js might currently be flawed. The names createRoot and createBlockingRoot are both defines as constants and then in functions with the same names are created. This will result in a SyntaxError.

image

PS: I remember I created another createRoot related issue somewhere in the past in a different repository. Not sure anymore where :thinking: Sorry for duplication

mjrussell commented 2 years ago

Hi @andys8, this library has not yet been updated to React 18 (you can see that in the npm deps here - https://github.com/lumihq/purescript-react-basic-dom/blob/main/package.json#L17-L18). We plan to start working on releasing a supported version shortly but I would expect 5.x to only work with React 17

andys8 commented 2 years ago

Neat :)

React Update: Is there a branch with progress or early stages? I could maybe offer some help.

Issue: If I'm not mistaken the name conflict is part of the current version and will also not work if people use the Concurrent module with react v17.

mjrussell commented 2 years ago

Not yet, but any help would absolutely be appreciated if you want to start a PR and work there. We just finished spending some time cleaning up the various react-basic libraries and getting them to purescript 15. I also hope to add some tests, even if basic things to get a little better coverage.

Thanks for the note about the concurrent issue, I'm not super familiar with that API so I'll have to look into that more.

mjrussell commented 2 years ago

Can this issue be closed now? Or is there any follow up for createRoot?

andys8 commented 2 years ago

Done.