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

Feature request: Would be nice if -w timer reset when new changes detected #108

Closed
SystemDisc opened this Issue Sep 29, 2017 · 15 comments

Comments

2 participants
@SystemDisc

SystemDisc commented Sep 29, 2017

If I do:
livereload 'dist/server/main.js, dist/client/main.js' -w 1000
and dist/server/main.js or dist/client/main.js change, the 1000ms timer starts and when the timer reaches zero livereload tells the browser to refresh.

Howevever, if dist/server/main.js or dist/client/main.js change, and during the 1000ms delay/timer dist/server/main.js or dist/client/main.js change again, the timer does not reset. In my setup, that could result in the browser refreshing when the files are not ready.

Ideally, in my setup, I'd like the timer to start when a change is detected, and the timer to be reset if a change is detected during that countdown. This would mean that livereload would only tell the browser to refresh if a change was detected and it's been at least X milliseconds (where X is the value after -w) since the last detected change.

It would be fine with me if instead of changing how -w worked, a new parameter was added. However, I think adding this would be an improvement on how -w works.

@napcs

This comment has been minimized.

Show comment
Hide comment
@napcs

napcs Oct 3, 2017

Owner

I get what you're asking - but can you clarify why you're using the -w flag in the first place? You almost never need it unless your OS is being incredibly silly. The file watcher under the hood should just work as you describe.

Owner

napcs commented Oct 3, 2017

I get what you're asking - but can you clarify why you're using the -w flag in the first place? You almost never need it unless your OS is being incredibly silly. The file watcher under the hood should just work as you describe.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 3, 2017

I use -w because I'm using Angular 4 with AoT, webpack, ExpressJS with server-side rendering, and nodemon. There's some time between the files being built/updated and them being available to the browser (i.e. node has to restart, start listening, and be ready to serve information). Also, my request for it to wait until things settle down is because webpack might start rebuilding things, and I might save a second file during that process, which will cause webpack to rebuild again after it's finished.

SystemDisc commented Oct 3, 2017

I use -w because I'm using Angular 4 with AoT, webpack, ExpressJS with server-side rendering, and nodemon. There's some time between the files being built/updated and them being available to the browser (i.e. node has to restart, start listening, and be ready to serve information). Also, my request for it to wait until things settle down is because webpack might start rebuilding things, and I might save a second file during that process, which will cause webpack to rebuild again after it's finished.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 3, 2017

If I remove -w in that project, the browser reloads before node is ready and I get a connection error in Chrome.

SystemDisc commented Oct 3, 2017

If I remove -w in that project, the browser reloads before node is ready and I get a connection error in Chrome.

@napcs

This comment has been minimized.

Show comment
Hide comment
@napcs

napcs Oct 5, 2017

Owner

Ah. Ok. With that setup, you shouldn't really be using livereload. You probably should look at webpack-dev-server or one of the other plugins for webpack and reloading.

Owner

napcs commented Oct 5, 2017

Ah. Ok. With that setup, you shouldn't really be using livereload. You probably should look at webpack-dev-server or one of the other plugins for webpack and reloading.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 5, 2017

I'm sorry. That's not an option. Express needs to be the server for Angular 4 server-side-rendering to work.

I think for development, livereload works fine. It could work better, and I think a possible solution would be to manually set up livereload in src/server/main.ts instead of using livereload CLI with connect-livereload. However, I still think what I described in the first post would be a beneficial feature to have.

SystemDisc commented Oct 5, 2017

I'm sorry. That's not an option. Express needs to be the server for Angular 4 server-side-rendering to work.

I think for development, livereload works fine. It could work better, and I think a possible solution would be to manually set up livereload in src/server/main.ts instead of using livereload CLI with connect-livereload. However, I still think what I described in the first post would be a beneficial feature to have.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 5, 2017

On second thought, I don't think I can set up livereload in src/server/main.ts since the websocket will close when the server is restarted via nodemon.

SystemDisc commented Oct 5, 2017

On second thought, I don't think I can set up livereload in src/server/main.ts since the websocket will close when the server is restarted via nodemon.

@napcs

This comment has been minimized.

Show comment
Hide comment
@napcs

napcs Oct 5, 2017

Owner

The way the timeout is implemented isn't currently something I can just reset. I'm happy to review a pull request, but it's really not what the timeout is intended for.

Would something like this work? Start livereload with your nodemon script:

// livereload setup

const liveReload = require('../lib/livereload');
const liveReloadServer = liveReload.createServer({
  port: 35729,
  debug: true,
});

liveReloadServer.watch(__dirname);

// some other setup

nodemon.on('restart', function() {
    setTimeout(function() {
      liveReloadServer.refresh('');
    }, 3000);
});

I know this worked for someone else who needed livereload to trigger reloads when their server restarted.

