From 69061dc73e6ed8130f73c685b1cb35da9d90b10d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 7 May 2020 17:09:51 +0200 Subject: [PATCH] repl: replace hard coded core module list with actual list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 PR-URL: https://github.com/nodejs/node/pull/33282 Reviewed-By: James M Snell Reviewed-By: Michaël Zasso --- lib/internal/modules/cjs/helpers.js | 58 ++++-------------------- lib/repl.js | 5 +- test/parallel/test-bootstrap-modules.js | 2 +- test/parallel/test-module-cjs-helpers.js | 9 ---- test/parallel/test-repl-mode.js | 4 ++ test/parallel/test-repl-tab-complete.js | 6 ++- 6 files changed, 22 insertions(+), 62 deletions(-) delete mode 100644 test/parallel/test-module-cjs-helpers.js diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 2f62c8623e27be..2181fc2a42cab3 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -2,6 +2,7 @@ const { ObjectDefineProperty, + ObjectPrototypeHasOwnProperty, SafeMap, } = primordials; const { @@ -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 @@ -203,7 +166,6 @@ function normalizeReferrerURL(referrer) { module.exports = { addBuiltinLibsToObject, - builtinLibs, loadNativeModule, makeRequireFunction, normalizeReferrerURL, diff --git a/lib/repl.js b/lib/repl.js index 320c57bc3ff4e3..fc8b36ed19eccb 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -63,7 +63,6 @@ const { } = primordials; const { - builtinLibs, makeRequireFunction, addBuiltinLibsToObject } = require('internal/modules/cjs/helpers'); @@ -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 { @@ -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, diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index ee2a40c0a6cd79..1ebeb01bddaa87 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -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', @@ -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'); } diff --git a/test/parallel/test-module-cjs-helpers.js b/test/parallel/test-module-cjs-helpers.js deleted file mode 100644 index 12de65598e54e1..00000000000000 --- a/test/parallel/test-module-cjs-helpers.js +++ /dev/null @@ -1,9 +0,0 @@ -'use strict'; -// Flags: --expose-internals - -require('../common'); -const assert = require('assert'); -const { builtinLibs } = require('internal/modules/cjs/helpers'); - -const expectedLibs = process.features.inspector ? 34 : 33; -assert.strictEqual(builtinLibs.length, expectedLibs); diff --git a/test/parallel/test-repl-mode.js b/test/parallel/test-repl-mode.js index 514fbbbbe3f8b5..d8131d34f93b44 100644 --- a/test/parallel/test-repl-mode.js +++ b/test/parallel/test-repl-mode.js @@ -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 diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index 6cf689c4b11074..584dbc21cf4a49 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -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) @@ -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`); }); }));