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

No WebWorker support #132

Open
mpfau opened this issue Mar 1, 2017 · 11 comments
Open

No WebWorker support #132

mpfau opened this issue Mar 1, 2017 · 11 comments

Comments

@mpfau
Copy link
Contributor

mpfau commented Mar 1, 2017

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
Copy link
Contributor Author

mpfau commented Mar 1, 2017

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
Copy link
Owner

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

@alexisvincent
Copy link
Owner

@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
Copy link
Contributor Author

mpfau commented Mar 1, 2017

@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
Copy link
Owner

@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
Copy link
Owner

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

@mpfau
Copy link
Contributor Author

mpfau commented Mar 2, 2017

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

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

@alexisvincent
Copy link
Owner

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

@alexisvincent
Copy link
Owner

@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
Copy link
Contributor Author

mpfau commented Mar 2, 2017

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

@mpfau
Copy link
Contributor Author

mpfau commented Mar 2, 2017

#137

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

No branches or pull requests

2 participants