Gatsby: [pluign-preact]: changes to hmr/development

[moonmeister]

I have two concerns, I'm happy to open a PR if these are real, but maybe I'm just confused on this implementation hoping @teodragovic or whomever can help.

1. console.log('[HMR] disabled: preact is not compatible with RHL')

Q: I'm taking this to imply that preact is not compatible with react-hot-loader. But I'm actively attempting to enable the experimental react-refresh and this is coming up. Have I failed at enabling react-refresh or is this log incorrect and I am indeed using it but preact isn't compatible with react-refresh?

A: From what I can tell is this should only be logging if I've enable react-refresh. and it should read:
[HMR] disabled: preact is not compatible with react-refresh

1. Proper gating

Q: It seems this code use to be gated to not run in NODE_ENV=development...those gates were appropriately removed...but should they not have been replaced by GATSBY_HOT_LOADER=fast-refresh?

Q: preact/debug should only be included if GATSBY_HOT_LOADER=fast-refresh, yes?

1. IMHO: maybe we can make this opt-in in-case devs would perfer HMR over preact in development?

[teodragovic]

I'll try to add some context since question is related to #22441 where @wardpeet and I went back and forth a bit and switched then reverted implementation.

1. Technically, there is no current support for fast-refresh nor RHL when using Preact with Gatsby. fast-refresh is not compatible with Preact. reach-hot-loader can be used with Preact, but inside Gatsby it would require significant changes to core package (I think) so basically no support for both :(

Maybe we can change that log to something like [HMR] disabled: hot module reloading not supported when using Gastby with Preact?

1. My initial proposal was to straight up disable HMR by forcing fast-refresh so Preact is loaded in development without conflicts. I revisited this by refactoring PR to make it opt-in by manually passing GATSBY_HOT_LOADER=fast-refresh. That change was ultimately reverted so plugin now works in development by default and HMR is disabled.

Final code change: https://github.com/gatsbyjs/gatsby/pull/22441/files

Personally, I'm ok with either approach.

[smashercosmo]

Can we at least enable vanilla webpack HMR? Because currently HMR is disabled completely and I have to reload browser after every change.

[teodragovic]

That's a good idea. Just tried it and this seems to work:

// gastby-node.js
const webpack = require('webpack');
const path = require('path');

exports.onCreateWebpackConfig = () => {
  actions.setWebpackConfig({
    devServer: {
      contentBase: path.join(__dirname, 'src'),
      watchContentBase: true,
      hot: true,
    },
    plugins: [
      new webpack.HotModuleReplacementPlugin()
    ],
  })
}

update: tested with [email protected]

[moonmeister]

If the default HMR works with preact that's great. If we want to allow people to use react-hot-loader with react we could put flag in the plugin config but I think attaching it to fast-refresh is just confusing cause it goes from seemingly working to not when you're attempting to make it faster. Thanks for all the input.

[smashercosmo]

@teodragovic solution didn't work for me. I got

[HMR] bundle rebuilt in 1138ms
VM8 commons.js:34522 [HMR] Checking for updates on the server...
VM8 commons.js:34371 [HMR] bundle rebuilt in 1138ms
VM8 commons.js:1004 Uncaught RangeError: Maximum call stack size exceeded
    at hotAddUpdateChunk (VM8 commons.js:1004)
    at webpackHotUpdateCallback (VM8 commons.js:8)
    at webpackHotUpdateCallback (VM8 commons.js:9)
    at webpackHotUpdateCallback (VM8 commons.js:9)
    at webpackHotUpdateCallback (VM8 commons.js:9)
    at webpackHotUpdateCallback (VM8 commons.js:9)

on rebuild

[blainekasten]

Some good news, Preact is currently working on support for FastRefresh: https://github.com/JoviDeCroock/preact-refresh

I know that doesn't really solve things for right now. However, there is a lot of movement in the fast refresh space right now and I believe it'll all stabilize in the next few months. There are a few of us that are cross collaborating right now to make fast refresh stable and work for the broad ecosystem.

Since Preact is aiming to support FastRefresh, I strongly believe we'll entirely drop RHL in gatsby v3.

So again, this isn't great news because I'm offering no solution for right now. But I know it's not going to be realistic for us to prioritize work on RHL right now as we are working on a lot of other features _and_ fast refresh is the soon to be realized future.

I'm going to close this issue for now. Feel free to keep the conversation going, but the answer for now is we're not going to put more effort into RHL.

[smashercosmo]

@blainekasten No need to put any efforts in RHL, but would be nice to make vanilla webpack HMR work.

[blainekasten]

@smashercosmo if you want to take a look at how to do it and add a PR i'd be happy to support you through it. I currently have a lot of big features on my plate (and most of the team does), so I don't think we'd be able to prioritize that right now. Let me know if you're interested

[luizcieslak]

I tried @teodragovic solution to HMR and have the same error that @smashercosmo had. It seems that HMR plugin is being added twice (https://github.com/webpack/webpack/issues/7380#issuecomment-391633727).

I haven't been able to come up with a valid configuration though and disabled gatsby-plugin-preact for now.

If anyone could make HMR work with preact until it supports fast-refresh, please post it here.