From 0ae90ecd3d481bd2e8f2334468bc442435f8b5c6 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 Conflicts: lib/module.js --- lib/_debugger.js | 2 +- lib/internal/module.js | 26 ++++++++++++++- lib/module.js | 65 ++++++++++++-------------------------- lib/repl.js | 6 +++- src/node.js | 2 +- test/parallel/test-repl.js | 4 +++ 6 files changed, 56 insertions(+), 49 deletions(-) diff --git a/lib/_debugger.js b/lib/_debugger.js index dd47cbff285149..ad7ee6295bee2a 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 7f3a39e539424a..ef55aa64bd5642 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 b3d29ffbd1c653..99c476210991fd 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,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 0d82a842be636a..42fb6ad3405a74 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 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(''); + 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('...') - 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 a9a85809096a6e..367f68b25c003c 100644 --- 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 4d65a1e9d084b4..0d05de9bc1863d 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'/ }, ]); }