-
Notifications
You must be signed in to change notification settings - Fork 31k
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
watch: ensure watch mode detects deleted and re-added files #52879
base: main
Are you sure you want to change the base?
Conversation
737ea70
to
95a9603
Compare
95a9603
to
4d63f01
Compare
shouldn't this be added to https://github.com/nodejs/node/blob/d3eb1cb3850b4194f652a07e74631788dc94e4c4/lib/internal/fs/recursive_watch.js instead? |
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 #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 |
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 |
0c5b9c0
to
38c0947
Compare
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:Split out from #50890