fs: update requested changes

doc/api/fs.md

@@ -6403,9 +6403,9 @@ By default, all {fs.FSWatcher} objects are "ref'ed", making it normally
 unnecessary to call `watcher.ref()` unless `watcher.unref()` had been
 called previously.
 
-`watcher.ref()` is not available on Linux. An `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM`
-exception will be thrown when the function is used on a platform that
-does not support it.
+`watcher.ref()` is only supported on macOS and Windows. An
+`ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` exception will be thrown when the
+function is used on a platform that does not support it.
 
 #### `watcher.unref()`
 
@@ -6423,9 +6423,9 @@ event loop running, the process may exit before the {fs.FSWatcher} object's
 callback is invoked. Calling `watcher.unref()` multiple times will have
 no effect.
 
-`watcher.unref()` is not available on Linux. An `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM`
-exception will be thrown when the function is used on a platform that
-does not support it.
+`watcher.unref()` is only supported on macOS and Windows. An
+`ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` exception will be thrown when the
+function is used on a platform that does not support it.
 
 ### Class: `fs.StatWatcher`
 

lib/fs.js

@@ -2303,10 +2303,8 @@ function watch(filename, options, listener) {
   // libuv does not support recursive file watch on Linux due to
   // the limitations of inotify.
   if (options.recursive && !isOSX && !isWindows) {
-    validateBoolean(options.recursive, 'options.recursive');
-
     watcher = new linuxWatcher.FSWatcher(options);
-    watcher[linuxWatcher.kFSWatchStart](filename);
+    watcher[watchers.kFSWatchStart](filename);
   } else {
     watcher = new watchers.FSWatcher();
     watcher[watchers.kFSWatchStart](filename,

lib/internal/fs/promises.js

@@ -12,6 +12,7 @@ const {
   PromiseReject,
   SafeArrayIterator,
   SafePromisePrototypeFinally,
+  SymbolAsyncIterator,
   Symbol,
   Uint8Array,
 } = primordials;
@@ -88,7 +89,7 @@ const {
 } = require('internal/util');
 const { EventEmitterMixin } = require('internal/event_target');
 const { StringDecoder } = require('string_decoder');
-const { watch } = require('internal/fs/watchers');
+const { kFSWatchStart, watch } = require('internal/fs/watchers');
 const linuxWatcher = require('internal/fs/watch/linux');
 const { isIterable } = require('internal/streams/utils');
 const assert = require('internal/assert');
@@ -930,8 +931,8 @@ function _watch(filename, options = kEmptyObject) {
     // the limitations of inotify.
     if (options.recursive && !isOSX && !isWindows) {
       const watcher = new linuxWatcher.FSWatcher(options);
-      watcher[linuxWatcher.kFSWatchStart](filename);
-      return watcher;
+      watcher[kFSWatchStart](filename);
+      return watcher[SymbolAsyncIterator]();
     }
   }
 
@@ -968,7 +969,7 @@ module.exports = {
     writeFile,
     appendFile,
     readFile,
-    watch: _watch,
+    watch: !isOSX && !isWindows ? _watch : watch,
     constants,
   },
 

lib/internal/fs/watch/linux.js

@@ -1,28 +1,30 @@
 'use strict';
 
 const {
+  SafePromiseAll,
+  PromisePrototypeThen,
   SafeMap,
+  SafeSet,
   StringPrototypeStartsWith,
-  Symbol,
+  StringPrototypeSlice,
+  StringPrototypeLastIndexOf,
   SymbolAsyncIterator,
-  ObjectCreate,
 } = primordials;
 
-const { Buffer } = require('buffer');
+const assert = require('assert');
+const { EventEmitter } = require('events');
 const {
   codes: {
     ERR_FEATURE_UNAVAILABLE_ON_PLATFORM,
-    ERR_INVALID_ARG_TYPE,
     ERR_INVALID_ARG_VALUE,
   },
 } = require('internal/errors');
-const { URL } = require('internal/url');
+const { getValidatedPath } = require('internal/fs/utils');
+const { kFSWatchStart } = require('internal/fs/watchers');
 const { kEmptyObject } = require('internal/util');
-const { validateObject, validateBoolean, validateAbortSignal, validateBuffer } = require('internal/validators');
-const { EventEmitter, once } = require('events');
+const { validateBoolean, validateAbortSignal } = require('internal/validators');
 const path = require('path');
-
-const kFSWatchStart = Symbol('kFSWatchStart');
+const { Readable } = require('stream');
 
 let internalSync;
 let internalPromises;
@@ -38,20 +40,23 @@ function lazyLoadFsSync() {
 }
 
 async function traverse(dir, files = new SafeMap()) {
-  const { readdir } = lazyLoadFsPromises();
+  const { opendir } = lazyLoadFsPromises();
 
-  const filenames = await readdir(dir, { withFileTypes: true });
+  const filenames = await opendir(dir);
+  const subdirectories = new SafeSet();
 
   for await (const file of filenames) {
     const f = path.join(dir, file.name);
 
     files.set(f, file);
 
     if (file.isDirectory()) {
-      await traverse(f, files);
+      subdirectories.add(traverse(f, files));
     }
   }
 
+  await SafePromiseAll(subdirectories);
+
   return files;
 }
 
@@ -60,32 +65,32 @@ class FSWatcher extends EventEmitter {
   #closed = false;
   #files = new SafeMap();
   #rootPath = path.resolve();
+  #watchingFile = false;
 
   constructor(options = kEmptyObject) {
     super();
 
-    validateObject(options, 'options');
+    assert(typeof options === 'object' && options.recursive === true);
 
-    if (options.persistent != null) {
-      validateBoolean(options.persistent, 'options.persistent');
-    }
+    const { persistent, recursive, signal, encoding } = options;
 
-    if (options.recursive != null) {
-      validateBoolean(options.recursive, 'options.recursive');
+    if (persistent != null) {
+      validateBoolean(persistent, 'options.persistent');
     }
 
-    if (options.signal != null) {
-      validateAbortSignal(options.signal, 'options.signal');
+    if (signal != null) {
+      validateAbortSignal(signal, 'options.signal');
     }
 
-    if (options.encoding != null) {
+    if (encoding != null) {
       // This is required since on macOS and Windows it throws ERR_INVALID_ARG_VALUE
-      if (typeof options.encoding !== 'string') {
-        throw new ERR_INVALID_ARG_VALUE('encoding', 'options.encoding');
+      if (typeof encoding !== 'string') {
+        throw new ERR_INVALID_ARG_VALUE(encoding, 'options.encoding');
       }
     }
 
-    this.#options = options;
+
+    this.#options = { persistent, recursive, signal, encoding };
   }
 
   close() {
@@ -106,20 +111,21 @@ class FSWatcher extends EventEmitter {
 
   /**
    * @param {string} file
-   * @return {string}
+   * @return {string} the full path of `file` if we're watching a directory, or the
+                      filename when watching a file.
    */
   #getPath(file) {
-    const root = this.#files.get(this.#rootPath);
-
-    // When watching files the path is always the root.
-    if (root.isFile()) {
-      return this.#rootPath.slice(this.#rootPath.lastIndexOf('/') + 1);
+    // Always return the filename instead of the full path if watching a file (not a directory)
+    if (this.#watchingFile) {
+      return StringPrototypeSlice(this.#rootPath, StringPrototypeLastIndexOf(this.#rootPath, '/') + 1);
     }
 
+    // When watching directories, and root directory is changed, return the full file path
     if (file === this.#rootPath) {
       return this.#rootPath;
     }
 
+    // Otherwise return relative path for all files & folders
     return path.relative(this.#rootPath, file);
   }
 
@@ -140,12 +146,12 @@ class FSWatcher extends EventEmitter {
       const files = await opendir(folder);
 
       for await (const file of files) {
-        const f = path.join(folder, file.name);
-
         if (this.#closed) {
           break;
         }
 
+        const f = path.join(folder, file.name);
+
         if (!this.#files.has(f)) {
           this.#files.set(f, file);
           this.emit('change', 'rename', this.#getPath(f));
@@ -163,12 +169,11 @@ class FSWatcher extends EventEmitter {
   }
 
   #watchFile(file) {
-    const { watchFile } = lazyLoadFsSync();
-
     if (this.#closed) {
       return;
     }
 
+    const { watchFile } = lazyLoadFsSync();
     const existingStat = this.#files.get(file);
 
     watchFile(file, {
@@ -200,36 +205,25 @@ class FSWatcher extends EventEmitter {
   }
 
   [kFSWatchStart](filename) {
-    if (typeof filename !== 'string' && !(filename instanceof Buffer) && !(filename instanceof URL)) {
-      throw new ERR_INVALID_ARG_TYPE('filename', 'string', filename);
-    }
-
-    if (typeof filename === 'string') {
-      if (filename.length === 0) {
-        throw new ERR_INVALID_ARG_TYPE('filename', 'string', filename);
-      }
-    } else if (Buffer.isBuffer(filename)) {
-      validateBuffer(filename, 'filename');
-
-      if (filename.length === 0) {
-        throw new ERR_INVALID_ARG_TYPE('filename', 'buffer', filename);
-      }
-    }
+    filename = path.resolve(getValidatedPath(filename));
 
     try {
       const file = lazyLoadFsSync().statSync(filename);
 
       this.#rootPath = filename;
       this.#closed = false;
       this.#files.set(filename, file);
+      this.#watchingFile = file.isFile();
 
       if (file.isDirectory()) {
-        traverse(filename, this.#files)
-          .then(() => {
+        PromisePrototypeThen(
+          traverse(filename, this.#files),
+          () => {
             for (const f of this.#files.keys()) {
               this.#watchFile(f);
             }
-          });
+          },
+        );
       } else {
         this.#watchFile(filename);
       }
@@ -253,22 +247,23 @@ class FSWatcher extends EventEmitter {
   }
 
   [SymbolAsyncIterator]() {
-    const watcher = this;
-    return ObjectCreate({
-      __proto__: null,
-      async next() {
-        if (watcher.#closed) {
-          return { __proto__: null, done: true };
-        }
-        const { 0: eventType, 1: filename } = await once(watcher, 'change');
-        return {
-          __proto__: null,
-          value: { __proto__: null, eventType, filename },
-          done: false,
-        };
+    const self = this;
+    const stream = new Readable({
+      destroy(error, callback) {
+        self.close();
+        callback(error);
       },
-      [SymbolAsyncIterator]() { return this; },
+      read() { },
+      autoDestroy: true,
+      objectMode: true,
+      signal: this.#options.signal,
     });
+
+    this.on('change', (eventType, filename) => {
+      stream.push({ eventType, filename });
+    });
+
+    return stream;
   }
 }
 

test/parallel/test-fs-watch-recursive-linux-promise.js

@@ -12,32 +12,47 @@ const { randomUUID } = require('crypto');
 const assert = require('assert');
 const path = require('path');
 const fs = require('fs/promises');
+const fsSync = require('fs');
 
 const tmpdir = require('../common/tmpdir');
 const testDir = tmpdir.path;
 tmpdir.refresh();
 
-(async () => {
-  {
-    // Add a file to already watching folder
+(async function run() {
+  // Add a file to already watching folder
 
-    const testsubdir = await fs.mkdtemp(testDir + path.sep);
-    const file = `${randomUUID()}.txt`;
-    const filePath = path.join(testsubdir, file);
-    const watcher = fs.watch(testsubdir, { recursive: true });
+  const testsubdir = await fs.mkdtemp(testDir + path.sep);
+  const file = `${randomUUID()}.txt`;
+  const filePath = path.join(testsubdir, file);
+  const watcher = fs.watch(testsubdir, { recursive: true });
 
-    setTimeout(async () => {
-      await fs.writeFile(filePath, 'world');
-    }, 100);
+  let interval;
 
+  process.nextTick(common.mustCall(() => {
+    interval = setInterval(() => {
+      fsSync.writeFileSync(filePath, 'world');
+    }, 500);
+  }));
+
+  try {
     for await (const payload of watcher) {
       const { eventType, filename } = payload;
 
       assert.ok(eventType === 'change' || eventType === 'rename');
 
       if (filename === file) {
+        clearInterval(interval);
+        interval = null;
         break;
       }
     }
+  } catch (error) {
+    if (error.name !== 'AbortError') {
+      assert.fail(error);
+    }
   }
+
+  process.on('exit', function() {
+    assert.ok(interval === null, 'watcher Object was not closed');
+  });
 })().then(common.mustCall());