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 <[email protected]> PR-URL: #33294 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · May 16, 2020 · ab7d520 · ab7d520
1 parent ee176f1
commit ab7d520
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 27 deletions.
diff --git 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`.
 
+<a id="DEP0XX1"></a>
+### DEP0XX1: `repl._builtinLibs`
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/33294
+    description: Documentation-only (supports [`--pending-deprecation`][]).
+-->
+
+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
@@ -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.
@@ -923,22 +922,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);
@@ -993,18 +982,18 @@ REPLServer.prototype.createContext = function() {
     });
   }
 
-  const module = new CJSModule('<repl>');
-  module.paths = CJSModule._resolveLookupPaths('<repl>', parentModule);
+  const replModule = new CJSModule('<repl>');
+  replModule.paths = CJSModule._resolveLookupPaths('<repl>', 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);
@@ -1016,6 +1005,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 = [];
 
@@ -1217,7 +1207,7 @@ function complete(line, callback) {
     }
 
     if (!subdir) {
-      completionGroups.push(exports._builtinLibs);
+      completionGroups.push(_builtinLibs);
     }
 
     completionGroupsLoaded();
@@ -1610,4 +1600,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
@@ -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',