diff options
| -rw-r--r-- | doc/api/deprecations.md | 20 | ||||
| -rw-r--r-- | lib/internal/fs/streams.js | 65 | ||||
| -rw-r--r-- | test/parallel/test-fs-read-stream-patch-open.js | 15 | ||||
| -rw-r--r-- | test/parallel/test-fs-write-stream-patch-open.js | 34 |
4 files changed, 113 insertions, 21 deletions
diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index aaec4a3c99..8b36e0c3aa 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2532,6 +2532,22 @@ Type: Documentation-only (supports [`--pending-deprecation`][]) The `process._tickCallback` property was never documented as an officially supported API. +<a id="DEP0XXX"></a> +### DEP0XXX: `WriteStream.open()` and `ReadStream.open()` are internal +<!-- YAML +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/29061 + description: Runtime deprecation. +--> + +Type: Runtime + +[`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. + [`--pending-deprecation`]: cli.html#cli_pending_deprecation [`--throw-deprecation`]: cli.html#cli_throw_deprecation [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size @@ -2542,10 +2558,12 @@ an officially supported API. [`Decipher`]: crypto.html#crypto_class_decipher [`EventEmitter.listenerCount(emitter, eventName)`]: events.html#events_eventemitter_listenercount_emitter_eventname [`REPLServer.clearBufferedCommand()`]: repl.html#repl_replserver_clearbufferedcommand +[`ReadStream.open()`]: fs.html#fs_class_fs_readstream [`Server.connections`]: net.html#net_server_connections [`Server.getConnections()`]: net.html#net_server_getconnections_callback [`Server.listen({fd: <number>})`]: net.html#net_server_listen_handle_backlog_callback [`SlowBuffer`]: buffer.html#buffer_class_slowbuffer +[`WriteStream.open()`]: fs.html#fs_class_fs_writestream [`assert`]: assert.html [`asyncResource.runInAsyncScope()`]: async_hooks.html#async_hooks_asyncresource_runinasyncscope_fn_thisarg_args [`child_process`]: child_process.html @@ -2568,6 +2586,8 @@ an officially supported API. [`ecdh.setPublicKey()`]: crypto.html#crypto_ecdh_setpublickey_publickey_encoding [`emitter.listenerCount(eventName)`]: events.html#events_emitter_listenercount_eventname [`fs.access()`]: fs.html#fs_fs_access_path_mode_callback +[`fs.createReadStream()`]: fs.html#fs_fs_createreadstream_path_options +[`fs.createWriteStream()`]: fs.html#fs_fs_createwritestream_path_options [`fs.exists(path, callback)`]: fs.html#fs_fs_exists_path_callback [`fs.lchmod(path, mode, callback)`]: fs.html#fs_fs_lchmod_path_mode_callback [`fs.lchmodSync(path, mode)`]: fs.html#fs_fs_lchmodsync_path_mode diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 110ad11493..3674a71f92 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -5,6 +5,7 @@ const { Math, Object } = primordials; const { ERR_OUT_OF_RANGE } = require('internal/errors').codes; +const internalUtil = require('internal/util'); const { validateNumber } = require('internal/validators'); const fs = require('fs'); const { Buffer } = require('buffer'); @@ -100,7 +101,7 @@ function ReadStream(path, options) { } if (typeof this.fd !== 'number') - this.open(); + _openReadFs(this); this.on('end', function() { if (this.autoClose) { @@ -111,23 +112,34 @@ function ReadStream(path, options) { Object.setPrototypeOf(ReadStream.prototype, Readable.prototype); Object.setPrototypeOf(ReadStream, Readable); -ReadStream.prototype.open = function() { - fs.open(this.path, this.flags, this.mode, (er, fd) => { +const openReadFs = internalUtil.deprecate(function() { + _openReadFs(this); +}, 'ReadStream.prototype.open() is deprecated', 'DEP0XXX'); +ReadStream.prototype.open = openReadFs; + +function _openReadFs(stream) { + // Backwards compat for overriden open. + if (stream.open !== openReadFs) { + stream.open(); + return; + } + + fs.open(stream.path, stream.flags, stream.mode, (er, fd) => { if (er) { - if (this.autoClose) { - this.destroy(); + if (stream.autoClose) { + stream.destroy(); } - this.emit('error', er); + stream.emit('error', er); return; } - this.fd = fd; - this.emit('open', fd); - this.emit('ready'); + stream.fd = fd; + stream.emit('open', fd); + stream.emit('ready'); // Start the flow of data. - this.read(); + stream.read(); }); -}; +} ReadStream.prototype._read = function(n) { if (typeof this.fd !== 'number') { @@ -266,7 +278,7 @@ function WriteStream(path, options) { this.setDefaultEncoding(options.encoding); if (typeof this.fd !== 'number') - this.open(); + _openWriteFs(this); } Object.setPrototypeOf(WriteStream.prototype, Writable.prototype); Object.setPrototypeOf(WriteStream, Writable); @@ -279,21 +291,32 @@ WriteStream.prototype._final = function(callback) { callback(); }; -WriteStream.prototype.open = function() { - fs.open(this.path, this.flags, this.mode, (er, fd) => { +const openWriteFs = internalUtil.deprecate(function() { + _openWriteFs(this); +}, 'WriteStream.prototype.open() is deprecated', 'DEP0XXX'); +WriteStream.prototype.open = openWriteFs; + +function _openWriteFs(stream) { + // Backwards compat for overriden open. + if (stream.open !== openWriteFs) { + stream.open(); + return; + } + + fs.open(stream.path, stream.flags, stream.mode, (er, fd) => { if (er) { - if (this.autoClose) { - this.destroy(); + if (stream.autoClose) { + stream.destroy(); } - this.emit('error', er); + stream.emit('error', er); return; } - this.fd = fd; - this.emit('open', fd); - this.emit('ready'); + stream.fd = fd; + stream.emit('open', fd); + stream.emit('ready'); }); -}; +} WriteStream.prototype._write = function(data, encoding, cb) { diff --git a/test/parallel/test-fs-read-stream-patch-open.js b/test/parallel/test-fs-read-stream-patch-open.js new file mode 100644 index 0000000000..80f8888de3 --- /dev/null +++ b/test/parallel/test-fs-read-stream-patch-open.js @@ -0,0 +1,15 @@ +'use strict'; +const common = require('../common'); +const fs = require('fs'); + +common.expectWarning( + 'DeprecationWarning', + 'ReadStream.prototype.open() is deprecated', 'DEP0XXX'); +const s = fs.createReadStream('asd') + // We don't care about errors in this test. + .on('error', () => {}); +s.open(); + +// Allow overriding open(). +fs.ReadStream.prototype.open = common.mustCall(); +fs.createReadStream('asd'); diff --git a/test/parallel/test-fs-write-stream-patch-open.js b/test/parallel/test-fs-write-stream-patch-open.js new file mode 100644 index 0000000000..1c82e80213 --- /dev/null +++ b/test/parallel/test-fs-write-stream-patch-open.js @@ -0,0 +1,34 @@ +'use strict'; +const common = require('../common'); +const fs = require('fs'); + +const tmpdir = require('../common/tmpdir'); + +// Run in a child process because 'out' is opened twice, blocking the tmpdir +// and preventing cleanup. +if (process.argv[2] !== 'child') { + // Parent + const assert = require('assert'); + const { fork } = require('child_process'); + tmpdir.refresh(); + + // Run test + const child = fork(__filename, ['child'], { stdio: 'inherit' }); + child.on('exit', common.mustCall(function(code) { + assert.strictEqual(code, 0); + })); + + return; +} + +// Child + +common.expectWarning( + 'DeprecationWarning', + 'WriteStream.prototype.open() is deprecated', 'DEP0XXX'); +const s = fs.createWriteStream(`${tmpdir.path}/out`); +s.open(); + +// Allow overriding open(). +fs.WriteStream.prototype.open = common.mustCall(); +fs.createWriteStream('asd'); |