elmish / hmr

Hot Module Replacement for Elmish apps
https://elmish.github.io/hmr
Other
28 stars 9 forks source link

HMR crashes the app with Fable.Core 4.3 or/and Fable 4.10.0 and above #44

Closed laurentpayot closed 7 months ago

laurentpayot commented 7 months ago

Description

HMR crashes the app with Fable.Core 4.3 or/and Fable 4.10.0 or 4.11.0. Downgrading to Fable.Core 4.2 and Fable 4.9.0 makes HMR work again.

Repro code

I encountered this bug when converting a non-trivial Elm app to F# Elmish. I could reproduce it exactly with https://github.com/laurentpayot/fsharp-fable-elmish-example. Inside this repo just do dotnet paket update or dotnet tool update fable and the bug will appear.

Related information

MangelMaxime commented 7 months ago

Hello @laurentpayot,

Could you please make a smaller reproduction example? As this one seems to have a lot of stuff and also I can't seems to run it because it trying to access my Photo gallery and I don't know why it wants.

Something that I saw while looking thought your code is that you seems to be using Preact and not react. So this is perhaps why in my project HMR is working with latest version of Fable and not in yours.

CleanShot 2024-01-31 at 14 19 26

laurentpayot commented 7 months ago

Oh yes sorry, I forgot to mention I tried with React instead of Preact and got the same bug too. I will create a branch with React for you.

laurentpayot commented 7 months ago

The React branch is there: https://github.com/laurentpayot/fsharp-fable-elmish-example/tree/react

Btw, the Photo gallery access is a joke, right? I hope so because it’s just safe FOSS without sketchy dependencies :sweat_smile:

laurentpayot commented 7 months ago

One more precision: the bug happens both with Fable.React (example repo) and Feliz (my non-trivial app).

MangelMaxime commented 7 months ago

Btw, the Photo gallery access is a joke, right? I hope so because it’s just safe FOSS without sketchy dependencies 😅

I don't know, I just know that Mac didn't gave it permission to access this folder 🤷‍♂️ 🤣

MangelMaxime commented 7 months ago

Moving the paket.references to the "correct place" seems to fix the issue.

paket.references should leave near the fsproj file and I also only included the client dependencies for the client project:

Fable.Core
Fable.Browser.Dom
Fable.Elmish
Fable.Elmish.Browser
Fable.Elmish.Debugger
Fable.Elmish.HMR
Fable.Elmish.React
Fable.React
Thoth.Json
laurentpayot commented 7 months ago

Thank you so much to have taken the time to find this. I duplicated paket.refences in the src and the test project (removing unneeded deps in each) and the bug seems to be fixed for Fable.Core 4.3 with Fable 4.9.0. But the bug is back with Fable.Core 4.3 after upgrading Fable to 4.11.0 :roll_eyes:

MangelMaxime commented 7 months ago

By "seems to fix the issue" do you mean the HMR bug is fixed or is it simply that you can run the example?

I meant fix the issue regarding the path/ me running the code.

I am looking into the HMR issue, and I feel like when you say that this is broken this is because you reach this bug https://github.com/fable-compiler/Fable/issues/3631

One way to solve it that I found is to use --verbose on the Fable command. This makes it so it doesn't rewrite console but just keep appending to it.

I send you a PR to test out https://github.com/laurentpayot/fsharp-fable-elmish-example/pull/2

If it works for you, you can close this issue as this is not related to Elmish.HMR but a bug in Fable

laurentpayot commented 7 months ago

I found out that, while keeping a single paket config at the root, moving down Fable.Elmish.HMR after the other app dependencies in paket.references fixes the bug for Fable 4.9.0. But with Fable 4.10.0 and above the bug comes back.

You released the working Fable 4.9.0 on December the 14th 2023, so I don’t think that this bug is related to the freezing bug that was open on November the 30th...

As this bug depends on the order of the dependencies, could the culprit be the side effects HMR relies on? (it has to be the last of the Elmish modules to be opened) :thinking:

laurentpayot commented 7 months ago

I have to go but I will try the verbose logs tomorrow…

MangelMaxime commented 7 months ago

Unless you can make a reproduction with a paket.references which his along side the fsproj we will probably not do something.

From my test, when using paket the expected way the problem related to HMR is because the freezing issue.

On another point, in F# file order matter so you should not do <Compile Include="Shared/**/*.fs" /> but write a Compile node for each F# files you want to compile.

Indeed, right now it only works the alphabetical order is the correct order but it is not going to work when the scale increase.

laurentpayot commented 7 months ago

I was going crazy with this bug, I tried everything. Then I noticed that HMR works perfectly with latest Fable modules if you do not use the --run Fable option but instead launch Vite in another terminal window after the Fable compilation is done. So I rewrote my start script to launch Vite in parallel with the shell operator & instead of the Fable option --run:

pnpm vite & dotnet fable watch --noCache --silent --cwd src --outDir ../output --sourceMaps

I also set server.hmr.overlay to false in vite.config.js to avoid an error message in the browser when Fable is compiling for the first time.

Now HMR works like a charm. So it wasn’t a problem with paket modules order or files location. The --run option has an issue, at least on my Ubuntu machine. HMR crashed with --runFast too by the way.

I’m closing this issue as for me it is not related to HMR but rather to Fable --run options.

PS: I knew for file order in fsproject, simply enforcing a specific order was not needed in this trivial example. But you’re right I will do it anyway for good practice reasons. Thanks!

laurentpayot commented 7 months ago

PPS: I should have read https://github.com/fable-compiler/Fable/issues/3631 before! 🤦‍♂️ You are right, it is the same bug.