Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HMR is not working for web workers #14722

Open
wclr opened this issue Nov 12, 2021 · 14 comments
Open

HMR is not working for web workers #14722

wclr opened this issue Nov 12, 2021 · 14 comments

Comments

@wclr
Copy link

wclr commented Nov 12, 2021

Bug report

What is the current behavior?

When updating the code loaded by web worker nothing happens.

If the current behavior is a bug, please provide the steps to reproduce.

This is a very simple configuration where HMR doesn't work:
https://github.com/wclr/webpack-worker-hot-reload

What is the expected behavior?

I believe it should work the following way:

  1. If the worker's entry script accepts hot reload, hot reload should be executed in the context of running web worker(s).
  2. Otherwise it should be upstreamed to the parent context (probably parent script where new URL of the worker script was created).

Other relevant information:
webpack version: 5
Node.js version: 14
Operating System:
Additional tools:

@alexander-akait
Copy link
Member

Yep, the same webpack/webpack-dev-server#3794

@vankop
Copy link
Member

vankop commented Feb 2, 2022

@alexander-akait should we track issue in webpack repo?

@alexander-akait
Copy link
Member

No, it should be implemented on webpack-dev-server side

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@donalffons
Copy link
Contributor

I'm also having a use-case where I would like to use hot reloading in a worker thread. I'm using a custom react renderer in a worker to perform heavy computations. Reloading the page every time in development is becoming a bit of a deal breaker for that use case. I have spent a lot of time researching this topic and it looks like this is not supported in Webpack?!

@alexander-akait (hope you don't mind me asking directly), do you have any tips / hints if / how this could be done? I would be happy to create a minimal reproducer (using a dummy react renderer in a worker?!), if you're interested.

@alexander-akait
Copy link
Member

@donalffons yes, it will be useful, I will investigate it deeply

@donalffons
Copy link
Contributor

I greatly appreciate that! Thanks for your work on this!

Fwiw, I have found this PR for vite, which aims to implement HMR for workers. This does seem to work and provides the module.hot api in a worker, which lets me accept and react to module changes. However, it does not work in combination with react (i.e. as soon as I start using JSX syntax) - because their React tooling relies on main-thread properties like window etc. to be available.

For my specific use case & webpack, I wonder if it would be possible to

  • create multiple entry-points for the main thread and the worker,
  • then "manually" start the worker using the worker entrypoint (i.e. w/o webpack's interference)
  • then using some sort of modified version of @pmmmwh/react-refresh-webpack-plugin, to enable react-refresh on my worker.

@alexander-akait, does this sound like a reasonable approach or am I missing something?

@alexander-akait
Copy link
Member

then "manually" start the worker using the worker entrypoint (i.e. w/o webpack's interference)

In theory you can multi compiler mode, so you don't need to manually start, also you can set dependecies so worker will be built first

But yes, approach sounds good

@donalffons
Copy link
Contributor

donalffons commented May 17, 2022

@alexander-akait Just wanted to quickly thank you (your suggestion to use multi compiler mode was very helpful) and share my learnings here.

I was able to make HMR and react-refresh work nicely with my custom React renderer (at least I haven't noticed any issues yet 😉). The setup is fairly simple and updates work as expected and preserve the application state. I did not have to make any modifications to webpack or @pmmmwh/react-refresh-webpack-plugin (they don't rely on window, as it is the case with vite). Here's what I did:

// ...
  entry: {
-    main: './src/index.tsx',
+    worker: './src/worker/index.tsx',
  },
+  target: "webworker",
  plugins: [
    isDevelopment && new ReactRefreshPlugin(),
    new ForkTsCheckerWebpackPlugin(),
-    new HtmlWebpackPlugin({
-      filename: './index.html',
-      template: './public/index.html',
-    }),
  ].filter(Boolean),
// ...
import webpack from 'webpack';
import config from './webpack.config.js';
import configWorker from './webpack.config.worker.js';
import WebpackDevServer from 'webpack-dev-server';

const compiler = webpack([configWorker, config]);

const devServer = new WebpackDevServer({}, compiler);
devServer.start();
  • Start worker from main thread:
const w = new Worker("/worker.js")

After that, I can module.hot.accept HMR updates from worker/index.tsx. If I don't accept these changes, there will be an error about window being undefined, when webpack tries to window.location.reload(). Most importantly for me, though: React-refresh updates in React components imported from worker/index.tsx just work without any further modifications (...so far, at least 😉).

(This was much less painful than I anticipated 🙂)

@alexander-akait
Copy link
Member

@donalffons

If I don't accept these changes, there will be an error about window being undefined, when webpack tries to window.location.reload().

hm, sounds like a bug in webpack-dev-server too, dev server assumes we have window's context and try to reload, but it is wrong 😕 can you provide small example with original configuration too, I would like to to look

@donalffons
Copy link
Contributor

donalffons commented May 23, 2022

Here is a simple example that reproduces the error I had: ocjsx-hello-world-webpack-issue.zip @alexander-akait

To reproduce:

  1. npm i && npm run start, visit localhost:3000
  2. change src/worker/Models.tsx, e.g.
    -  <bool op="cut">
    +  <bool op="common">
  3. notice how the scene updates
  4. change src/worker/index.tsx
    + console.log("Hello, World!");
  5. notice the console error
    Uncaught (in promise) ReferenceError: window is not defined
    

I'm ~80% happy with this proof of concept. What's missing for me would be the possibility to ship the custom react renderer (i.e. ~ src/worker/index.tsx) as an npm package that can be used with user-defined models (src/worker/Model.tsx). Currently, /src/worker/index.tsx statically imports src/worker/Model.tsx, which wouldn't work in that case. I'm not sure what would be the best way of doing that - maybe by writing some sort of Webpack Plugin?

@alexander-akait
Copy link
Member

Here is a simple example that reproduces the error I had: ocjsx-hello-world-webpack-issue.zip @alexander-akait

Yep, bug, we should fix it, if you want to help feel free to send a PR

I'm not sure what would be the best way of doing that - maybe by writing some sort of Webpack Plugin?

Yep, a good question, ideally you need to pass path to worker as argument (you will faced with the same problem not only for webpack, for any other bundlers), multi compiler mode can be useful here too

@donalffons
Copy link
Contributor

if you want to help feel free to send a PR

Will do!

@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants