Skip to content

Commit e5437ef

Browse files
ZYSzysBethGriggs
authored andcommitted
module: fix require in node repl
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>
1 parent 168dd92 commit e5437ef

3 files changed

Lines changed: 87 additions & 27 deletions

File tree

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -810,11 +810,11 @@ Module._resolveLookupPaths = function(request, parent) {
810810
returnpaths.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-
constmainPaths=['.'].concat(Module._nodeModulePaths('.'),modulePaths);
816+
// from realpath(__filename) but in REPL there is no filename
817+
constmainPaths=['.'];
818818

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

‎test/parallel/test-repl-require.js‎

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

14-
constserver=net.createServer((conn)=>{
15-
repl.start('',conn).on('exit',()=>{
16-
conn.destroy();
17-
server.close();
14+
{
15+
constserver=net.createServer((conn)=>{
16+
repl.start('',conn).on('exit',()=>{
17+
conn.destroy();
18+
server.close();
19+
});
1820
});
19-
});
20-
21-
consthost=common.localhostIPv4;
22-
constport=0;
23-
constoptions={ host, port };
24-
25-
letanswer='';
26-
server.listen(options,function(){
27-
options.port=this.address().port;
28-
constconn=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(/Cannotfindmodule/.test(answer),false);
36-
assert.strictEqual(/Error/.test(answer),false);
37-
assert.strictEqual(answer,'\'eye catcher\'\n\'perhaps I work\'\n');
38-
});
21+
22+
consthost=common.localhostIPv4;
23+
constport=0;
24+
constoptions={ host, port };
25+
26+
letanswer='';
27+
server.listen(options,function(){
28+
options.port=this.address().port;
29+
constconn=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(/Cannotfindmodule/.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+
constserver=net.createServer((conn)=>{
46+
repl.start('',conn).on('exit',()=>{
47+
conn.destroy();
48+
server.close();
49+
});
50+
});
51+
52+
consthost=common.localhostIPv4;
53+
constport=0;
54+
constoptions={ host, port };
55+
56+
letanswer='';
57+
server.listen(options,function(){
58+
options.port=this.address().port;
59+
constconn=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(/UncaughtError:Cannotfindmodule'\.\/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‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
constcommon=require('../common');
2424
constfixtures=require('../common/fixtures');
2525
constassert=require('assert');
26+
const{ builtinModules }=require('module');
27+
constpath=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+
constresolvedPaths=require.resolve.paths('eslint');
67+
assert.strictEqual(Array.isArray(resolvedPaths),true);
68+
assert.strictEqual(resolvedPaths[0].includes('node_modules'),true);
69+
70+
// relativeModules.
71+
constrelativeModules=['.','..','./foo','../bar'];
72+
relativeModules.forEach((mod)=>{
73+
constresolvedPaths=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)