Skip to content

Commit

Permalink
module: fix require in node repl
Browse files Browse the repository at this point in the history
Fixes: #30808

PR-URL: #30835
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
ZYSzys authored and targos committed Jan 14, 2020
1 parent 01ea5ba commit ffc910f
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 27 deletions.
6 changes: 3 additions & 3 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -810,11 +810,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;
Expand Down
81 changes: 57 additions & 24 deletions test/parallel/test-repl-require.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: \[ '<repl>' \]/.test(answer), true);
});
}
27 changes: 27 additions & 0 deletions test/parallel/test-require-resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
});
}

0 comments on commit ffc910f

Please sign in to comment.