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

LiveReload always sends all assets #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tbranyen
Copy link

@tbranyen tbranyen commented Jun 2, 2017

A problem with this plugin is that regardless of what has changed, the
same output assets are always emitted. I've taken a stab at filtering
down, but to be honest, I made it a total hack job. For some reason the
webpack plugin API doesn't make it easy (even after reading docs) to
figure out what files changed and what assets they belong to.

That aside, the goal of this PR is to allow CSS to LiveReload without
doing a full page refresh. It does work. However, it relies on adjusting
the startTime
(https://github.com/webpack/docs/wiki/how-to-write-a-plugin#monitoring-the-watch-graph)
by offseting process.uptime() for the first call, otherwise
the modified timestamps aren't accurate. It also relies on enabling
sourceMaps for css-loader to get access to the css asset children.

I'd love a review and tips on how to improve this, but for now this
should greatly assist with reloading ExtractTextPlugin-based webpack
configs, without requiring the webpack-dev-server.

@tbranyen tbranyen force-pushed the support-css-modules branch 2 times, most recently from 4948774 to d5c63d8 Compare June 2, 2017 23:30
@tbranyen tbranyen mentioned this pull request Jun 2, 2017
@tbranyen tbranyen force-pushed the support-css-modules branch from 37f677c to 8972480 Compare June 3, 2017 00:23
@tbranyen
Copy link
Author

tbranyen commented Jun 3, 2017

I published this to make it easier for folks to try in their own apps:

I'll deprecate the package after this gets updated and merged.

@rstacruz
Copy link

rstacruz commented Jun 4, 2017

Nice work!

@tbranyen if I may suggest, when publishing your own forks, you can name them as @tbranyen/webpack-livereload-plugin :)

@kayue
Copy link

kayue commented Jun 9, 2017

@tbranyen There is a failed test case, does it need to be fixed?

@statianzo what do you think about this feature?

@tbranyen tbranyen force-pushed the support-css-modules branch from f7ac10b to dbe6a5e Compare February 20, 2018 17:38
A problem with this plugin is that regardless of what has changed, the
same output assets are always emitted. I've taken a stable at filtering
down, but to be honest, I made it a total hack job. For some reason the
webpack plugin API doesn't not make it easy (even after reading docs) to
figure out what files changed and what assets they belong to.

That aside, the goal of this PR is to allow CSS to LiveReload without
doing a full page refresh. It does work. However, it relies on adjusting
the `startTime`
(https://github.com/webpack/docs/wiki/how-to-write-a-plugin#monitoring-the-watch-graph)
by offseting `process.uptime()` at least for the first call, otherwise
the modified timestamps aren't accurate. It also relies on enabling
sourceMaps for css-loader to get access to the css asset children.

I'd love a review and tips on how to improve this, but for now this
should greatly assist with reloading ExtractTextPlugin-based webpack
configs, without requiring the webpack-dev-server.
@tbranyen tbranyen force-pushed the support-css-modules branch from dbe6a5e to 5c2efa9 Compare February 20, 2018 17:41
@tbranyen
Copy link
Author

@statianzo I've updated and rebased this PR. Let me know if you need anything else to merge.

Copy link
Owner

@statianzo statianzo left a comment

Choose a reason for hiding this comment

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

One comment. Other than that, it looks good to me. Thanks @tbranyen!

}
else {
this.changedFiles = Object.keys(timestamps).filter(function(watchfile) {
return this.startTime < Math.ceil(statSync(watchfile).mtime);
Copy link
Owner

Choose a reason for hiding this comment

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

Synchronously stating all files seems like it could be a cause for a hang in a larger product.

Copy link
Author

Choose a reason for hiding this comment

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

True, and I suspect webpack has already stat'd these files at some other point. Wonder if there is a way to tap into a stat cache of some kind?

Copy link
Owner

Choose a reason for hiding this comment

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

In the webpack wiki you linked, it looks like they're using prevTimestamps as a way to see changes.

 var changedFiles = Object.keys(compilation.fileTimestamps).filter(function(watchfile) {
      return (this.prevTimestamps[watchfile] || this.startTime) < (compilation.fileTimestamps[watchfile] || Infinity);
    }.bind(this));


this.startTime = new Date();
this.startTime.setSeconds(-process.uptime());
this.prevTimestamps = {};
Copy link
Owner

Choose a reason for hiding this comment

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

I see no usage of prevTimestamps

@tbranyen
Copy link
Author

I noticed this PR falls short when using glob patterns within other files. So for instance, I updated somefile.css that is embedded in myapp.css through a glob pattern. This code I added is unable to reconcile nested changes.

Not sure if you have any thoughts on handling this case.

@Kagami
Copy link

Kagami commented Mar 11, 2018

This doesn't work for me in WebPack 4.

And it's hard to make right because you need to find output name by source name which was changed. There're ways to do that, e.g. changed chunks example seems to provide required info.

But this won't help much: if you required .css in JS module and use extract-text-webpack-plugin to extract it to separate file, technically you depend on CSS in JS module and so JS also needs to be refreshed when you update CSS, so full page should be reloaded.

I don't know any good way to solve this in general case because some may use result of require() in JS module so we should also update all dependency tree. So I made this quick hack instead:

diff --git a/index.js b/index.js
index 89a4ff2..642616f 100755
--- a/index.js
+++ b/index.js
@@ -17,6 +17,7 @@ function LiveReloadPlugin(options) {
   this.protocol = this.options.protocol ? this.options.protocol + ':' : '';
   this.hostname = this.options.hostname || '" + location.hostname + "';
   this.server = null;
+  this.lastUpdate = Date.now();
 }
 
 function arraysEqual(a1, a2){
@@ -56,6 +57,23 @@ LiveReloadPlugin.prototype.done = function done(stats) {
   var hash = stats.compilation.hash;
   var childHashes = (stats.compilation.children || []).map(child => child.hash);
   var files = Object.keys(stats.compilation.assets);
+  const timestamps = stats.compilation.fileTimestamps;
+  if (timestamps.size) {
+    let fastUpdate = true;
+    const fastUpdateRe = /\.css$/;
+    for (const [name, timestamp] of timestamps) {
+      if (timestamp > this.lastUpdate) {
+        if (!fastUpdateRe.test(name)) {
+          fastUpdate = false;
+          break;
+        }
+      }
+    }
+    if (fastUpdate) {
+      files = files.filter(name => fastUpdateRe.test(name));
+    }
+  }
+  this.lastUpdate = Date.now();
   var include = files.filter(function(file) {
     return !file.match(this.ignore);
   }, this);

It only updates CSS when nothing else was changed. It's wrong, but works for me in simple webpack setup.

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.

5 participants