Feature/hmr parcel webpack vite

MangelMaxime commented 2 years ago

Status:

Webpack :heavy_check_mark:
Parcel :heavy_check_mark:
Vite ❌
Snowpack ❌

@alfonsogarciacaro I don't understand why Vite HMR is not triggering.

Webpack

Webpack is supported via import.meta.webpackHot and module.hot if the first one is not defined.

Parcel

Webpack is supported via module.hot

Vite

Vite should be supported via import.meta.hot and I do access it correctly when log it's value but the HMR is not working. It is always triggering a full reload.

I know that the current way I use to access the hot module is not standard:

[<Emit("import.meta.hot ? import.meta.hot : (import.meta.webpackHot ? import.meta.webpackHot : module.hot)")>]
let hot : IHot = jsNative

But even, when I tried to just write code specific to Vite to follow the "Required Conditional Guard".

[<Emit("import.meta.hot")>]
let hot : IHot = jsNative

Also, in our case we don't really need to add the guard because when bundling for production we don't generate HMR code thanks to compiler directives.

I was not able to make it works. Do I need to do something special when starting Vite?

Snowpack

I can't seems to configure Snowpack correctly... I always get an error like that:

[23:39:08] [snowpack] Package "react-native" not found. Have you installed it?

According to the documentation this happen when you don't import package using ./ or / in the path but I don't have any reference to react-native in my code so I don't know why it is asking for it.

If we don't have Snowpack working, this is not so much of a problem to me as the way Snowpack is working should be similar to Vite.

I didn't finish cleaning up the repo architecture but to run the tests project you can do:

npm install
npm run tests:watch

This is going to start Fable and all the bundlers at the same time.
- Webpack on http://localhost:3000
- Parcel on http://localhost:3001
- Vite on http://localhost:3002

Than you can edit the App.fs file line 96 and see that:

The timer is not reset
That the display is stable, by that I mean that if we didn't dispose correctly of the previous App, you will have a strange behaviour where both instance of the application are flickering because they both render to the same div.

alfonsogarciacaro commented 2 years ago

It's not documented but it seems Vite parses the file and looks for the if (import.meta.hot) pattern (see this code for example), that's why they require it and why I had to put the code to deal with Vite and Webpack in two different branches. In fact, at one point also the HMR bindings were not working for me because Fable was adding parentheses, so I had to make sure import.meta.XXX expressions were called without parentheses.

MangelMaxime commented 2 years ago

Hum, I am ranting but there choice is just ...

Who though it would be a good idea that the code will depends on how it is written and not how it executing...

The key point to make Vite works seems to be that:

In fact, at one point also the HMR bindings were not working for me because Fable was adding parentheses

For Vite (import.meta.hot).dispose is not the same as import.meta.hot.dispose.

In JavaScript (at execution) they are the same. It reminds me of React-Refresh etc. which ask you to write the code in a specific way... People complained in the past that they had to write code in a specific way for HMR to works, but they manage to make it even more specific 🤦

Thank you for your input, I will have to duplicate code like you did to make it works.

MangelMaxime commented 2 years ago

I now have Webpack, Vite and Parcel working with a not too much code duplication

Accept HMR

// Vite needs to be the first HMR tested
// because Vite is pretty stricit about how HMR is detected
if HMR.Vite.active then
    updateElmish_HMR_Count ()
    HMR.Vite.hot.accept()
    Internal.tryRestoreState hmrState HMR.Vite.hot.data

else if HMR.Webpack.active then
    updateElmish_HMR_Count ()
    HMR.Webpack.hot.accept()
    Internal.tryRestoreState hmrState HMR.Webpack.hot.data

else if HMR.Parcel.active then
    updateElmish_HMR_Count ()
    HMR.Parcel.hot.accept()
    Internal.tryRestoreState hmrState HMR.Parcel.hot.data

Stop the old program

// Vite needs to be the first HMR tested
// because Vite is pretty stricit about how HMR is detected
if HMR.Vite.active then
    HMR.Vite.hot.dispose(fun data ->
        Internal.saveState data !hmrState

        dispatch Stop
    )

else if HMR.Webpack.active then
    HMR.Webpack.hot.dispose(fun data ->
        Internal.saveState data !hmrState

        dispatch Stop
    )

else if HMR.Parcel.active then
    HMR.Parcel.hot.dispose(fun data ->
        Internal.saveState data !hmrState

        dispatch Stop
    )

Browser Navigation listener handling

// Vite needs to be the first HMR tested
// because Vite is pretty stricit about how HMR is detected
if HMR.Vite.active then
    HMR.Vite.hot.dispose(fun _ ->
        Navigation.Program.Internal.unsubscribe ()
    )

else if HMR.Webpack.active then
    HMR.Webpack.hot.dispose(fun _ ->
        Navigation.Program.Internal.unsubscribe ()
    )

else if HMR.Parcel.active then
    HMR.Parcel.hot.dispose(fun _ ->
        Navigation.Program.Internal.unsubscribe ()
    )

LazyView component

override this.shouldComponentUpdate(nextProps, _nextState) =
    // Vite needs to be the first HMR tested
    // because Vite is pretty stricit about how HMR is detected
    if HMR.Vite.active then
        this.shouldComponentUpdateHMR nextProps

    else if HMR.Webpack.active then
        this.shouldComponentUpdateHMR nextProps

    else if HMR.Parcel.active then
        this.shouldComponentUpdateHMR nextProps

    else
        not <| this.props.equal this.props.model nextProps.model

I am still unable to configure Snowpack because it is asking about react-native but hopefully it use the same API as one of this 3 bundlers.

I will do a pause before re-testing this PR and if everything is good, I will publish a new version.

elmish / hmr