Skip to content

Commit ffc910f

Browse files
ZYSzystargos
authored andcommitted
module: fix require in node repl
Fixes: #30808 PR-URL: #30835 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Rich Trott <[email protected]>
1 parent 01ea5ba commit ffc910f

File tree

3 files changed

+87
-27
lines changed

3 files changed

+87
-27
lines changed

lib/internal/modules/cjs/loader.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -810,11 +810,11 @@ Module._resolveLookupPaths = function(request, parent) {
810810
return paths.length > 0 ? paths : null;
811811
}
812812

813-
// With --eval, parent.id is not set and parent.filename is null.
813+
// In REPL, parent.filename is null.
814814
if (!parent || !parent.id || !parent.filename) {
815815
// Make require('./path/to/foo') work - normally the path is taken
816-
// from realpath(__filename) but with eval there is no filename
817-
const mainPaths = ['.'].concat(Module._nodeModulePaths('.'), modulePaths);
816+
// from realpath(__filename) but in REPL there is no filename
817+
const mainPaths = ['.'];
818818

819819
debug('looking for %j in %j', request, mainPaths);
820820
return mainPaths;

test/parallel/test-repl-require.js

+57-24
Original file line numberDiff line numberDiff line change
@@ -11,28 +11,61 @@ if (!common.isMainThread)
1111
process.chdir(fixtures.fixturesDir);
1212
const repl = require('repl');
1313

14-
const server = net.createServer((conn) => {
15-
repl.start('', conn).on('exit', () => {
16-
conn.destroy();
17-
server.close();
14+
{
15+
const server = net.createServer((conn) => {
16+
repl.start('', conn).on('exit', () => {
17+
conn.destroy();
18+
server.close();
19+
});
1820
});
19-
});
20-
21-
const host = common.localhostIPv4;
22-
const port = 0;
23-
const options = { host, port };
24-
25-
let answer = '';
26-
server.listen(options, function() {
27-
options.port = this.address().port;
28-
const conn = net.connect(options);
29-
conn.setEncoding('utf8');
30-
conn.on('data', (data) => answer += data);
31-
conn.write('require("baz")\nrequire("./baz")\n.exit\n');
32-
});
33-
34-
process.on('exit', function() {
35-
assert.strictEqual(/Cannot find module/.test(answer), false);
36-
assert.strictEqual(/Error/.test(answer), false);
37-
assert.strictEqual(answer, '\'eye catcher\'\n\'perhaps I work\'\n');
38-
});
21+
22+
const host = common.localhostIPv4;
23+
const port = 0;
24+
const options = { host, port };
25+
26+
let answer = '';
27+
server.listen(options, function() {
28+
options.port = this.address().port;
29+
const conn = net.connect(options);
30+
conn.setEncoding('utf8');
31+
conn.on('data', (data) => answer += data);
32+
conn.write('require("baz")\nrequire("./baz")\n.exit\n');
33+
});
34+
35+
process.on('exit', function() {
36+
assert.strictEqual(/Cannot find module/.test(answer), false);
37+
assert.strictEqual(/Error/.test(answer), false);
38+
assert.strictEqual(answer, '\'eye catcher\'\n\'perhaps I work\'\n');
39+
});
40+
}
41+
42+
// Test for https://github.com/nodejs/node/issues/30808
43+
// In REPL, we shouldn't look up relative modules from 'node_modules'.
44+
{
45+
const server = net.createServer((conn) => {
46+
repl.start('', conn).on('exit', () => {
47+
conn.destroy();
48+
server.close();
49+
});
50+
});
51+
52+
const host = common.localhostIPv4;
53+
const port = 0;
54+
const options = { host, port };
55+
56+
let answer = '';
57+
server.listen(options, function() {
58+
options.port = this.address().port;
59+
const conn = net.connect(options);
60+
conn.setEncoding('utf8');
61+
conn.on('data', (data) => answer += data);
62+
conn.write('require("./bar")\n.exit\n');
63+
});
64+
65+
process.on('exit', function() {
66+
assert.strictEqual(/Uncaught Error: Cannot find module '\.\/bar'/.test(answer), true);
67+
68+
assert.strictEqual(/code: 'MODULE_NOT_FOUND'/.test(answer), true);
69+
assert.strictEqual(/requireStack: \[ '<repl>' \]/.test(answer), true);
70+
});
71+
}

test/parallel/test-require-resolve.js

+27
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
const common = require('../common');
2424
const fixtures = require('../common/fixtures');
2525
const assert = require('assert');
26+
const { builtinModules } = require('module');
27+
const path = require('path');
2628

2729
assert.strictEqual(
2830
require.resolve(fixtures.path('a')).toLowerCase(),
@@ -52,3 +54,28 @@ const re = /^The "request" argument must be of type string\. Received type \w+$/
5254
message: re
5355
});
5456
});
57+
58+
// Test require.resolve.paths.
59+
{
60+
// builtinModules.
61+
builtinModules.forEach((mod) => {
62+
assert.strictEqual(require.resolve.paths(mod), null);
63+
});
64+
65+
// node_modules.
66+
const resolvedPaths = require.resolve.paths('eslint');
67+
assert.strictEqual(Array.isArray(resolvedPaths), true);
68+
assert.strictEqual(resolvedPaths[0].includes('node_modules'), true);
69+
70+
// relativeModules.
71+
const relativeModules = ['.', '..', './foo', '../bar'];
72+
relativeModules.forEach((mod) => {
73+
const resolvedPaths = require.resolve.paths(mod);
74+
assert.strictEqual(Array.isArray(resolvedPaths), true);
75+
assert.strictEqual(resolvedPaths.length, 1);
76+
assert.strictEqual(resolvedPaths[0], path.dirname(__filename));
77+
78+
// Shouldn't look up relative modules from 'node_modules'.
79+
assert.strictEqual(resolvedPaths.includes('/node_modules'), false);
80+
});
81+
}

0 commit comments

Comments
 (0)