search

alexisvincent / systemjs-hot-reloader

reloads your modules as needed so that you can have satisfyingly fast feedback loop when developing your app
MIT License
228 stars 36 forks source link

No WebWorker support #132

Open mpfau opened 7 years ago

mpfau commented 7 years ago

There are three window and two document references spread through systemjs-hmr and systemjs-hot-reloader:

However, even after removing them, hot-reloading from a worker does not work.

The update event seems to be triggered correctly and systemjs-hot-reloader logs:

reloading src/updatedModule.js
reloading cache.json

However, the updated module is not re-evaluated (break points are not hidden/console.log statements are not printed).

Unfortunately, I couldn't find the source of the problem... :(

mpfau commented 7 years ago

Just digged a bit deeper in it. The base functions (System.unload and System.import) are also functional in the worker. Modules are successfully re-imported when manually triggering System.unload and System.imports in a worker context, too.

However, when system-hmr is not able to detect the right entries. That means that a changed module gets unloaded by system-hmr but not imported again, as its module is not a dependency of the entries that are identified by system-hmr.

@alexisvincent Is there a way to provide the entries to systemjs-hot-reloader (https://github.com/alexisvincent/systemjs-hmr/blob/master/lib/index.js#L223)?

alexisvincent commented 7 years ago

yeah, System.reload('someModule', {entries: ["entry"]}). Will give a better explanation when I'm not on my phone

alexisvincent commented 7 years ago

@mpfau Having a look now,

The following might not be necessary.

This is legacy and only there for backwards compatibility.

These are needed if we want full page reload and automatic host detection.

Is what you're wanting to achieve here being able to hot reload web worker scripts. Or run the hot-reloader in a web worker?

As I said before, you can manually provide entries, but then no entry discovery will happen. This could change in the future if it's more useful to merge entries.

Manually specifying entries is achieved as follows

System.reload('someModule', {entries: ["entry"]})
mpfau commented 7 years ago

@alexisvincent full page reload is not possible from a worker anyways. Could be wrapped in a if (typeof window !== 'undefined').

I'm currently trying to run the hot-reloader in a webworker. However, I think that both approaches (reload the worker script if a module loaded by that script is modified (1) / updating a script in the webworker on module change (2)) are valid depending on your use-case.

Manually specifying the entries only works when systemjs-hmr is used directly. What do you think about an entries option for systemjs-hot-reloader?

alexisvincent commented 7 years ago

@mpfau Yeah could totally add entries as an option to the connect function.

in terms of window detection, this could probably be generalized to an environment detection. Eg. web, web-worker, node etc. Then we can do the following:

if (isNode) {
    ...
} else if (isWorker) {
    ...
}
alexisvincent commented 7 years ago

@mpfau Have pushed entries option in connect function. See if that works for you.

mpfau commented 7 years ago

@alexisvincent thanks! This is working fine and enables an absolutely awesome workflow :)

I created a the pull requests https://github.com/alexisvincent/systemjs-hmr/pull/23 and https://github.com/alexisvincent/systemjs-hot-reloader/issues/132 in order to enable systemjs-hmr and systemjs-hot-reloader for worker usage.

alexisvincent commented 7 years ago

@mpfau are there any real benefits to running the hot-reloader in a web worker by default?

alexisvincent commented 7 years ago

@mpfau Can you give me a short description of what the current codebase can do with regards to web workers. Would like to add something to the README

mpfau commented 7 years ago

I am working on a description. Give me a few more minutes... :)

mpfau commented 7 years ago

137