module,repl: remove repl require() hack

Remove a hack that was introduced in commit bb6d468d from November 2010.
This is groundwork for a follow-up commit that makes it possible to use
internal modules in lib/repl.js.

PR-URL: https://github.com/nodejs/node/pull/4026
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Conflicts:
	lib/module.js

diff --git a/lib/_debugger.js b/lib/_debugger.js
index dd47cbf..ad7ee62 100644 (file)
--- a/lib/_debugger.js
+++ b/lib/_debugger.js
@@ -5,7 +5,7 @@ const path = require('path');
 const net = require('net');
 const vm = require('vm');
 const Module = require('module');
-const repl = Module.requireRepl();
+const repl = require('repl');
 const inherits = util.inherits;
 const assert = require('assert');
 const spawn = require('child_process').spawn;

diff --git a/lib/internal/module.js b/lib/internal/module.js
index 7f3a39e..ef55aa6 100644 (file)
--- a/lib/internal/module.js
+++ b/lib/internal/module.js
@@ -1,6 +1,30 @@
 'use strict';
 
-module.exports.stripBOM = stripBOM;
+module.exports = { makeRequireFunction, stripBOM };
+
+// Invoke with makeRequireFunction.call(module) where |module| is the
+// Module object to use as the context for the require() function.
+function makeRequireFunction() {
+  const Module = this.constructor;
+  const self = this;
+
+  function require(path) {
+    return self.require(path);
+  }
+
+  require.resolve = function(request) {
+    return Module._resolveFilename(request, self);
+  };
+
+  require.main = process.mainModule;
+
+  // Enable support to add extra extension types.
+  require.extensions = Module._extensions;
+
+  require.cache = Module._cache;
+
+  return require;
+}
 
 /**
  * Remove byte order marker. This catches EF BB BF (the UTF-8 BOM)

diff --git a/lib/module.js b/lib/module.js
index b3d29ff..99c4762 100644 (file)
--- a/lib/module.js
+++ b/lib/module.js
@@ -274,17 +274,6 @@ Module._load = function(request, parent, isMain) {
     debug('Module._load REQUEST %s parent: %s', request, parent.id);
   }
 
-  // REPL is a special case, because it needs the real require.
-  if (request === 'internal/repl' || request === 'repl') {
-    if (Module._cache[request]) {
-      return Module._cache[request];
-    }
-    var replModule = new Module(request);
-    replModule._compile(NativeModule.getSource(request), `${request}.js`);
-    NativeModule._cache[request] = replModule;
-    return replModule.exports;
-  }
-
   var filename = Module._resolveFilename(request, parent);
 
   var cachedModule = Module._cache[filename];
@@ -376,37 +365,9 @@ var resolvedArgv;
 // the file.
 // Returns exception, if any.
 Module.prototype._compile = function(content, filename) {
-  var self = this;
   // remove shebang
   content = content.replace(shebangRe, '');
 
-  function require(path) {
-    return self.require(path);
-  }
-
-  require.resolve = function(request) {
-    return Module._resolveFilename(request, self);
-  };
-
-  Object.defineProperty(require, 'paths', { get: function() {
-    throw new Error('require.paths is removed. Use ' +
-                    'node_modules folders, or the NODE_PATH ' +
-                    'environment variable instead.');
-  }});
-
-  require.main = process.mainModule;
-
-  // Enable support to add extra extension types
-  require.extensions = Module._extensions;
-  require.registerExtension = function() {
-    throw new Error('require.registerExtension() removed. Use ' +
-                    'require.extensions instead.');
-  };
-
-  require.cache = Module._cache;
-
-  var dirname = path.dirname(filename);
-
   // create wrapper function
   var wrapper = Module.wrap(content);
 
@@ -431,8 +392,22 @@ Module.prototype._compile = function(content, filename) {
       global.v8debug.Debug.setBreakPoint(compiledWrapper, 0, 0);
     }
   }
-  var args = [self.exports, require, self, filename, dirname];
-  return compiledWrapper.apply(self.exports, args);
+  const dirname = path.dirname(filename);
+  const require = internalModule.makeRequireFunction.call(this);
+
+  Object.defineProperty(require, 'paths', { get: function() {
+    throw new Error('require.paths is removed. Use ' +
+                    'node_modules folders, or the NODE_PATH ' +
+                    'environment variable instead.');
+  }});
+
+  require.registerExtension = function() {
+    throw new Error('require.registerExtension() removed. Use ' +
+                    'require.extensions instead.');
+  };
+
+  const args = [this.exports, require, this, filename, dirname];
+  return compiledWrapper.apply(this.exports, args);
 };
 
 
@@ -498,10 +473,10 @@ Module._initPaths = function() {
   Module.globalPaths = modulePaths.slice(0);
 };
 
-// bootstrap repl
-Module.requireRepl = function() {
-  return Module._load('internal/repl', '.');
-};
+// TODO(bnoordhuis) Unused, remove in the future.
+Module.requireRepl = internalUtil.deprecate(function() {
+  return NativeModule.require('internal/repl');
+}, 'Module.requireRepl is deprecated.');
 
 Module._preloadModules = function(requests) {
   if (!Array.isArray(requests))

diff --git a/lib/repl.js b/lib/repl.js
index 0d82a84..42fb6ad 100644 (file)
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -21,6 +21,7 @@
 
 'use strict';
 
+const internalModule = require('internal/module');
 const util = require('util');
 const inherits = util.inherits;
 const Stream = require('stream');
@@ -29,6 +30,7 @@ const path = require('path');
 const fs = require('fs');
 const rl = require('readline');
 const Console = require('console').Console;
+const Module = require('module');
 const domain = require('domain');
 const debug = util.debuglog('repl');
 
@@ -508,6 +510,8 @@ REPLServer.prototype.createContext = function() {
     context.global.global = context;
   }
 
+  const module = new Module('<repl>');
+  const require = internalModule.makeRequireFunction.call(module);
   context.module = module;
   context.require = require;
 
@@ -647,7 +651,7 @@ REPLServer.prototype.complete = function(line, callback) {
     completionGroupsLoaded();
   } else if (match = line.match(requireRE)) {
     // require('...<Tab>')
-    var exts = Object.keys(require.extensions);
+    const exts = Object.keys(this.context.require.extensions);
     var indexRe = new RegExp('^index(' + exts.map(regexpEscape).join('|') +
                              ')$');
 

diff --git a/src/node.js b/src/node.js
index a9a8580..367f68b 100644 (file)
--- a/src/node.js
+++ b/src/node.js
@@ -142,7 +142,7 @@
         // If -i or --interactive were passed, or stdin is a TTY.
         if (process._forceRepl || NativeModule.require('tty').isatty(0)) {
           // REPL
-          var cliRepl = Module.requireRepl();
+          var cliRepl = NativeModule.require('internal/repl');
           cliRepl.createInternalRepl(process.env, function(err, repl) {
             if (err) {
               throw err;

diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js
index 4d65a1e..0d05de9 100644 (file)
--- a/test/parallel/test-repl.js
+++ b/test/parallel/test-repl.js
@@ -278,6 +278,10 @@ function error_test() {
       expect: 'undefined\n' + prompt_unix },
     { client: client_unix, send: '/* \'\n"\n\'"\'\n*/',
       expect: 'undefined\n' + prompt_unix },
+    // REPL should get a normal require() function, not one that allows
+    // access to internal modules without the --expose_internals flag.
+    { client: client_unix, send: 'require("internal/repl")',
+      expect: /^Error: Cannot find module 'internal\/repl'/ },
   ]);
 }