Skip to content

Commit 10f6fd1

Browse files
committed
module,repl: add subpaths builtins in addBuiltinLibsToObject
This also fixes unrelated completions with e.g. `require('`.
1 parent fd86dad commit 10f6fd1

File tree

6 files changed

+145
-127
lines changed

6 files changed

+145
-127
lines changed

doc/api/repl.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,13 @@ global or scoped variable, the input `fs` will be evaluated on-demand as
147147
> fs.createReadStream('./some/file');
148148
```
149149

150+
Subpaths Node.js core modules are exposed with a underscore,
151+
for example, `stream/consumers` will become `stream_consumers`.
152+
153+
```console
154+
> stream_consumers.text(process.stdin);
155+
```
156+
150157
#### Global uncaught exceptions
151158
<!-- YAML
152159
changes:

lib/internal/modules/cjs/helpers.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const {
99
SafeMap,
1010
SafeSet,
1111
StringPrototypeCharCodeAt,
12-
StringPrototypeIncludes,
12+
StringPrototypeReplace,
1313
StringPrototypeSlice,
1414
StringPrototypeStartsWith,
1515
} = primordials;
@@ -150,10 +150,8 @@ function addBuiltinLibsToObject(object, dummyModuleName) {
150150
const dummyModule = new Module(dummyModuleName);
151151

152152
ArrayPrototypeForEach(builtinModules, (name) => {
153-
// Neither add underscored modules, nor ones that contain slashes (e.g.,
154-
// 'fs/promises') or ones that are already defined.
153+
// Neither add underscored modules or ones that are already defined.
155154
if (StringPrototypeStartsWith(name, '_') ||
156-
StringPrototypeIncludes(name, '/') ||
157155
ObjectPrototypeHasOwnProperty(object, name)) {
158156
return;
159157
}
@@ -163,21 +161,23 @@ function addBuiltinLibsToObject(object, dummyModuleName) {
163161
// - Allowing the user to re-assign these variables as if there were no
164162
// pre-existing globals with the same name.
165163

164+
const realName = StringPrototypeReplace(name, '/', '_')
165+
166166
const setReal = (val) => {
167167
// Deleting the property before re-assigning it disables the
168168
// getter/setter mechanism.
169-
delete object[name];
170-
object[name] = val;
169+
delete object[realName];
170+
object[realName] = val;
171171
};
172172

173-
ObjectDefineProperty(object, name, {
173+
ObjectDefineProperty(object, realName, {
174174
get: () => {
175175
const lib = dummyModule.require(name);
176176

177177
// Disable the current getter/setter and set up a new
178178
// non-enumerable property.
179-
delete object[name];
180-
ObjectDefineProperty(object, name, {
179+
delete object[realName];
180+
ObjectDefineProperty(object, realName, {
181181
get: () => lib,
182182
set: setReal,
183183
configurable: true,

lib/repl.js

Lines changed: 109 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const { Console } = require('console');
125125
const CJSModule = require('internal/modules/cjs/loader').Module;
126126
let _builtinLibs = ArrayPrototypeFilter(
127127
CJSModule.builtinModules,
128-
(e) => !StringPrototypeStartsWith(e, '_') && !StringPrototypeIncludes(e, '/')
128+
(e) => !StringPrototypeStartsWith(e, '_'),
129129
);
130130
const nodeSchemeBuiltinLibs = ArrayPrototypeMap(
131131
_builtinLibs, (lib) => `node:${lib}`);
@@ -178,6 +178,7 @@ const {
178178
extensionFormatMap,
179179
legacyExtensionFormatMap,
180180
} = require('internal/modules/esm/get_format');
181+
const console = require('console');
181182

182183
let nextREPLResourceNumber = 1;
183184
// This prevents v8 code cache from getting confused and using a different
@@ -1276,7 +1277,6 @@ function complete(line, callback) {
12761277

12771278
// Ignore right whitespace. It could change the outcome.
12781279
line = StringPrototypeTrimLeft(line);
1279-
12801280
// REPL commands (e.g. ".break").
12811281
let filter = '';
12821282
if (RegExpPrototypeTest(/^\s*\.(\w*)$/, line)) {
@@ -1285,138 +1285,137 @@ function complete(line, callback) {
12851285
if (completeOn.length) {
12861286
filter = completeOn;
12871287
}
1288-
} else if (RegExpPrototypeTest(requireRE, line) &&
1289-
this.allowBlockingCompletions) {
1288+
} else if (RegExpPrototypeTest(requireRE, line)) {
12901289
// require('...<Tab>')
1291-
const extensions = ObjectKeys(this.context.require.extensions);
1292-
const indexes = ArrayPrototypeMap(extensions,
1293-
(extension) => `index${extension}`);
1294-
ArrayPrototypePush(indexes, 'package.json', 'index');
1295-
12961290
const match = StringPrototypeMatch(line, requireRE);
12971291
completeOn = match[1];
12981292
const subdir = match[2] || '';
12991293
filter = completeOn;
1300-
group = [];
1301-
let paths = [];
1302-
1303-
if (completeOn === '.') {
1304-
group = ['./', '../'];
1305-
} else if (completeOn === '..') {
1306-
group = ['../'];
1307-
} else if (RegExpPrototypeTest(/^\.\.?\//, completeOn)) {
1308-
paths = [process.cwd()];
1309-
} else {
1310-
paths = ArrayPrototypeConcat(module.paths, CJSModule.globalPaths);
1311-
}
1312-
1313-
ArrayPrototypeForEach(paths, (dir) => {
1314-
dir = path.resolve(dir, subdir);
1315-
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
1316-
ArrayPrototypeForEach(dirents, (dirent) => {
1317-
if (RegExpPrototypeTest(versionedFileNamesRe, dirent.name) ||
1318-
dirent.name === '.npm') {
1319-
// Exclude versioned names that 'npm' installs.
1320-
return;
1321-
}
1322-
const extension = path.extname(dirent.name);
1323-
const base = StringPrototypeSlice(dirent.name, 0, -extension.length);
1324-
if (!dirent.isDirectory()) {
1325-
if (StringPrototypeIncludes(extensions, extension) &&
1326-
(!subdir || base !== 'index')) {
1327-
ArrayPrototypePush(group, `${subdir}${base}`);
1294+
if (this.allowBlockingCompletions) {
1295+
const extensions = ObjectKeys(this.context.require.extensions);
1296+
const indexes = ArrayPrototypeMap(extensions,
1297+
(extension) => `index${extension}`);
1298+
ArrayPrototypePush(indexes, 'package.json', 'index');
1299+
1300+
group = [];
1301+
let paths = [];
1302+
1303+
if (completeOn === '.') {
1304+
group = ['./', '../'];
1305+
} else if (completeOn === '..') {
1306+
group = ['../'];
1307+
} else if (RegExpPrototypeTest(/^\.\.?\//, completeOn)) {
1308+
paths = [process.cwd()];
1309+
} else {
1310+
paths = ArrayPrototypeConcat(module.paths, CJSModule.globalPaths);
1311+
}
1312+
1313+
ArrayPrototypeForEach(paths, (dir) => {
1314+
dir = path.resolve(dir, subdir);
1315+
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
1316+
ArrayPrototypeForEach(dirents, (dirent) => {
1317+
if (RegExpPrototypeTest(versionedFileNamesRe, dirent.name) ||
1318+
dirent.name === '.npm') {
1319+
// Exclude versioned names that 'npm' installs.
1320+
return;
13281321
}
1329-
return;
1330-
}
1331-
ArrayPrototypePush(group, `${subdir}${dirent.name}/`);
1332-
const absolute = path.resolve(dir, dirent.name);
1333-
if (ArrayPrototypeSome(
1334-
gracefulReaddir(absolute) || [],
1335-
(subfile) => ArrayPrototypeIncludes(indexes, subfile)
1336-
)) {
1337-
ArrayPrototypePush(group, `${subdir}${dirent.name}`);
1338-
}
1322+
const extension = path.extname(dirent.name);
1323+
const base = StringPrototypeSlice(dirent.name, 0, -extension.length);
1324+
if (!dirent.isDirectory()) {
1325+
if (StringPrototypeIncludes(extensions, extension) &&
1326+
(!subdir || base !== 'index')) {
1327+
ArrayPrototypePush(group, `${subdir}${base}`);
1328+
}
1329+
return;
1330+
}
1331+
ArrayPrototypePush(group, `${subdir}${dirent.name}/`);
1332+
const absolute = path.resolve(dir, dirent.name);
1333+
if (ArrayPrototypeSome(
1334+
gracefulReaddir(absolute) || [],
1335+
(subfile) => ArrayPrototypeIncludes(indexes, subfile)
1336+
)) {
1337+
ArrayPrototypePush(group, `${subdir}${dirent.name}`);
1338+
}
1339+
});
13391340
});
1340-
});
1341-
if (group.length) {
1342-
ArrayPrototypePush(completionGroups, group);
1341+
if (group.length) {
1342+
ArrayPrototypePush(completionGroups, group);
1343+
}
13431344
}
13441345

1345-
if (!subdir) {
1346-
ArrayPrototypePush(completionGroups, _builtinLibs, nodeSchemeBuiltinLibs);
1347-
}
1348-
} else if (RegExpPrototypeTest(importRE, line) &&
1349-
this.allowBlockingCompletions) {
1346+
ArrayPrototypePush(completionGroups, _builtinLibs, nodeSchemeBuiltinLibs);
1347+
} else if (RegExpPrototypeTest(importRE, line)) {
13501348
// import('...<Tab>')
1351-
// File extensions that can be imported:
1352-
const extensions = ObjectKeys(
1353-
getOptionValue('--experimental-specifier-resolution') === 'node' ?
1354-
legacyExtensionFormatMap :
1355-
extensionFormatMap);
1356-
1357-
// Only used when loading bare module specifiers from `node_modules`:
1358-
const indexes = ArrayPrototypeMap(extensions, (ext) => `index${ext}`);
1359-
ArrayPrototypePush(indexes, 'package.json');
1360-
13611349
const match = StringPrototypeMatch(line, importRE);
13621350
completeOn = match[1];
13631351
const subdir = match[2] || '';
13641352
filter = completeOn;
1365-
group = [];
1366-
let paths = [];
1367-
if (completeOn === '.') {
1368-
group = ['./', '../'];
1369-
} else if (completeOn === '..') {
1370-
group = ['../'];
1371-
} else if (RegExpPrototypeTest(/^\.\.?\//, completeOn)) {
1372-
paths = [process.cwd()];
1373-
} else {
1374-
paths = ArrayPrototypeSlice(module.paths);
1375-
}
1353+
if (this.allowBlockingCompletions) {
1354+
// File extensions that can be imported:
1355+
const extensions = ObjectKeys(
1356+
getOptionValue('--experimental-specifier-resolution') === 'node' ?
1357+
legacyExtensionFormatMap :
1358+
extensionFormatMap);
1359+
1360+
// Only used when loading bare module specifiers from `node_modules`:
1361+
const indexes = ArrayPrototypeMap(extensions, (ext) => `index${ext}`);
1362+
ArrayPrototypePush(indexes, 'package.json');
1363+
1364+
group = [];
1365+
let paths = [];
1366+
if (completeOn === '.') {
1367+
group = ['./', '../'];
1368+
} else if (completeOn === '..') {
1369+
group = ['../'];
1370+
} else if (RegExpPrototypeTest(/^\.\.?\//, completeOn)) {
1371+
paths = [process.cwd()];
1372+
} else {
1373+
paths = ArrayPrototypeSlice(module.paths);
1374+
}
13761375

1377-
ArrayPrototypeForEach(paths, (dir) => {
1378-
dir = path.resolve(dir, subdir);
1379-
const isInNodeModules = path.basename(dir) === 'node_modules';
1380-
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
1381-
ArrayPrototypeForEach(dirents, (dirent) => {
1382-
const { name } = dirent;
1383-
if (RegExpPrototypeTest(versionedFileNamesRe, name) ||
1384-
name === '.npm') {
1385-
// Exclude versioned names that 'npm' installs.
1386-
return;
1387-
}
1376+
ArrayPrototypeForEach(paths, (dir) => {
1377+
dir = path.resolve(dir, subdir);
1378+
const isInNodeModules = path.basename(dir) === 'node_modules';
1379+
const dirents = gracefulReaddir(dir, { withFileTypes: true }) || [];
1380+
ArrayPrototypeForEach(dirents, (dirent) => {
1381+
const { name } = dirent;
1382+
if (RegExpPrototypeTest(versionedFileNamesRe, name) ||
1383+
name === '.npm') {
1384+
// Exclude versioned names that 'npm' installs.
1385+
return;
1386+
}
13881387

1389-
if (!dirent.isDirectory()) {
1390-
const extension = path.extname(name);
1391-
if (StringPrototypeIncludes(extensions, extension)) {
1392-
ArrayPrototypePush(group, `${subdir}${name}`);
1388+
if (!dirent.isDirectory()) {
1389+
const extension = path.extname(name);
1390+
if (StringPrototypeIncludes(extensions, extension)) {
1391+
ArrayPrototypePush(group, `${subdir}${name}`);
1392+
}
1393+
return;
13931394
}
1394-
return;
1395-
}
13961395

1397-
ArrayPrototypePush(group, `${subdir}${name}/`);
1398-
if (!subdir && isInNodeModules) {
1399-
const absolute = path.resolve(dir, name);
1400-
const subfiles = gracefulReaddir(absolute) || [];
1401-
if (ArrayPrototypeSome(subfiles, (subfile) => {
1402-
return ArrayPrototypeIncludes(indexes, subfile);
1403-
})) {
1404-
ArrayPrototypePush(group, `${subdir}${name}`);
1396+
ArrayPrototypePush(group, `${subdir}${name}/`);
1397+
if (!subdir && isInNodeModules) {
1398+
const absolute = path.resolve(dir, name);
1399+
const subfiles = gracefulReaddir(absolute) || [];
1400+
if (ArrayPrototypeSome(subfiles, (subfile) => {
1401+
return ArrayPrototypeIncludes(indexes, subfile);
1402+
})) {
1403+
ArrayPrototypePush(group, `${subdir}${name}`);
1404+
}
14051405
}
1406-
}
1406+
});
14071407
});
1408-
});
14091408

1410-
if (group.length) {
1411-
ArrayPrototypePush(completionGroups, group);
1409+
if (group.length) {
1410+
ArrayPrototypePush(completionGroups, group);
1411+
}
14121412
}
14131413

1414-
if (!subdir) {
1415-
ArrayPrototypePush(completionGroups, _builtinLibs, nodeSchemeBuiltinLibs);
1414+
ArrayPrototypePush(completionGroups, _builtinLibs, nodeSchemeBuiltinLibs);
1415+
} else if (RegExpPrototypeTest(fsAutoCompleteRE, line)) {
1416+
if (this.allowBlockingCompletions) {
1417+
({ 0: completionGroups, 1: completeOn } = completeFSFunctions(line));
14161418
}
1417-
} else if (RegExpPrototypeTest(fsAutoCompleteRE, line) &&
1418-
this.allowBlockingCompletions) {
1419-
({ 0: completionGroups, 1: completeOn } = completeFSFunctions(line));
14201419
// Handle variable member lookup.
14211420
// We support simple chained expressions like the following (no function
14221421
// calls, etc.). That is for simplicity and also because we *eval* that
@@ -1560,7 +1559,6 @@ function complete(line, callback) {
15601559
if (completions[0] === '') {
15611560
ArrayPrototypeShift(completions);
15621561
}
1563-
15641562
callback(null, [completions, completeOn]);
15651563
}
15661564
}

test/parallel/test-repl-autolibs.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ function test1() {
3636
putIn.write = function(data) {
3737
gotWrite = true;
3838
if (data.length) {
39-
4039
// Inspect output matches repl output
4140
assert.strictEqual(data,
4241
`${util.inspect(require('fs'), null, 2, false)}\n`);
@@ -59,6 +58,7 @@ function test2() {
5958
assert.strictEqual(data, '{}\n');
6059
// Original value wasn't overwritten
6160
assert.strictEqual(val, global.url);
61+
test3();
6262
}
6363
};
6464
const val = {};
@@ -68,3 +68,20 @@ function test2() {
6868
putIn.run(['url']);
6969
assert(gotWrite);
7070
}
71+
72+
function test3() {
73+
let gotWrite = false;
74+
putIn.write = function(data) {
75+
gotWrite = true;
76+
if (data.length) {
77+
// Inspect output matches repl output
78+
assert.strictEqual(data,
79+
`${util.inspect(require('stream/consumers'), null, 2, false)}\n`);
80+
// Globally added lib matches required lib
81+
assert.strictEqual(global.stream_consumers, require('stream/consumers'));
82+
}
83+
};
84+
assert(!gotWrite);
85+
putIn.run(['stream_consumers']);
86+
assert(gotWrite);
87+
}

test/parallel/test-repl-tab-complete-import.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ const ArrayStream = require('../common/arraystream');
55
const fixtures = require('../common/fixtures');
66
const assert = require('assert');
77
const { builtinModules } = require('module');
8-
const publicModules = builtinModules.filter(
9-
(lib) => !lib.startsWith('_') && !lib.includes('/'),
10-
);
8+
const publicModules = builtinModules.filter((lib) => !lib.startsWith('_'));
119

1210
if (!common.isMainThread)
1311
common.skip('process.chdir is not available in Workers');

0 commit comments

Comments
 (0)