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

Html reload #54

Merged
merged 3 commits into from
Feb 16, 2016
Merged

Html reload #54

merged 3 commits into from
Feb 16, 2016

Conversation

JordanBelford
Copy link

I implemented basic html reloading that is working well for my project. Load times went from 10-15 seconds to about 3 seconds.

It uses System.loads to find the module that imported the html file, and then hot reloads that parent module.

I welcome any feedback about the implementation, as I am not very familiar with this project or SystemJS's internals.

One thing to note, if there are multiple 'importers' of the html file, it should probably hot reload all of them. Currently this is only reloading the first one.

I am using browsersync, which injects css files, so combined with this project, hot reloading html, css, and js are all working.
#50

@JordanBelford JordanBelford deleted the html-reload-merge branch February 13, 2016 00:47
@JordanBelford JordanBelford restored the html-reload-merge branch February 13, 2016 00:47
@JordanBelford JordanBelford reopened this Feb 13, 2016
@kwesterfeld
Copy link

Is there a way to test this PR? I just ran across this in my app and would like to have html reload work

@JordanBelford
Copy link
Author

There are a couple of easy ways to test.
Install from my fork using jspm

jspm uninstall systemjs-hot-reloader
jspm install systemjs-hot-reloader=github:JordanBelford/systemjs-hot-reloader

Or, drill down into your jspm_packages and find the systemjs-hot-reloader source. Then copy paste the PR.

@capaj
Copy link
Collaborator

capaj commented Feb 16, 2016

@JordanBelford thanks, will try it out before the end of this week and release a new version

capaj added a commit that referenced this pull request Feb 16, 2016
@capaj capaj merged commit d2c1589 into alexisvincent:master Feb 16, 2016
@kwesterfeld
Copy link

I tested this, works awesome!

Thanks

let module = System.loads[moduleImportName]
let parentModuleName
if (module && module.importers && module.importers.length) {
parentModuleName = System.loads[moduleImportName].importers[0].name
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JordanBelford shouldn't we iterate all importers? Won't this cause problems if one html file is imported by more than one file?

Copy link
Author

Choose a reason for hiding this comment

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

@capaj I agree. I will try to implement multiple importers today.

@kwesterfeld
Copy link

Can you release/tag the reloader library with these changes as-is? They are working well enough to get started, and perhaps you can create another issue to track the multi-module import ?

@npbenjohnson
Copy link
Contributor

Hardcoding .html to !text is a breaking change. You should check if SystemJs has a way to resolve importers from the meta section for a file.

@jonricaurte
Copy link

#74 (comment)

@npbenjohnson
Copy link
Contributor

This broke working hot-reloading for me. Transpiling and using text plugin 0.0.8 works without this code, but not with it.

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.

None yet

5 participants