From b32f1aeecdcbeef2e8828cacae40e564bb459de5 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 8 May 2020 00:20:08 +0200 Subject: [PATCH 1/3] repl: deprecate repl.inputStream and repl.outputStream The stream is exposed twice. As such it's best to rely upon the .input and .output properties set by readline. Signed-off-by: Ruben Bridgewater --- doc/api/deprecations.md | 14 ++++ lib/repl.js | 84 +++++++++++++------ test/parallel/test-repl-history-navigation.js | 2 + test/parallel/test-repl-options.js | 6 +- 4 files changed, 79 insertions(+), 27 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 7ff8503f695aa0..23fd9f7f20667c 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2668,6 +2668,20 @@ Type: Documentation-only Use [`request.destroy()`][] instead of [`request.abort()`][]. + +### DEP0XXX: `repl.inputStream` and `repl.outputStream` + + +Type: Documentation-only (supports [`--pending-deprecation`][]) + +The `repl` module exported the input and output stream twice. Use `.input` +instead of `.inputStream` and `.output` instead of `.outputStream`. + [`--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 diff --git a/lib/repl.js b/lib/repl.js index bd7b80a91b57ea..d7c0e140204702 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -100,9 +100,11 @@ const { overrideStackTrace, } = require('internal/errors'); const { sendInspectorCommand } = require('internal/util/inspector'); -const experimentalREPLAwait = require('internal/options').getOptionValue( +const { getOptionValue } = require('internal/options'); +const experimentalREPLAwait = getOptionValue( '--experimental-repl-await' ); +const pendingDeprecation = getOptionValue('--pending-deprecation'); const { REPL_MODE_SLOPPY, REPL_MODE_STRICT, @@ -220,8 +222,39 @@ function REPLServer(prompt, const preview = options.terminal && (options.preview !== undefined ? !!options.preview : !eval_); - this.inputStream = options.input; - this.outputStream = options.output; + ObjectDefineProperty(this, 'inputStream', { + get: pendingDeprecation ? + deprecate(() => this.input, + 'repl.inputStream and repl.outputStream is deprecated. ' + + 'Use repl.input and repl.output instead', + 'DEP0XXX') : + () => this.input, + set: pendingDeprecation ? + deprecate((val) => this.input = val, + 'repl.inputStream and repl.outputStream is deprecated. ' + + 'Use repl.input and repl.output instead', + 'DEP0XXX') : + (val) => this.input = val, + enumerable: false, + configurable: true + }); + ObjectDefineProperty(this, 'outputStream', { + get: pendingDeprecation ? + deprecate(() => this.output, + 'repl.inputStream and repl.outputStream is deprecated. ' + + 'Use repl.input and repl.output instead', + 'DEP0XXX') : + () => this.output, + set: pendingDeprecation ? + deprecate((val) => this.output = val, + 'repl.inputStream and repl.outputStream is deprecated. ' + + 'Use repl.input and repl.output instead', + 'DEP0XXX') : + (val) => this.output = val, + enumerable: false, + configurable: true + }); + this.useColors = !!options.useColors; this._domain = options.domain || domain.create(); this.useGlobal = !!useGlobal; @@ -596,16 +629,13 @@ function REPLServer(prompt, } // Normalize line endings. errStack += errStack.endsWith('\n') ? '' : '\n'; - self.outputStream.write(errStack); + self.output.write(errStack); self.clearBufferedCommand(); self.lines.level = []; self.displayPrompt(); } }); - self.resetContext(); - self.lines.level = []; - self.clearBufferedCommand(); ObjectDefineProperty(this, 'bufferedCommand', { get: deprecate(() => self[kBufferedCommandSymbol], @@ -627,14 +657,16 @@ function REPLServer(prompt, } Interface.call(this, { - input: self.inputStream, - output: self.outputStream, + input: options.input, + output: options.output, completer: self.completer, terminal: options.terminal, historySize: options.historySize, prompt }); + self.resetContext(); + this.commands = ObjectCreate(null); defineDefaultCommands(this); @@ -752,7 +784,7 @@ function REPLServer(prompt, return; } if (!self[kBufferedCommandSymbol]) { - self.outputStream.write('Invalid REPL keyword\n'); + self.output.write('Invalid REPL keyword\n'); finish(null); return; } @@ -769,7 +801,7 @@ function REPLServer(prompt, _memory.call(self, cmd); if (e && !self[kBufferedCommandSymbol] && cmd.trim().startsWith('npm ')) { - self.outputStream.write('npm should be run outside of the ' + + self.output.write('npm should be run outside of the ' + 'node repl, in your normal shell.\n' + '(Press Control-D to exit.)\n'); self.displayPrompt(); @@ -805,7 +837,7 @@ function REPLServer(prompt, if (!self.underscoreAssigned) { self.last = ret; } - self.outputStream.write(self.writer(ret) + '\n'); + self.output.write(self.writer(ret) + '\n'); } // Display prompt again @@ -815,10 +847,10 @@ function REPLServer(prompt, self.on('SIGCONT', function onSigCont() { if (self.editorMode) { - self.outputStream.write(`${self._initialPrompt}.editor\n`); - self.outputStream.write( + self.output.write(`${self._initialPrompt}.editor\n`); + self.output.write( '// Entering editor mode (^D to finish, ^C to cancel)\n'); - self.outputStream.write(`${self[kBufferedCommandSymbol]}\n`); + self.output.write(`${self[kBufferedCommandSymbol]}\n`); self.prompt(true); } else { self.displayPrompt(true); @@ -958,7 +990,7 @@ REPLServer.prototype.createContext = function() { } } context.global = context; - const _console = new Console(this.outputStream); + const _console = new Console(this.output); ObjectDefineProperty(context, 'console', { configurable: true, writable: true, @@ -999,7 +1031,7 @@ REPLServer.prototype.resetContext = function() { this.last = value; if (!this.underscoreAssigned) { this.underscoreAssigned = true; - this.outputStream.write('Expression assignment to _ now disabled.\n'); + this.output.write('Expression assignment to _ now disabled.\n'); } } }); @@ -1011,7 +1043,7 @@ REPLServer.prototype.resetContext = function() { this.lastError = value; if (!this.underscoreErrAssigned) { this.underscoreErrAssigned = true; - this.outputStream.write( + this.output.write( 'Expression assignment to _error now disabled.\n'); } } @@ -1496,7 +1528,7 @@ function defineDefaultCommands(repl) { action: function() { this.clearBufferedCommand(); if (!this.useGlobal) { - this.outputStream.write('Clearing context...\n'); + this.output.write('Clearing context...\n'); this.resetContext(); } this.displayPrompt(); @@ -1523,9 +1555,9 @@ function defineDefaultCommands(repl) { const cmd = this.commands[name]; const spaces = ' '.repeat(longestNameLength - name.length + 3); const line = `.${name}${cmd.help ? spaces + cmd.help : ''}\n`; - this.outputStream.write(line); + this.output.write(line); } - this.outputStream.write('\nPress ^C to abort current expression, ' + + this.output.write('\nPress ^C to abort current expression, ' + '^D to exit the repl\n'); this.displayPrompt(); } @@ -1536,9 +1568,9 @@ function defineDefaultCommands(repl) { action: function(file) { try { fs.writeFileSync(file, this.lines.join('\n')); - this.outputStream.write(`Session saved to: ${file}\n`); + this.output.write(`Session saved to: ${file}\n`); } catch { - this.outputStream.write(`Failed to save: ${file}\n`); + this.output.write(`Failed to save: ${file}\n`); } this.displayPrompt(); } @@ -1556,12 +1588,12 @@ function defineDefaultCommands(repl) { _turnOffEditorMode(this); this.write('\n'); } else { - this.outputStream.write( + this.output.write( `Failed to load: ${file} is not a valid file\n` ); } } catch { - this.outputStream.write(`Failed to load: ${file}\n`); + this.output.write(`Failed to load: ${file}\n`); } this.displayPrompt(); } @@ -1571,7 +1603,7 @@ function defineDefaultCommands(repl) { help: 'Enter editor mode', action() { _turnOnEditorMode(this); - this.outputStream.write( + this.output.write( '// Entering editor mode (^D to finish, ^C to cancel)\n'); } }); diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 275e92821a6bbf..9155ad722b8710 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -15,6 +15,8 @@ common.skipIfDumbTerminal(); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); +process.throwDeprecation = true; + const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history'); // Create an input stream specialized for testing an array of actions diff --git a/test/parallel/test-repl-options.js b/test/parallel/test-repl-options.js index 99f9fa8a605142..a9d3315eb317f5 100644 --- a/test/parallel/test-repl-options.js +++ b/test/parallel/test-repl-options.js @@ -19,6 +19,8 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. +// Flags: --pending-deprecation + 'use strict'; const common = require('../common'); const ArrayStream = require('../common/arraystream'); @@ -30,7 +32,9 @@ assert.strictEqual(repl.repl, undefined); common.expectWarning({ DeprecationWarning: { - DEP0124: 'REPLServer.rli is deprecated' + DEP0XXX: 'repl.inputStream and repl.outputStream is deprecated. ' + + 'Use repl.input and repl.output instead', + DEP0124: 'REPLServer.rli is deprecated', } }); From bae3a62b2bc2d86b13af74220a4ea1220978e355 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 8 May 2020 00:37:33 +0200 Subject: [PATCH 2/3] repl: remove obsolete completer variable It is also assigned in readline. There is not need to assign the variable twice. Signed-off-by: Ruben Bridgewater --- lib/repl.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index d7c0e140204702..6fa76e588ed1de 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -647,10 +647,6 @@ function REPLServer(prompt, enumerable: true }); - // Figure out which "complete" function to use. - self.completer = (typeof options.completer === 'function') ? - options.completer : completer; - function completer(text, cb) { complete.call(self, text, self.editorMode ? self.completeOnEditorMode(cb) : cb); @@ -659,7 +655,7 @@ function REPLServer(prompt, Interface.call(this, { input: options.input, output: options.output, - completer: self.completer, + completer: options.completer || completer, terminal: options.terminal, historySize: options.historySize, prompt From 369d494c1d107d4a54a617422133704f67531858 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 8 May 2020 00:35:15 +0200 Subject: [PATCH 3/3] repl: deprecate repl._builtinLibs This is a manually edited and outdated list of builtin modules. Instead, it is better to rely upon the officially documented way to get a list of builtin modules. As a side by fix this makes sure all exports are in one place. Thus, it is easier to see what parts are actually exported and which are not. Signed-off-by: Ruben Bridgewater --- doc/api/deprecations.md | 15 +++++++ lib/repl.js | 67 ++++++++++++++++++------------ test/parallel/test-repl-options.js | 3 ++ 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 23fd9f7f20667c..35686572888720 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2682,6 +2682,21 @@ Type: Documentation-only (supports [`--pending-deprecation`][]) The `repl` module exported the input and output stream twice. Use `.input` instead of `.inputStream` and `.output` instead of `.outputStream`. + +### DEP0XX1: `repl._builtinLibs` + + +Type: Documentation-only + +The `repl` module exports a `_builtinLibs` property that contains an array with +native modules. It was incomplete so far and instead it's better to rely upon +`require('module').builtinModules`. + [`--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 diff --git a/lib/repl.js b/lib/repl.js index 6fa76e588ed1de..b738d689e14abb 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -85,7 +85,7 @@ const { } = require('internal/readline/utils'); const { Console } = require('console'); const CJSModule = require('internal/modules/cjs/loader').Module; -const builtinModules = [...CJSModule.builtinModules] +let _builtinLibs = [...CJSModule.builtinModules] .filter((e) => !e.startsWith('_')); const domain = require('domain'); const debug = require('internal/util/debuglog').debuglog('repl'); @@ -158,11 +158,9 @@ module.paths = CJSModule._nodeModulePaths(module.filename); // This is the default "writer" value, if none is passed in the REPL options, // and it can be overridden by custom print functions, such as `probe` or // `eyes.js`. -const writer = exports.writer = (obj) => inspect(obj, writer.options); +const writer = (obj) => inspect(obj, writer.options); writer.options = { ...inspect.defaultOptions, showProxy: true }; -exports._builtinLibs = builtinModules; - function REPLServer(prompt, stream, eval_, @@ -259,7 +257,7 @@ function REPLServer(prompt, this._domain = options.domain || domain.create(); this.useGlobal = !!useGlobal; this.ignoreUndefined = !!ignoreUndefined; - this.replMode = replMode || exports.REPL_MODE_SLOPPY; + this.replMode = replMode || module.exports.REPL_MODE_SLOPPY; this.underscoreAssigned = false; this.last = undefined; this.underscoreErrAssigned = false; @@ -278,7 +276,7 @@ function REPLServer(prompt, if (options[kStandaloneREPL]) { // It is possible to introspect the running REPL accessing this variable // from inside the REPL. This is useful for anyone working on the REPL. - exports.repl = this; + module.exports.repl = this; } else if (!addedNewListener) { // Add this listener only once and use a WeakSet that contains the REPLs // domains. Otherwise we'd have to add a single listener to each REPL @@ -399,7 +397,8 @@ function REPLServer(prompt, } while (true) { try { - if (self.replMode === exports.REPL_MODE_STRICT && !/^\s*$/.test(code)) { + if (self.replMode === module.exports.REPL_MODE_STRICT && + !/^\s*$/.test(code)) { // "void 0" keeps the repl from returning "use strict" as the result // value for statements and declarations that don't return a value. code = `'use strict'; void 0;\n${code}`; @@ -579,7 +578,7 @@ function REPLServer(prompt, e.stack = e.stack .replace(/^repl:\d+\r?\n/, '') .replace(/^\s+at\s.*\n?/gm, ''); - } else if (self.replMode === exports.REPL_MODE_STRICT) { + } else if (self.replMode === module.exports.REPL_MODE_STRICT) { e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/, (_, pre, line) => pre + (line - 1)); } @@ -667,7 +666,7 @@ function REPLServer(prompt, defineDefaultCommands(this); // Figure out which "writer" function to use - self.writer = options.writer || exports.writer; + self.writer = options.writer || module.exports.writer; if (self.writer === writer) { // Conditionally turn on ANSI coloring. @@ -924,22 +923,12 @@ function REPLServer(prompt, ObjectSetPrototypeOf(REPLServer.prototype, Interface.prototype); ObjectSetPrototypeOf(REPLServer, Interface); -exports.REPLServer = REPLServer; - -exports.REPL_MODE_SLOPPY = REPL_MODE_SLOPPY; -exports.REPL_MODE_STRICT = REPL_MODE_STRICT; - // Prompt is a string to print on each line for the prompt, // source is a stream to use for I/O, defaulting to stdin/stdout. -exports.start = function(prompt, - source, - eval_, - useGlobal, - ignoreUndefined, - replMode) { +function start(prompt, source, eval_, useGlobal, ignoreUndefined, replMode) { return new REPLServer( prompt, source, eval_, useGlobal, ignoreUndefined, replMode); -}; +} REPLServer.prototype.setupHistory = function setupHistory(historyFile, cb) { history(this, historyFile, cb); @@ -994,18 +983,18 @@ REPLServer.prototype.createContext = function() { }); } - const module = new CJSModule(''); - module.paths = CJSModule._resolveLookupPaths('', parentModule); + const replModule = new CJSModule(''); + replModule.paths = CJSModule._resolveLookupPaths('', parentModule); ObjectDefineProperty(context, 'module', { configurable: true, writable: true, - value: module + value: replModule }); ObjectDefineProperty(context, 'require', { configurable: true, writable: true, - value: makeRequireFunction(module) + value: makeRequireFunction(replModule) }); addBuiltinLibsToObject(context); @@ -1017,6 +1006,7 @@ REPLServer.prototype.resetContext = function() { this.context = this.createContext(); this.underscoreAssigned = false; this.underscoreErrAssigned = false; + // TODO(BridgeAR): Deprecate the lines. this.lines = []; this.lines.level = []; @@ -1218,7 +1208,7 @@ function complete(line, callback) { } if (!subdir) { - completionGroups.push(exports._builtinLibs); + completionGroups.push(_builtinLibs); } completionGroupsLoaded(); @@ -1611,4 +1601,27 @@ function Recoverable(err) { } ObjectSetPrototypeOf(Recoverable.prototype, SyntaxError.prototype); ObjectSetPrototypeOf(Recoverable, SyntaxError); -exports.Recoverable = Recoverable; + +module.exports = { + start, + writer, + REPLServer, + REPL_MODE_SLOPPY, + REPL_MODE_STRICT, + Recoverable +}; + +ObjectDefineProperty(module.exports, '_builtinLibs', { + get: pendingDeprecation ? deprecate( + () => _builtinLibs, + 'repl._builtinLibs is deprecated. Check module.builtinModules instead', + 'DEP0XX1' + ) : () => _builtinLibs, + set: pendingDeprecation ? deprecate( + (val) => _builtinLibs = val, + 'repl._builtinLibs is deprecated. Check module.builtinModules instead', + 'DEP0XX1' + ) : (val) => _builtinLibs = val, + enumerable: false, + configurable: true +}); diff --git a/test/parallel/test-repl-options.js b/test/parallel/test-repl-options.js index a9d3315eb317f5..a7e8e663d4ed94 100644 --- a/test/parallel/test-repl-options.js +++ b/test/parallel/test-repl-options.js @@ -29,9 +29,12 @@ const repl = require('repl'); const cp = require('child_process'); assert.strictEqual(repl.repl, undefined); +repl._builtinLibs; common.expectWarning({ DeprecationWarning: { + DEP0XX1: + 'repl._builtinLibs is deprecated. Check module.builtinModules instead', DEP0XXX: 'repl.inputStream and repl.outputStream is deprecated. ' + 'Use repl.input and repl.output instead', DEP0124: 'REPLServer.rli is deprecated',