node icon indicating copy to clipboard operation
node copied to clipboard

watch: ensure watch mode detects deleted and re-added files

Open znewsham opened this issue 9 months ago • 4 comments

In systems that don't support recursive file watching, If you watch a file that gets replaced as sometimes happens (e.g., gedit and certain configurations of docker), subsequent changes wont be watched.

This PR detects rename events and adds a 60s timeout with a 1s interval to watch for the path to exist, then emits a change event for that file, ensuring that:

  1. if the contents of the file changed between being removed and re-added that change is detected
  2. subsequent changes are detected

Split out from https://github.com/nodejs/node/pull/50890

znewsham avatar May 07 '24 18:05 znewsham

shouldn't this be added to https://github.com/nodejs/node/blob/d3eb1cb3850b4194f652a07e74631788dc94e4c4/lib/internal/fs/recursive_watch.js instead?

MoLow avatar May 08 '24 17:05 MoLow

I'm not familiar with that, are you suggesting that the change be made to the general recursive watch mechanism instead of the watch/restart functionality? Currently, on linux - the watch functionality isn't recursive at all, so I'm not sure how changes to the recursive mechanism would change the behaviour in watch mode?

znewsham avatar May 08 '24 17:05 znewsham

I'm not familiar with that, are you suggesting that the change be made to the general recursive watch mechanism instead of the watch/restart functionality? Currently, on linux - the watch functionality isn't recursive at all, so I'm not sure how changes to the recursive mechanism would change the behaviour in watch mode?

Yes, that is my suggestion. that file is an implementation for environments where recursive watch isn't natively implemented via libuv, and it was added after the addition of watch mode.

my guess that file already handles this edge case, and if not it can be added there

I started working on usage of non native recursive watch at https://github.com/nodejs/node/pull/45271 and there were some issues (in the CI if I recall), but I would be happy if you or someone else picked it up

MoLow avatar May 08 '24 17:05 MoLow

what's the advantage of a recursive watch? Particularly in environments where it isn't natively supported, where it's still one watcher per file.

The nice thing about non recursive watch mode is it should have near perfect detection - no false positives where an unused file triggers a restart. It's possible I've missunderstood how the recursive watch works though.

A trivial limitation of the recursive watch implementation you linked is that it appears it doesn't follow symlinks, so changes to local NPM modules (installed via symlink) won't trigger a restart.

My goal with this PR is not that it implements (or fixes) recursive file watching, but that in non recursive mode it correctly detects changes to a single watched file

znewsham avatar May 09 '24 15:05 znewsham