FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: add runtime deprecate for file stream open() · nodejs/node@773769d · GitHub

/ node Public

Commit 773769d

Browse files
ronag
authored and
Trott
committed
fs: add runtime deprecate for file stream open()
WriteStream.open() and ReadStream.open() are undocumented internal APIs that do not make sense to use in userland. File streams should always be opened through their corresponding factory methods (fs.createWriteStream() and fs.createReadStream()) or by passing a file descriptor in options. PR-URL: #29061 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 039eb56 commit 773769d

4 files changed

Lines changed: 113 additions & 21 deletions

File tree

‎doc/api/deprecations.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,6 +2532,22 @@ Type: Documentation-only (supports [`--pending-deprecation`][])
25322532
The `process._tickCallback` property was never documented as
25332533
an officially supported API.
25342534
2535+
<a id="DEP0XXX"></a>
2536+
### DEP0XXX: `WriteStream.open()` and `ReadStream.open()` are internal
2537+
<!-- YAML
2538+
changes:
2539+
- version: REPLACEME
2540+
pr-url: https://github.com/nodejs/node/pull/29061
2541+
description: Runtime deprecation.
2542+
-->
2543+
2544+
Type: Runtime
2545+
2546+
[`WriteStream.open()`][] and [`ReadStream.open()`][] are undocumented internal
2547+
APIs that do not make sense to use in userland. File streams should always be
2548+
opened through their corresponding factory methods [`fs.createWriteStream()`][]
2549+
and [`fs.createReadStream()`][]) or by passing a file descriptor in options.
2550+
25352551
[`--pending-deprecation`]: cli.html#cli_pending_deprecation
25362552
[`--throw-deprecation`]: cli.html#cli_throw_deprecation
25372553
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
@@ -2542,10 +2558,12 @@ an officially supported API.
25422558
[`Decipher`]: crypto.html#crypto_class_decipher
25432559
[`EventEmitter.listenerCount(emitter, eventName)`]: events.html#events_eventemitter_listenercount_emitter_eventname
25442560
[`REPLServer.clearBufferedCommand()`]: repl.html#repl_replserver_clearbufferedcommand
2561+
[`ReadStream.open()`]: fs.html#fs_class_fs_readstream
25452562
[`Server.connections`]: net.html#net_server_connections
25462563
[`Server.getConnections()`]: net.html#net_server_getconnections_callback
25472564
[`Server.listen({fd: <number>})`]: net.html#net_server_listen_handle_backlog_callback
25482565
[`SlowBuffer`]: buffer.html#buffer_class_slowbuffer
2566+
[`WriteStream.open()`]: fs.html#fs_class_fs_writestream
25492567
[`assert`]: assert.html
25502568
[`asyncResource.runInAsyncScope()`]: async_hooks.html#async_hooks_asyncresource_runinasyncscope_fn_thisarg_args
25512569
[`child_process`]: child_process.html
@@ -2568,6 +2586,8 @@ an officially supported API.
25682586
[`ecdh.setPublicKey()`]: crypto.html#crypto_ecdh_setpublickey_publickey_encoding
25692587
[`emitter.listenerCount(eventName)`]: events.html#events_emitter_listenercount_eventname
25702588
[`fs.access()`]: fs.html#fs_fs_access_path_mode_callback
2589+
[`fs.createReadStream()`]: fs.html#fs_fs_createreadstream_path_options
2590+
[`fs.createWriteStream()`]: fs.html#fs_fs_createwritestream_path_options
25712591
[`fs.exists(path, callback)`]: fs.html#fs_fs_exists_path_callback
25722592
[`fs.lchmod(path, mode, callback)`]: fs.html#fs_fs_lchmod_path_mode_callback
25732593
[`fs.lchmodSync(path, mode)`]: fs.html#fs_fs_lchmodsync_path_mode

‎lib/internal/fs/streams.js‎

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { Math, Object } = primordials;
55
const {
66
ERR_OUT_OF_RANGE
77
} = require('internal/errors').codes;
8+
const internalUtil = require('internal/util');
89
const { validateNumber } = require('internal/validators');
910
const fs = require('fs');
1011
const { Buffer } = require('buffer');
@@ -100,7 +101,7 @@ function ReadStream(path, options) {
100101
}
101102

102103
if (typeof this.fd !== 'number')
103-
this.open();
104+
_openReadFs(this);
104105

