fixup! fs: add stream utilities to FileHandle

nodejs · aduh95 · Sep 5, 2021 · Sep 6, 2021 · Sep 6, 2021 · Sep 8, 2021
commit 158823acb1f5e1d9cc557705a4568905a198059a
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -377,15 +377,12 @@ added: REPLACEME
 -->
 
 * `options` {Object}
-  * `flags` {string} See [support of file system `flags`][]. **Default:**
-    `'r'`.
   * `encoding` {string} **Default:** `null`
   * `autoClose` {boolean} **Default:** `true`
   * `emitClose` {boolean} **Default:** `true`
   * `start` {integer}
   * `end` {integer} **Default:** `Infinity`
   * `highWaterMark` {integer} **Default:** `64 * 1024`
-  * `fs` {Object|null} **Default:** `null`
 * Returns: {fs.ReadStream} See [Readable Stream][].
 
 Unlike the 16 kb default `highWaterMark` for a readable stream, the stream
@@ -408,10 +405,6 @@ By default, the stream will emit a `'close'` event after it has been
 destroyed, like most `Readable` streams.  Set the `emitClose` option to
 `false` to change this behavior.
 
-By providing the `fs` option, it is possible to override the corresponding `fs`
-implementations for `open`, `read`, and `close`. When providing the `fs` option,
-overrides for `open`, `read`, and `close` are required.
-
 ```mjs
 import { open } from 'fs/promises';
 
@@ -666,7 +659,6 @@ added: REPLACEME
   * `autoClose` {boolean} **Default:** `true`
   * `emitClose` {boolean} **Default:** `true`
   * `start` {integer}
-  * `fs` {Object|null} **Default:** `null`
 * Returns: {fs.WriteStream} See [Writable Stream][].
 
 `options` may also include a `start` option to allow writing data at some
@@ -685,12 +677,6 @@ By default, the stream will emit a `'close'` event after it has been
 destroyed, like most `Writable` streams.  Set the `emitClose` option to
 `false` to change this behavior.
 
-By providing the `fs` option it is possible to override the corresponding `fs`
-implementations for `open`, `write`, `writev` and `close`. Overriding `write()`
-without `writev()` can reduce performance as some optimizations (`_writev()`)
-will be disabled. When providing the `fs` option,  overrides for `open`,
-`close`, and at least one of `write` and `writev` are required.
-
 #### `filehandle.writev(buffers[, position])`
 <!-- YAML
 added: v12.9.0

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -106,7 +106,6 @@ const {
 const {
   readableStreamCancel,
 } = require('internal/webstreams/readablestream');
-const { ReadStream, WriteStream } = require('internal/fs/streams');
 
 const getDirectoryEntriesPromise = promisify(getDirents);
 const validateRmOptionsPromise = promisify(validateRmOptions);
@@ -116,6 +115,12 @@ function lazyLoadCpPromises() {
   return cpPromises ??= require('internal/fs/cp/cp').cpFn;
 }
 
+// Lazy loaded to avoid circular dependency.
+let fsStreams;
+function lazyFsStreams() {
+  return fsStreams ??= require('internal/fs/streams');
+}
+
 class FileHandle extends EventEmitterMixin(JSTransferable) {
   /**
    * @param {InternalFSBinding.FileHandle | undefined} filehandle
@@ -257,36 +262,34 @@ class FileHandle extends EventEmitterMixin(JSTransferable) {
    * @typedef {import('./streams').ReadStream
    * } NodeJSReadStream
    * @param {{
-   *   flags?: string;
    *   encoding?: string;
    *   autoClose?: boolean;
    *   emitClose?: boolean;
    *   start: number;
    *   end?: number;
    *   highWaterMark?: number;
-   *   fs?: Object | null;
    *   }} [options]
    * @returns {NodeJSReadStream}
    */
   readStream(options = undefined) {
-    return new ReadStream(undefined, { ...options, fd: this[kFd] });
+    const { ReadStream } = lazyFsStreams();
+    return new ReadStream(undefined, { ...options, fd: this });
   }
 
   /**
    * @typedef {import('./streams').WriteStream
    * } NodeJSWriteStream
    * @param {{
-   *   flags?: string;
    *   encoding?: string;
    *   autoClose?: boolean;
    *   emitClose?: boolean;
    *   start: number;
-   *   fs?: Object | null;
    *   }} [options]
    * @returns {NodeJSWriteStream}
    */
   writeStream(options = undefined) {
-    return new WriteStream(undefined, { ...options, fd: this[kFd] });
+    const { WriteStream } = lazyFsStreams();
+    return new WriteStream(undefined, { ...options, fd: this });
   }
 
   [kTransfer]() {