From 4a4b599ce8510bafb0c6b2b8499265f906b61363 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 17 Nov 2022 23:00:15 +0200 Subject: [PATCH 1/5] fs: fix `nonNativeWatcher` watching folder with existing files --- lib/internal/fs/recursive_watch.js | 3 +++ test/parallel/test-fs-watch-recursive.js | 31 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index ed146fa28bed8d..a1d5999249c759 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -202,6 +202,9 @@ class FSWatcher extends EventEmitter { this.emit('change', 'rename', pathRelative(this.#rootPath, file)); } else if (currentStats.isDirectory()) { this.#watchFolder(file); + } else { + // Watching a dircetory will trigger a change event for child files + this.emit('change', 'change', pathRelative(this.#rootPath, file)); } }); } diff --git a/test/parallel/test-fs-watch-recursive.js b/test/parallel/test-fs-watch-recursive.js index a4c22bfbd2ce11..911e4eed332092 100644 --- a/test/parallel/test-fs-watch-recursive.js +++ b/test/parallel/test-fs-watch-recursive.js @@ -22,6 +22,37 @@ const tmpdir = require('../common/tmpdir'); const testDir = tmpdir.path; tmpdir.refresh(); +(async () => { + // Watch a folder and update an already existing file in it. + + const rootDirectory = fs.mkdtempSync(testDir + path.sep); + const testDirectory = path.join(rootDirectory, 'test-0'); + fs.mkdirSync(testDirectory); + + const testFile = path.join(testDirectory, 'file-1.txt'); + fs.writeFileSync(testFile, 'hello'); + + const watcher = fs.watch(testDirectory, { recursive: true }); + let watcherClosed = false; + watcher.on('change', common.mustCallAtLeast(function(event, filename) { + // Libuv inconsistenly emits a rename event for the file we are watching + assert.ok(event === 'change' || event === 'rename'); + + if (filename === path.basename(testFile)) { + watcher.close(); + watcherClosed = true; + } + })); + + await setTimeout(common.platformTimeout(100)); + fs.writeFileSync(testFile, 'hello'); + + process.once('exit', function() { + assert(watcherClosed, 'watcher Object was not closed'); + }); +})().then(common.mustCall()); + + (async () => { // Add a file to already watching folder From 4ab294b440d3af0cb7f0f08cb83ef9416697d48a Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 17 Nov 2022 23:54:22 +0200 Subject: [PATCH 2/5] Update lib/internal/fs/recursive_watch.js Co-authored-by: Yagiz Nizipli --- lib/internal/fs/recursive_watch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index a1d5999249c759..c7f0071de9a500 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -203,7 +203,7 @@ class FSWatcher extends EventEmitter { } else if (currentStats.isDirectory()) { this.#watchFolder(file); } else { - // Watching a dircetory will trigger a change event for child files + // Watching a directory will trigger a change event for child files this.emit('change', 'change', pathRelative(this.#rootPath, file)); } }); From 3a852d50fe7978a52000e92655f19c22ef1b9ba5 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 25 Nov 2022 09:15:50 +0200 Subject: [PATCH 3/5] CR --- lib/internal/fs/recursive_watch.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index c7f0071de9a500..17e32f18be6760 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -33,6 +33,8 @@ const { let internalSync; let internalPromises; +const NEW_FILES_AGE = 20_000; + function lazyLoadFsPromises() { internalPromises ??= require('fs/promises'); return internalPromises; @@ -203,8 +205,9 @@ class FSWatcher extends EventEmitter { } else if (currentStats.isDirectory()) { this.#watchFolder(file); } else { - // Watching a directory will trigger a change event for child files - this.emit('change', 'change', pathRelative(this.#rootPath, file)); + // Watching a directory will trigger a change event for child files) + const event = Date.now() - currentStats.birthtimeMs < NEW_FILES_AGE ? 'rename' : 'change'; + this.emit('change', event, pathRelative(this.#rootPath, file)); } }); } From 8fca3e2f14f4985d88f91c19aabcfef923afbe78 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 25 Nov 2022 09:22:24 +0200 Subject: [PATCH 4/5] fix --- lib/internal/fs/recursive_watch.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index 17e32f18be6760..13324f0c4d89dd 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -2,6 +2,7 @@ const { ArrayPrototypePush, + DateNow, SafePromiseAllReturnVoid, Promise, PromisePrototypeThen, @@ -206,7 +207,7 @@ class FSWatcher extends EventEmitter { this.#watchFolder(file); } else { // Watching a directory will trigger a change event for child files) - const event = Date.now() - currentStats.birthtimeMs < NEW_FILES_AGE ? 'rename' : 'change'; + const event = DateNow() - currentStats.birthtimeMs < NEW_FILES_AGE ? 'rename' : 'change'; this.emit('change', event, pathRelative(this.#rootPath, file)); } }); From e65d6172a291ff1c813d898e548bc5654e67c31c Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sun, 27 Nov 2022 08:52:35 +0200 Subject: [PATCH 5/5] CR --- lib/internal/fs/recursive_watch.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/internal/fs/recursive_watch.js b/lib/internal/fs/recursive_watch.js index 13324f0c4d89dd..950758547afd96 100644 --- a/lib/internal/fs/recursive_watch.js +++ b/lib/internal/fs/recursive_watch.js @@ -2,7 +2,6 @@ const { ArrayPrototypePush, - DateNow, SafePromiseAllReturnVoid, Promise, PromisePrototypeThen, @@ -34,8 +33,6 @@ const { let internalSync; let internalPromises; -const NEW_FILES_AGE = 20_000; - function lazyLoadFsPromises() { internalPromises ??= require('fs/promises'); return internalPromises; @@ -207,8 +204,7 @@ class FSWatcher extends EventEmitter { this.#watchFolder(file); } else { // Watching a directory will trigger a change event for child files) - const event = DateNow() - currentStats.birthtimeMs < NEW_FILES_AGE ? 'rename' : 'change'; - this.emit('change', event, pathRelative(this.#rootPath, file)); + this.emit('change', 'change', pathRelative(this.#rootPath, file)); } }); }