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

updated README (entries/workers) #137

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

mpfau
Copy link
Contributor

@mpfau mpfau commented Mar 2, 2017

No description provided.

@mpfau mpfau mentioned this pull request Mar 2, 2017
README.md Outdated
### Production
In production, `systemjs-hot-reloader` maps to an empty module so you can leave
the `systemjs-hot-reloader` import in your `index.html`.
the `systemjs-hot-reloader` import in your `index.html`. Just make sure, that `connect()` is only invoked, if it is available (`if (connect instanceof Function)`).
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, systemjs-hot-reloader maps to a module exporting a single function so you don't need to check for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that does not work for me. I am building with systemjs-builder and have to manually add the mapping ("systemjs-hot-reloader": "@empty"). And even then, connect is not a function but an empty Symbol...

Am I doing sth. wrong?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theres an entry in the package.json
https://github.com/alexisvincent/systemjs-hot-reloader/blob/master/package.json#L42
which maps it to lib/production.js when production == true. systemjs-builder should respect that.

Also mapping "systemjs-hot-reloader": "@empty" will override my mapping and give you a null option so you have to do what you described in the README

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! But that mapping is only active if you are using jspm. I am using SystemJS without JSPM...

Is there any way with systemjs to apply the jspm mapping?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think actually that systemjs-builder uses the jspm package info. But its possible not.

The alternative is to in your own systemjs config,

{
    "packages": {
        "systemjs-hot-reloader": {
            "map": {
                "./dist/index.js": {
                    "production": "./lib/production.js"
                }
            }
        }
    }
}

Or something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that does not work for me.

However, I removed the function check around connect() as this is definitely not needed for jspm users.

* Using a single `systemjs` configuration (`system.config.js` in this case) for the GUI/main and the worker threads simplyfies the setup.
* The worker is instantiated as usal: `new Worker("WorkerBootstrap.js")`

### Circular dependencies / detection of entry points
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually recently updated systemjs-hmr so that it supports circular dependencies in the roots as well :) Check out the findEntries function.

The entries docs are helpful thanks, but in terms of ordering, I would prefer that the workers and entries docs came after the basic usage (including state reloading)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findEntries (systemjs-hmr@2.0.8) did not work in my case as the entry point is included from multiple other points of the application, too. In my case, the resulting entries list only contains modules that are not imported.

Sure, just change the order to your preferred variant!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpfau The entry points it detects should be sufficient to reimport the full dependency graph. Do you have a reproducible case where it doesn't do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. However, in my case there are hundreds of modules involved. I will try to provide a simple example.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Because if thats the case then theres a bug

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

Successfully merging this pull request may close these issues.

2 participants