node-dev
node-dev copied to clipboard
Handle multiple file changes.
Without it if multiple changes occured at once, it will try to start process multiple times.
@bjornstar are you going to review this somehow?
@bjornstar see please)
I see test failures, and no tests to validate the behavior.
Well, I've tried to reproduce the problem with tests, and could not: watcher.removeAll()
in the handler removes watching after just the first change. So the next change is not caught and handled.
I'm pretty sure that in my case I see that multiple file changes happened at once resulted in multiple Restarting
for each file. My case is dev app that is running in docker (which requires polling) and it is watching for quite a lot of files. And simultaneous change happen due to re-compilation and emitting a great amount of new files. So, I guess, this could probably occur because watcher removal is not completed to prevent sequential change events from firing and handling. And setting isPaused
right after the first change, and then checking it for subsequent change solved this problem (I'm currently using a modified version to avoid this).
Btw if to add:
watcher.on('change', file => {
if (isPaused) return;
isPaused = true;
I don't see any current tests failing.
@bjornstar So it seems that this case is is not easy to reproduce in tests. But logically it should be understood that this:
watcher.on('change', file => {
if (isPaused) return;
isPaused = true;
allows to prevent consequent changes that should not be handled.
What do you think?
hi @bjornstar, i use this PR as a patch and test:
- save many files at the same time
- switch between branches with many file changes
and works fine 🔥 prevents a lot of unnecessary restarts.
@wclr good work
@bjornstar Look at this again please, this is a simple logical change that we will NOT handle any changes as soon as isPaused
set, currently it is possible that multiple file changes will pass without such filter and try to restart.
Current:
watcher.on('change', file => {
clearOutput();
notify('Restarting', `${file} has been modified`);
watcher.removeAll();
isPaused = true;
if (child) {
// Child is still running, restart upon exit
child.on('exit', start);
With change:
watcher.on('change', file => {
if (isPaused) return;
isPaused = true;
clearOutput();
notify('Restarting', `${file} has been modified`);
watcher.removeAll();
if (child) {
// Child is still running, restart upon exit
child.on('exit', start);