Skip to content

Commit

Permalink
repl: replace hard coded core module list with actual list
Browse files Browse the repository at this point in the history
This replaces the internally used hard coded Node.js core module
list with the actual internal existent modules. That way all modules
are automatically picked up instead of having to update the list
manually.

This currently only applies to the REPL and to the Node.js `eval`
functionality (User passed `-e` or `--eval` arguments to Node without
`-i` or `--interactive`).

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #33282
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
BridgeAR authored and codebytere committed May 16, 2020
1 parent d33dcf1 commit 69061dc
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 62 deletions.
58 changes: 10 additions & 48 deletions lib/internal/modules/cjs/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const {
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
SafeMap,
} = primordials;
const {
Expand Down Expand Up @@ -109,55 +110,17 @@ function stripBOM(content) {
return content;
}

const builtinLibs = [
'assert',
'async_hooks',
'buffer',
'child_process',
'cluster',
'crypto',
'dgram',
'dns',
'domain',
'events',
'fs',
'http',
'http2',
'https',
'net',
'os',
'path',
'perf_hooks',
'punycode',
'querystring',
'readline',
'repl',
'stream',
'string_decoder',
'tls',
'trace_events',
'tty',
'url',
'util',
'v8',
'vm',
'worker_threads',
'zlib',
];

if (internalBinding('config').experimentalWasi) {
builtinLibs.push('wasi');
builtinLibs.sort();
}

if (typeof internalBinding('inspector').open === 'function') {
builtinLibs.push('inspector');
builtinLibs.sort();
}

function addBuiltinLibsToObject(object) {
// Make built-in modules available directly (loaded lazily).
builtinLibs.forEach((name) => {
const { builtinModules } = require('internal/modules/cjs/loader').Module;
builtinModules.forEach((name) => {
// Neither add underscored modules, nor ones that contain slashes (e.g.,
// 'fs/promises') or ones that are already defined.
if (name.startsWith('_') ||
name.includes('/') ||
ObjectPrototypeHasOwnProperty(object, name)) {
return;
}
// Goals of this mechanism are:
// - Lazy loading of built-in modules
// - Having all built-in modules available as non-enumerable properties
Expand Down Expand Up @@ -203,7 +166,6 @@ function normalizeReferrerURL(referrer) {

module.exports = {
addBuiltinLibsToObject,
builtinLibs,
loadNativeModule,
makeRequireFunction,
normalizeReferrerURL,
Expand Down
5 changes: 3 additions & 2 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ const {
} = primordials;

const {
builtinLibs,
makeRequireFunction,
addBuiltinLibsToObject
} = require('internal/modules/cjs/helpers');
Expand All @@ -86,6 +85,8 @@ const {
} = require('internal/readline/utils');
const { Console } = require('console');
const CJSModule = require('internal/modules/cjs/loader').Module;
const builtinModules = [...CJSModule.builtinModules]
.filter((e) => !e.startsWith('_'));
const domain = require('domain');
const debug = require('internal/util/debuglog').debuglog('repl');
const {
Expand Down Expand Up @@ -158,7 +159,7 @@ module.paths = CJSModule._nodeModulePaths(module.filename);
const writer = exports.writer = (obj) => inspect(obj, writer.options);
writer.options = { ...inspect.defaultOptions, showProxy: true };

exports._builtinLibs = builtinLibs;
exports._builtinLibs = builtinModules;

function REPLServer(prompt,
stream,
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const expectedModules = new Set([
'Internal Binding credentials',
'Internal Binding fs',
'Internal Binding fs_dir',
'Internal Binding inspector',
'Internal Binding module_wrap',
'Internal Binding native_module',
'Internal Binding options',
Expand Down Expand Up @@ -116,6 +115,7 @@ if (common.hasIntl) {
}

if (process.features.inspector) {
expectedModules.add('Internal Binding inspector');
expectedModules.add('NativeModule internal/inspector_async_hook');
expectedModules.add('NativeModule internal/util/inspector');
}
Expand Down
9 changes: 0 additions & 9 deletions test/parallel/test-module-cjs-helpers.js

This file was deleted.

4 changes: 4 additions & 0 deletions test/parallel/test-repl-mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ function testStrictMode() {
}

function testStrictModeTerminal() {
if (!process.features.inspector) {
console.warn('Test skipped: V8 inspector is disabled');
return;
}
// Verify that ReferenceErrors are reported in strict mode previews.
const cli = initRepl(repl.REPL_MODE_STRICT, {
terminal: true
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const {
const assert = require('assert');
const path = require('path');
const fixtures = require('../common/fixtures');
const { builtinModules } = require('module');
const hasInspector = process.features.inspector;

if (!common.isMainThread)
Expand Down Expand Up @@ -222,8 +223,9 @@ putIn.run(['.clear']);

testMe.complete('require(\'', common.mustCall(function(error, data) {
assert.strictEqual(error, null);
repl._builtinLibs.forEach(function(lib) {
assert(data[0].includes(lib), `${lib} not found`);
builtinModules.forEach((lib) => {
if (!lib.startsWith('_'))
assert(data[0].includes(lib), `${lib} not found`);
});
}));

Expand Down

0 comments on commit 69061dc

Please sign in to comment.