Owner

napcs commented Oct 5, 2017

The way the timeout is implemented isn't currently something I can just reset. I'm happy to review a pull request, but it's really not what the timeout is intended for.

Would something like this work? Start livereload with your nodemon script:

// livereload setup

const liveReload = require('../lib/livereload');
const liveReloadServer = liveReload.createServer({
  port: 35729,
  debug: true,
});

liveReloadServer.watch(__dirname);

// some other setup

nodemon.on('restart', function() {
    setTimeout(function() {
      liveReloadServer.refresh('');
    }, 3000);
});

I know this worked for someone else who needed livereload to trigger reloads when their server restarted.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 5, 2017

This looks promising. I'll get back to you on this.

SystemDisc commented Oct 5, 2017

This looks promising. I'll get back to you on this.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 5, 2017

Thank you.

SystemDisc commented Oct 5, 2017

Thank you.

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Oct 11, 2017

Can I use the livereload CLI, have it not watch a directory/file, and tell it to refresh from my code? What you've suggested will not work for me.

SystemDisc commented Oct 11, 2017

Can I use the livereload CLI, have it not watch a directory/file, and tell it to refresh from my code? What you've suggested will not work for me.

@napcs

This comment has been minimized.

Show comment
Hide comment
@napcs

napcs Oct 11, 2017

Owner

No. But you could leave this out of the above code:

liveReloadServer.watch(__dirname);

Then livereload won't watch. But calling

      liveReloadServer.refresh('');

will still send the ping.

Working up a fix to #110 to support this more gracefully - it fails to shutdown correctly in the current release.

Owner

napcs commented Oct 11, 2017

No. But you could leave this out of the above code:

liveReloadServer.watch(__dirname);

Then livereload won't watch. But calling

      liveReloadServer.refresh('');

will still send the ping.

Working up a fix to #110 to support this more gracefully - it fails to shutdown correctly in the current release.

@napcs

This comment has been minimized.

Show comment
Hide comment
@napcs

napcs Oct 20, 2017

Owner

Just an update - 0.6.3 is out and it fixes #110

Owner

napcs commented Oct 20, 2017

Just an update - 0.6.3 is out and it fixes #110

@napcs napcs closed this Dec 21, 2017

@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Jan 13, 2018

For future reference, this is what I ended up putting in my webpack.config.js

if (process.env.NODE_ENV === 'development') {
	const { spawn } = require('child_process');
	let server;
	let startServer = () => {
		console.log('starting server');
		server = spawn('npm', ['run', 'server:dev']);
		server.on('exit', () => {
			startServer();
		});
		server.stdout.pipe(process.stdout);
		server.stderr.pipe(process.stderr);
		console.log('sending livereload signal');
		lrServer.refresh('');
	};
	let livereload = require('livereload');
	let lrServer = livereload.createServer();
	serverConfig.plugins.push(function() {
		this.plugin('done', (stats) => {
			console.log('build done');
			if (server) {
				console.log('stopping server');
				server.kill();
			}
			else {
				startServer();
			}
		});
	});
}

SystemDisc commented Jan 13, 2018

For future reference, this is what I ended up putting in my webpack.config.js

if (process.env.NODE_ENV === 'development') {
	const { spawn } = require('child_process');
	let server;
	let startServer = () => {
		console.log('starting server');
		server = spawn('npm', ['run', 'server:dev']);
		server.on('exit', () => {
			startServer();
		});
		server.stdout.pipe(process.stdout);
		server.stderr.pipe(process.stderr);
		console.log('sending livereload signal');
		lrServer.refresh('');
	};
	let livereload = require('livereload');
	let lrServer = livereload.createServer();
	serverConfig.plugins.push(function() {
		this.plugin('done', (stats) => {
			console.log('build done');
			if (server) {
				console.log('stopping server');
				server.kill();
			}
			else {
				startServer();
			}
		});
	});
}
@SystemDisc

This comment has been minimized.

Show comment
Hide comment
@SystemDisc

SystemDisc Jan 13, 2018

Also, in the case where it takes a bit for your server to start up and the server outputs something like log "listening on http://localhost:3000/" when it's ready, you can do the following:

server.stdout.on('data', (data) => {
	if (/^listening/.test(data)) {
		console.log('sending livereload signal');
		lrServer.refresh('');
	}
});

This would make sure livereload only causes the page to refresh when the server is ready.

SystemDisc commented Jan 13, 2018

Also, in the case where it takes a bit for your server to start up and the server outputs something like log "listening on http://localhost:3000/" when it's ready, you can do the following:

server.stdout.on('data', (data) => {
	if (/^listening/.test(data)) {
		console.log('sending livereload signal');
		lrServer.refresh('');
	}
});

This would make sure livereload only causes the page to refresh when the server is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment