From 8988e1e1170be7ea172b9e74480050d385b70dcb Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 25 Nov 2015 22:27:29 +0100 Subject: [PATCH] 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 --- lib/_debugger.js | 2 +- lib/internal/module.js | 26 ++++++++++++++++++++++- lib/module.js | 43 +++++++------------------------------- lib/repl.js | 6 +++++- src/node.js | 2 +- test/parallel/test-repl.js | 4 ++++ 6 files changed, 44 insertions(+), 39 deletions(-) diff --git a/lib/_debugger.js b/lib/_debugger.js index ba8ee0f30d..f76bf6e2ef 100644 --- 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 7f3a39e539..ef55aa64bd 100644 --- 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 8b9b59551e..6a4cd885a3 100644 --- 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,27 +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); - }; - - require.main = process.mainModule; - - // Enable support to add extra extension types - require.extensions = Module._extensions; - - require.cache = Module._cache; - - var dirname = path.dirname(filename); - // create wrapper function var wrapper = Module.wrap(content); @@ -421,8 +392,10 @@ 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); + const args = [this.exports, require, this, filename, dirname]; + return compiledWrapper.apply(this.exports, args); }; @@ -488,10 +461,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 bd3a75d88e..74c7729fdc 100644 --- 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 Interface = require('readline').Interface; const Console = require('console').Console; +const Module = require('module'); const domain = require('domain'); const debug = util.debuglog('repl'); @@ -522,6 +524,8 @@ REPLServer.prototype.createContext = function() { context.global.global = context; } + const module = new Module(''); + const require = internalModule.makeRequireFunction.call(module); context.module = module; context.require = require; @@ -661,7 +665,7 @@ REPLServer.prototype.complete = function(line, callback) { completionGroupsLoaded(); } else if (match = line.match(requireRE)) { // require('...') - 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 ca1312bb4b..1cf03972fe 100644 --- a/src/node.js +++ b/src/node.js @@ -144,7 +144,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 5234d8e009..03792789e8 100644 --- 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'/ }, ]); }