Zaid-Ajaj / Feliz

A fresh retake of the React API in Fable and a collection of high-quality components to build React applications in F#, optimized for happiness
https://zaid-ajaj.github.io/Feliz/
MIT License
534 stars 78 forks source link

Feliz.UseElmish doesn't work correctly with React.strictMode #550

Closed sydsutton closed 1 year ago

sydsutton commented 1 year ago

To re-create the issue that I was having, I created a SAFE stack app, updated all Feliz/Fable deps, updated to Fable 4, and React 17 -> 18.

This is the repo, in case you want to fork it to see what I'm talking about here: https://github.com/sydsutton/UpdateTest

dotnet tool restore (make sure fable is 4.0.0-theta-018) dotnet run

When opening the app, normally the list of pre-populated ToDos would show on the screen ("Create new SAFE project", etc.). It seems like the GotTodos msg is not being updated in the update function, so the ToDos are not being shown. In the Network tab of devtools (see attached image), I can see that the ToDos are being "got", they're just not being updated. The update function does respond to changing user input and adding a new ToDo, though.

This is, essentially, the same issue I'm having in another project after updating Fable, Feliz, and React-- the data isn't being retrieved, and therefore, the state is constantly "Loading".

Please let me know if I missed a dependency, or if I need to change something else in the migration process.

Thanks! :) image

MangelMaxime commented 1 year ago

The linked repository doesn't exist or is not public.

sydsutton commented 1 year ago

@MangelMaxime Sorry about that, I just changed it to public. Try again.

Zaid-Ajaj commented 1 year ago

Hi @sydsutton thanks for filing the issue! I've tried the application locally and reproduced the problem as you describe it. It looks like React.strictMode is the culprit. The following fixes the client:

let root = ReactDOM.createRoot (document.getElementById "elmish-app")
- root.render(React.strictMode [Index.View()])
+ root.render(Index.View())

I don't exactly understand why <StrictMode /> is causing an issue though. The implementation React.strictMode is correct but maybe because Feliz.UseElmish uses a useExternalStore 🤔 I will have to look into it.

sydsutton commented 1 year ago

@Zaid-Ajaj you're a gentleman and a scholar. Thanks for looking into it!

sydsutton commented 1 year ago

@Zaid-Ajaj Do you think the issue with React.strictMode could have to do with mounting/unmounting? https://reactjs.org/docs/strict-mode.html#ensuring-reusable-state

"To help surface these issues, React 18 introduces a new development-only check to Strict Mode. This new check will automatically unmount and remount every component, whenever a component mounts for the first time, restoring the previous state on the second mount."

Zaid-Ajaj commented 1 year ago

@sydsutton that is very possible but I am not entirely sure how to fix it because we work with useExternalStore, basically opting out of React's state management to implement our own Elmish dispatch loop.

sydsutton commented 1 year ago

After some experimenting, I found that if I remove the line cmd <- Cmd.none in the mapInit function in UseElmish, the command works as intended. It was changed here: https://github.com/Zaid-Ajaj/Feliz/commit/4c0dc24face05b77601338edf04fce6bb175e6af , and it has the comment // Don't run the original commands after hot reload. This one line of codes seems to be killing in-flight commands when using React.strictMode. Is there a different way to write this code that might not effect in-flight commands? Thanks! @alfonsogarciacaro

Zaid-Ajaj commented 1 year ago

Should be fixed as of Feliz.UseElmish v2.4.0