Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 59fff92

Browse filesBrowse files
aduh95BethGriggs
aduh95
authored and
BethGriggs
committed
fs: make open and close stream override optional when unused
When using `createReadStream` or `createWriteStream` with a specific file descriptor or `FileHandle` instead of a path, `open` method is not used, there is no point in forcing users to provide it. When using `createReadStream` or `createWriteStream` with `autoClose` set to false, `close` method is not used, there is no point in forcing users to provide it. PR-URL: #40013 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b7dc651 commit 59fff92
Copy full SHA for 59fff92

File tree

Expand file treeCollapse file tree

2 files changed

+77
-59
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+77
-59
lines changed
Open diff view settings
Collapse file

‎doc/api/fs.md‎

Copy file name to clipboardExpand all lines: doc/api/fs.md
+19-3Lines changed: 19 additions & 3 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1887,6 +1887,12 @@ behavior is similar to `cp dir1/ dir2/`.
18871887
<!-- YAML
18881888
added: v0.1.31
18891889
changes:
1890+
- version: REPLACEME
1891+
pr-url: https://github.com/nodejs/node/pull/40013
1892+
description: The `fs` option does not need `open` method if an `fd` was provided.
1893+
- version: REPLACEME
1894+
pr-url: https://github.com/nodejs/node/pull/40013
1895+
description: The `fs` option does not need `close` method if `autoClose` is `false`.
18901896
- version:
18911897
- v15.4.0
18921898
pr-url: https://github.com/nodejs/node/pull/35922
@@ -1962,7 +1968,9 @@ destroyed, like most `Readable` streams. Set the `emitClose` option to
19621968
19631969
By providing the `fs` option, it is possible to override the corresponding `fs`
19641970
implementations for `open`, `read`, and `close`. When providing the `fs` option,
1965-
overrides for `open`, `read`, and `close` are required.
1971+
an override for `read` is required. If no `fd` is provided, an override for
1972+
`open` is also required. If `autoClose` is `true`, an override for `close` is
1973+
also required.
19661974
19671975
```mjs
19681976
import { createReadStream } from 'fs';
@@ -2004,6 +2012,12 @@ If `options` is a string, then it specifies the encoding.
20042012
<!-- YAML
20052013
added: v0.1.31
20062014
changes:
2015+
- version: REPLACEME
2016+
pr-url: https://github.com/nodejs/node/pull/40013
2017+
description: The `fs` option does not need `open` method if an `fd` was provided.
2018+
- version: REPLACEME
2019+
pr-url: https://github.com/nodejs/node/pull/40013
2020+
description: The `fs` option does not need `close` method if `autoClose` is `false`.
20072021
- version:
20082022
- v15.4.0
20092023
pr-url: https://github.com/nodejs/node/pull/35922
@@ -2067,8 +2081,10 @@ destroyed, like most `Writable` streams. Set the `emitClose` option to
20672081
By providing the `fs` option it is possible to override the corresponding `fs`
20682082
implementations for `open`, `write`, `writev` and `close`. Overriding `write()`
20692083
without `writev()` can reduce performance as some optimizations (`_writev()`)
2070-
will be disabled. When providing the `fs` option, overrides for `open`,
2071-
`close`, and at least one of `write` and `writev` are required.
2084+
will be disabled. When providing the `fs` option, overrides for at least one of
2085+
`write` and `writev` are required. If no `fd` option is supplied, an override
2086+
for `open` is also required. If `autoClose` is `true`, an override for `close`
2087+
is also required.
20722088
20732089
Like {fs.ReadStream}, if `fd` is specified, {fs.WriteStream} will ignore the
20742090
`path` argument and will use the specified file descriptor. This means that no
Collapse file

‎lib/internal/fs/streams.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/streams.js
+58-56Lines changed: 58 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -120,29 +120,29 @@ function close(stream, err, cb) {
120120
}
121121

122122
function importFd(stream, options) {
123-
stream.fd = null;
124-
if (options.fd != null) {
125-
if (typeof options.fd === 'number') {
126-
// When fd is a raw descriptor, we must keep our fingers crossed
127-
// that the descriptor won't get closed, or worse, replaced with
128-
// another one
129-
// https://github.com/nodejs/node/issues/35862
130-
stream.fd = options.fd;
131-
} else if (typeof options.fd === 'object' &&
132-
options.fd instanceof FileHandle) {
133-
// When fd is a FileHandle we can listen for 'close' events
134-
if (options.fs)
135-
// FileHandle is not supported with custom fs operations
136-
throw new ERR_METHOD_NOT_IMPLEMENTED('FileHandle with fs');
137-
stream[kHandle] = options.fd;
138-
stream.fd = options.fd.fd;
139-
stream[kFs] = FileHandleOperations(stream[kHandle]);
140-
stream[kHandle][kRef]();
141-
options.fd.on('close', FunctionPrototypeBind(stream.close, stream));
142-
} else
143-
throw ERR_INVALID_ARG_TYPE('options.fd',
144-
['number', 'FileHandle'], options.fd);
123+
if (typeof options.fd === 'number') {
124+
// When fd is a raw descriptor, we must keep our fingers crossed
125+
// that the descriptor won't get closed, or worse, replaced with
126+
// another one
127+
// https://github.com/nodejs/node/issues/35862
128+
stream[kFs] = options.fs || fs;
129+
return options.fd;
130+
} else if (typeof options.fd === 'object' &&
131+
options.fd instanceof FileHandle) {
132+
// When fd is a FileHandle we can listen for 'close' events
133+
if (options.fs) {
134+
// FileHandle is not supported with custom fs operations
135+
throw new ERR_METHOD_NOT_IMPLEMENTED('FileHandle with fs');
136+
}
137+
stream[kHandle] = options.fd;
138+
stream[kFs] = FileHandleOperations(stream[kHandle]);
139+
stream[kHandle][kRef]();
140+
options.fd.on('close', FunctionPrototypeBind(stream.close, stream));
141+
return options.fd.fd;
145142
}
143+
144+
throw ERR_INVALID_ARG_TYPE('options.fd',
145+
['number', 'FileHandle'], options.fd);
146146
}
147147

148148
function ReadStream(path, options) {
@@ -158,21 +158,29 @@ function ReadStream(path, options) {
158158
options.autoDestroy = false;
159159
}
160160

161-
this[kFs] = options.fs || fs;
161+
if (options.fd == null) {
162+
this.fd = null;
163+
this[kFs] = options.fs || fs;
164+
validateFunction(this[kFs].open, 'options.fs.open');
162165

163-
validateFunction(this[kFs].open, 'options.fs.open');
164-
validateFunction(this[kFs].read, 'options.fs.read');
165-
validateFunction(this[kFs].close, 'options.fs.close');
166+
// Path will be ignored when fd is specified, so it can be falsy
167+
this.path = toPathIfFileURL(path);
168+
this.flags = options.flags === undefined ? 'r' : options.flags;
169+
this.mode = options.mode === undefined ? 0o666 : options.mode;
170+
171+
validatePath(this.path);
172+
} else {
173+
this.fd = getValidatedFd(importFd(this, options));
174+
}
166175

167176
options.autoDestroy = options.autoClose === undefined ?
168177
true : options.autoClose;
169178

170-
// Path will be ignored when fd is specified, so it can be falsy
171-
this.path = toPathIfFileURL(path);
172-
this.flags = options.flags === undefined ? 'r' : options.flags;
173-
this.mode = options.mode === undefined ? 0o666 : options.mode;
179+
validateFunction(this[kFs].read, 'options.fs.read');
174180

175-
importFd(this, options);
181+
if (options.autoDestroy) {
182+
validateFunction(this[kFs].close, 'options.fs.close');
183+
}
176184

177185
this.start = options.start;
178186
this.end = options.end;
@@ -187,12 +195,6 @@ function ReadStream(path, options) {
187195
this.pos = this.start;
188196
}
189197

190-
// If fd has been set, validate, otherwise validate path.
191-
if (this.fd != null) {
192-
this.fd = getValidatedFd(this.fd);
193-
} else {
194-
validatePath(this.path);
195-
}
196198

197199
if (this.end === undefined) {
198200
this.end = Infinity;
@@ -310,9 +312,23 @@ function WriteStream(path, options) {
310312
// Only buffers are supported.
311313
options.decodeStrings = true;
312314

313-
this[kFs] = options.fs || fs;
315+
if (options.fd == null) {
316+
this.fd = null;
317+
this[kFs] = options.fs || fs;
318+
validateFunction(this[kFs].open, 'options.fs.open');
319+
320+
// Path will be ignored when fd is specified, so it can be falsy
321+
this.path = toPathIfFileURL(path);
322+
this.flags = options.flags === undefined ? 'w' : options.flags;
323+
this.mode = options.mode === undefined ? 0o666 : options.mode;
324+
325+
validatePath(this.path);
326+
} else {
327+
this.fd = getValidatedFd(importFd(this, options));
328+
}
314329

315-
validateFunction(this[kFs].open, 'options.fs.open');
330+
options.autoDestroy = options.autoClose === undefined ?
331+
true : options.autoClose;
316332

317333
if (!this[kFs].write && !this[kFs].writev) {
318334
throw new ERR_INVALID_ARG_TYPE('options.fs.write', 'function',
@@ -327,7 +343,9 @@ function WriteStream(path, options) {
327343
validateFunction(this[kFs].writev, 'options.fs.writev');
328344
}
329345

330-
validateFunction(this[kFs].close, 'options.fs.close');
346+
if (options.autoDestroy) {
347+
validateFunction(this[kFs].close, 'options.fs.close');
348+
}
331349

332350
// It's enough to override either, in which case only one will be used.
333351
if (!this[kFs].write) {
@@ -337,28 +355,12 @@ function WriteStream(path, options) {
337355
this._writev = null;
338356
}
339357

340-
options.autoDestroy = options.autoClose === undefined ?
341-
true : options.autoClose;
342-
343-
// Path will be ignored when fd is specified, so it can be falsy
344-
this.path = toPathIfFileURL(path);
345-
this.flags = options.flags === undefined ? 'w' : options.flags;
346-
this.mode = options.mode === undefined ? 0o666 : options.mode;
347-
348-
importFd(this, options);
349-
350358
this.start = options.start;
351359
this.pos = undefined;
352360
this.bytesWritten = 0;
353361
this.closed = false;
354362
this[kIsPerformingIO] = false;
355363

356-
// If fd has been set, validate, otherwise validate path.
357-
if (this.fd != null) {
358-
this.fd = getValidatedFd(this.fd);
359-
} else {
360-
validatePath(this.path);
361-
}
362364

363365
if (this.start !== undefined) {
364366
validateInteger(this.start, 'start', 0);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.