fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.9k stars 296 forks source link

Non-bundling HMR and Fable watch dependencies #2617

Closed alfonsogarciacaro closed 2 years ago

alfonsogarciacaro commented 2 years ago

I'm working recently with Vite and Fable.Lit. Component-based HMR is working fine but I'm having issues with Fable watcher. Fable builds a watch dependency tree to know which files needs to recompile on changes. Example: FileB calls a class member in FileA, because class members are mangled by Fable, we need to recompile FileB if FileA changes in case the imports have changed). But if I touch a component in FileA and FileB contains another component that calls the one in FileA, even if HMR correctly works with FileA at the the component and its state are reset when HMR for FileB kicks in. I hope the explanation is clear šŸ˜…

For now I can think of a couple of tentative solutions for this, but I'm not entirely convinced of either:

What do you think? Can you think of an alternative solution?

cc @MangelMaxime @Zaid-Ajaj

MangelMaxime commented 2 years ago

Add a --watchDeps flag to the compiler. True by default but when set to false Fable won't recompile watch dependencies. I tried and works, but I don't like that users need to remember to set it and can put the compilation in a bad state.

I don't like this idea. I suspect that as long as you don't use any of the mangled methods it will works but if your code is using them, then it will try to call a method that doesn't exist any more because Fable partially compiled the files.

This means that the user will have to manually trigger the compilation to the right files or kill Fable and start it again to be sure that the generated files are correct.

Please don't do that.

The drawbacks is users need to remember to "deactivate" it when working with another file.

I don't understand that part. Do you mean that the user would have specific code during the development and need to delete that code before generating a production ready bundle?


What you are having as a problem right now, is known for years in JavaScript world. When you want to use HMR in JavaScript you are forced to write code in a specific way. This is the problem they are trying to fix with react/fast-refresh which required you to write your code in a more specific way...

In Fable/Elmish world, we have been saved from this because pure Elmish application has the state of the application in a single place and Elmish.HMR is doing all the hard work for you.

It would be better to see if you can do something similar to Elmish.HMR which when HMR is detected use specific codes directly at the code level.

It would be much better if Fable.Lit could detect HMR or based on a compilation flag decide to use specific code to activate HMR support for free for the end user.

Elmish.HMR works like that, the only thing required for it are:

  1. Having the DEBUG directives
  2. Having open Elmish.HMR opened after all the others functions

Elmish.HMR is not perfect because of the second point. It would be better if HMR support was build directly into Elmish.Browser, Elmish.React, etc. but this is not the case for different reason.

Can't you add the HMR logic directly in Fable.Lit html function and [<WebComponent>] decorated code?

alfonsogarciacaro commented 2 years ago

Can't you add the HMR logic directly in Fable.Lit html function and [] decorated code?

Yes, this should be already solved with Hook.useHmr. My explanation was not very good but the issue was HMR shouldn't be triggered by files that are being updated not because they've changed but because some dependency has changed. In any case. I think I managed to fix it in a transparent way to the user. I've added the triggeredByDependency flag to Fable.Core. This way the code can know whether the file is being recompiled because it's been changed directly or not. Then it can take decisions upon that, in this case whether HMR should be applied or not.

Hopefully this will fix the issue in 3.6, we can reopen if needed šŸ‘