105106
this.on('end', function() {
106107
if (this.autoClose) {
@@ -111,23 +112,34 @@ function ReadStream(path, options) {
111112
Object.setPrototypeOf(ReadStream.prototype, Readable.prototype);
112113
Object.setPrototypeOf(ReadStream, Readable);
113114

114-
ReadStream.prototype.open = function() {
115-
fs.open(this.path, this.flags, this.mode, (er, fd) => {
115+
const openReadFs = internalUtil.deprecate(function() {
116+
_openReadFs(this);
117+
}, 'ReadStream.prototype.open() is deprecated', 'DEP0XXX');
118+
ReadStream.prototype.open = openReadFs;
119+
120+
function _openReadFs(stream) {
121+
// Backwards compat for overriden open.
122+
if (stream.open !== openReadFs) {
123+
stream.open();
124+
return;
125+
}
126+
127+
fs.open(stream.path, stream.flags, stream.mode, (er, fd) => {
116128
if (er) {
117-
if (this.autoClose) {
118-
this.destroy();
129+
if (stream.autoClose) {
130+
stream.destroy();
119131
}
120-
this.emit('error', er);
132+
stream.emit('error', er);
121133
return;
122134
}
123135

124-
this.fd = fd;
125-
this.emit('open', fd);
126-
this.emit('ready');
136+
stream.fd = fd;
137+
stream.emit('open', fd);
138+
stream.emit('ready');
127139
// Start the flow of data.
128-
this.read();
140+
stream.read();
129141
});
130-
};
142+
}
131143

132144
ReadStream.prototype._read = function(n) {
133145
if (typeof this.fd !== 'number') {
@@ -266,7 +278,7 @@ function WriteStream(path, options) {
266278
this.setDefaultEncoding(options.encoding);
267279

268280
if (typeof this.fd !== 'number')
269-
this.open();
281+
_openWriteFs(this);
270282
}
271283
Object.setPrototypeOf(WriteStream.prototype, Writable.prototype);
272284
Object.setPrototypeOf(WriteStream, Writable);
@@ -279,21 +291,32 @@ WriteStream.prototype._final = function(callback) {
279291
callback();
280292
};
281293

282-
WriteStream.prototype.open = function() {
283-
fs.open(this.path, this.flags, this.mode, (er, fd) => {
294+
const openWriteFs = internalUtil.deprecate(function() {
295+
_openWriteFs(this);
296+
}, 'WriteStream.prototype.open() is deprecated', 'DEP0XXX');
297+
WriteStream.prototype.open = openWriteFs;
298+
299+
function _openWriteFs(stream) {
300+
// Backwards compat for overriden open.
301+
if (stream.open !== openWriteFs) {
302+
stream.open();
303+
return;
304+
}
305+
306+
fs.open(stream.path, stream.flags, stream.mode, (er, fd) => {
284307
if (er) {
285-
if (this.autoClose) {
286-
this.destroy();
308+
if (stream.autoClose) {
309+
stream.destroy();
287310
}
288-
this.emit('error', er);
311+
stream.emit('error', er);
289312
return;
290313
}
291314

292-
this.fd = fd;
293-
this.emit('open', fd);
294-
this.emit('ready');
315+
stream.fd = fd;
316+
stream.emit('open', fd);
317+
stream.emit('ready');
295318
});
296-
};
319+
}
297320

298321

299322
WriteStream.prototype._write = function(data, encoding, cb) {

‎test/parallel/test-fs-read-stream-patch-open.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use strict';
2+
const common = require('../common');
3+
const fs = require('fs');
4+
5+
common.expectWarning(
6+
'DeprecationWarning',
7+
'ReadStream.prototype.open() is deprecated', 'DEP0XXX');
8+
const s = fs.createReadStream('asd')
9+
// We don't care about errors in this test.
10+
.on('error', () => {});
11+
s.open();
12+
13+
// Allow overriding open().
14+
fs.ReadStream.prototype.open = common.mustCall();
15+
fs.createReadStream('asd');

‎test/parallel/test-fs-write-stream-patch-open.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
const common = require('../common');
3+
const fs = require('fs');
4+
5+
const tmpdir = require('../common/tmpdir');
6+
7+
// Run in a child process because 'out' is opened twice, blocking the tmpdir
8+
// and preventing cleanup.
9+
if (process.argv[2] !== 'child') {
10+
// Parent
11+
const assert = require('assert');
12+
const { fork } = require('child_process');
13+
tmpdir.refresh();
14+
15+
// Run test
16+
const child = fork(__filename, ['child'], { stdio: 'inherit' });
17+
child.on('exit', common.mustCall(function(code) {
18+
assert.strictEqual(code, 0);
19+
}));
20+
21+
return;
22+
}
23+
24+
// Child
25+
26+
common.expectWarning(
27+
'DeprecationWarning',
28+
'WriteStream.prototype.open() is deprecated', 'DEP0XXX');
29+
const s = fs.createWriteStream(`${tmpdir.path}/out`);
30+
s.open();
31+
32+
// Allow overriding open().
33+
fs.WriteStream.prototype.open = common.mustCall();
34+
fs.createWriteStream('asd');

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL