From 8e16093b646811c5b8f2624297e3667388f87352 Mon Sep 17 00:00:00 2001 From: ZYSzys Date: Sat, 7 Dec 2019 21:19:57 +0800 Subject: [PATCH] module: fix require in node repl Fixes: https://github.com/nodejs/node/issues/30808 PR-URL: https://github.com/nodejs/node/pull/30835 Reviewed-By: Ruben Bridgewater Reviewed-By: Guy Bedford Reviewed-By: Rich Trott --- lib/internal/modules/cjs/loader.js | 6 +- test/parallel/test-repl-require.js | 81 +++++++++++++++++++-------- test/parallel/test-require-resolve.js | 27 +++++++++ 3 files changed, 87 insertions(+), 27 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 10fbbb8d01e90e..ae9cfff52e9453 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -813,11 +813,11 @@ Module._resolveLookupPaths = function(request, parent) { return paths.length > 0 ? paths : null; } - // With --eval, parent.id is not set and parent.filename is null. + // In REPL, parent.filename is null. if (!parent || !parent.id || !parent.filename) { // Make require('./path/to/foo') work - normally the path is taken - // from realpath(__filename) but with eval there is no filename - const mainPaths = ['.'].concat(Module._nodeModulePaths('.'), modulePaths); + // from realpath(__filename) but in REPL there is no filename + const mainPaths = ['.']; debug('looking for %j in %j', request, mainPaths); return mainPaths; diff --git a/test/parallel/test-repl-require.js b/test/parallel/test-repl-require.js index 818fca7431fa94..391b629b1b3a3b 100644 --- a/test/parallel/test-repl-require.js +++ b/test/parallel/test-repl-require.js @@ -11,28 +11,61 @@ if (!common.isMainThread) process.chdir(fixtures.fixturesDir); const repl = require('repl'); -const server = net.createServer((conn) => { - repl.start('', conn).on('exit', () => { - conn.destroy(); - server.close(); +{ + const server = net.createServer((conn) => { + repl.start('', conn).on('exit', () => { + conn.destroy(); + server.close(); + }); }); -}); - -const host = common.localhostIPv4; -const port = 0; -const options = { host, port }; - -let answer = ''; -server.listen(options, function() { - options.port = this.address().port; - const conn = net.connect(options); - conn.setEncoding('utf8'); - conn.on('data', (data) => answer += data); - conn.write('require("baz")\nrequire("./baz")\n.exit\n'); -}); - -process.on('exit', function() { - assert.strictEqual(/Cannot find module/.test(answer), false); - assert.strictEqual(/Error/.test(answer), false); - assert.strictEqual(answer, '\'eye catcher\'\n\'perhaps I work\'\n'); -}); + + const host = common.localhostIPv4; + const port = 0; + const options = { host, port }; + + let answer = ''; + server.listen(options, function() { + options.port = this.address().port; + const conn = net.connect(options); + conn.setEncoding('utf8'); + conn.on('data', (data) => answer += data); + conn.write('require("baz")\nrequire("./baz")\n.exit\n'); + }); + + process.on('exit', function() { + assert.strictEqual(/Cannot find module/.test(answer), false); + assert.strictEqual(/Error/.test(answer), false); + assert.strictEqual(answer, '\'eye catcher\'\n\'perhaps I work\'\n'); + }); +} + +// Test for https://github.com/nodejs/node/issues/30808 +// In REPL, we shouldn't look up relative modules from 'node_modules'. +{ + const server = net.createServer((conn) => { + repl.start('', conn).on('exit', () => { + conn.destroy(); + server.close(); + }); + }); + + const host = common.localhostIPv4; + const port = 0; + const options = { host, port }; + + let answer = ''; + server.listen(options, function() { + options.port = this.address().port; + const conn = net.connect(options); + conn.setEncoding('utf8'); + conn.on('data', (data) => answer += data); + conn.write('require("./bar")\n.exit\n'); + }); + + process.on('exit', function() { + assert.strictEqual(/Uncaught Error: Cannot find module '\.\/bar'/.test(answer), true); + + assert.strictEqual(/code: 'MODULE_NOT_FOUND'/.test(answer), true); + assert.strictEqual(/requireStack: \[ '' \]/.test(answer), true); + }); +} diff --git a/test/parallel/test-require-resolve.js b/test/parallel/test-require-resolve.js index 2ffcc711854a87..484263c226f4a3 100644 --- a/test/parallel/test-require-resolve.js +++ b/test/parallel/test-require-resolve.js @@ -23,6 +23,8 @@ const common = require('../common'); const fixtures = require('../common/fixtures'); const assert = require('assert'); +const { builtinModules } = require('module'); +const path = require('path'); assert.strictEqual( require.resolve(fixtures.path('a')).toLowerCase(), @@ -52,3 +54,28 @@ const re = /^The "request" argument must be of type string\. Received type \w+$/ message: re }); }); + +// Test require.resolve.paths. +{ + // builtinModules. + builtinModules.forEach((mod) => { + assert.strictEqual(require.resolve.paths(mod), null); + }); + + // node_modules. + const resolvedPaths = require.resolve.paths('eslint'); + assert.strictEqual(Array.isArray(resolvedPaths), true); + assert.strictEqual(resolvedPaths[0].includes('node_modules'), true); + + // relativeModules. + const relativeModules = ['.', '..', './foo', '../bar']; + relativeModules.forEach((mod) => { + const resolvedPaths = require.resolve.paths(mod); + assert.strictEqual(Array.isArray(resolvedPaths), true); + assert.strictEqual(resolvedPaths.length, 1); + assert.strictEqual(resolvedPaths[0], path.dirname(__filename)); + + // Shouldn't look up relative modules from 'node_modules'. + assert.strictEqual(resolvedPaths.includes('/node_modules'), false); + }); +}