module,repl: support 'node:'-only core modules

This commit makes it possible to add new core modules that can only be require()'ed and imported when the 'node:' scheme is used. The 'test' module is the first such module. These 'node:'-only modules are not included in the list returned by module.builtinModules. PR-URL: nodejs/node#42325 Backport-PR-URL: nodejs/node#43904 Refs: nodejs/node#40954 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
noslate-project · Oct 10, 2022 · 88cf0d0 · 88cf0d0
1 parent 14d5f90
commit 88cf0d0
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 16 deletions.
diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
@@ -44,6 +44,7 @@
 /* global process, getLinkedBinding, getInternalBinding, primordials */
 
 const {
+  ArrayFrom,
   ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeSlice,
@@ -120,6 +121,11 @@ const legacyWrapperList = new SafeSet([
   'util',
 ]);
 
+// Modules that can only be imported via the node: scheme.
+const schemelessBlockList = new SafeSet([
+  'test',
+]);
+
 // Set up process.binding() and process._linkedBinding().
 {
   const bindingObj = ObjectCreate(null);
@@ -243,6 +249,16 @@ class NativeModule {
     return mod && mod.canBeRequiredByUsers;
   }
 
+  // Determine if a core module can be loaded without the node: prefix. This
+  // function does not validate if the module actually exists.
+  static canBeRequiredWithoutScheme(id) {
+    return !schemelessBlockList.has(id);
+  }
+
+  static getSchemeOnlyModuleNames() {
+    return ArrayFrom(schemelessBlockList);
+  }
+
   // Used by user-land module loaders to compile and load builtins.
   compileForPublicLoader() {
     if (!this.canBeRequiredByUsers) {

diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -182,7 +182,8 @@ function Module(id = '', parent) {
 
 const builtinModules = [];
 for (const { 0: id, 1: mod } of NativeModule.map) {
-  if (mod.canBeRequiredByUsers) {
+  if (mod.canBeRequiredByUsers &&
+    NativeModule.canBeRequiredWithoutScheme(id)) {
     ArrayPrototypePush(builtinModules, id);
   }
 }
@@ -805,7 +806,13 @@ Module._load = function(request, parent, isMain) {
   }
 
   const mod = loadNativeModule(filename, request);
-  if (mod?.canBeRequiredByUsers) return mod.exports;
+  if (mod?.canBeRequiredByUsers) {
+    if (!NativeModule.canBeRequiredWithoutScheme(filename)) {
+      throw new ERR_UNKNOWN_BUILTIN_MODULE(filename);
+    }
+
+    return mod.exports;
+  }
 
   // Don't call updateChildren(), Module constructor already does.
   const module = cachedModule || new Module(filename, parent);

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -57,6 +57,7 @@ const {
   ERR_PACKAGE_PATH_NOT_EXPORTED,
   ERR_UNSUPPORTED_DIR_IMPORT,
   ERR_NETWORK_IMPORT_DISALLOWED,
+  ERR_UNKNOWN_BUILTIN_MODULE,
   ERR_UNSUPPORTED_ESM_URL_SCHEME,
 } = require('internal/errors').codes;
 const { Module: CJSModule } = require('internal/modules/cjs/loader');
@@ -887,8 +888,13 @@ function parsePackageName(specifier, base) {
  * @returns {URL}
  */
 function packageResolve(specifier, base, conditions) {
-  if (NativeModule.canBeRequiredByUsers(specifier))
+  if (NativeModule.canBeRequiredByUsers(specifier)) {
+    if (!NativeModule.canBeRequiredWithoutScheme(specifier)) {
+      throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);
+    }
+
     return new URL('node:' + specifier);
+  }
 
   const { packageName, packageSubpath, isScoped } =
     parsePackageName(specifier, base);

diff --git a/lib/repl.js b/lib/repl.js
@@ -101,6 +101,7 @@ const {
   globalThis,
 } = primordials;
 
+const { NativeModule } = require('internal/bootstrap/loaders');
 const {
   makeRequireFunction,
   addBuiltinLibsToObject
@@ -130,6 +131,10 @@ let _builtinLibs = ArrayPrototypeFilter(
 );
 const nodeSchemeBuiltinLibs = ArrayPrototypeMap(
   _builtinLibs, (lib) => `node:${lib}`);
+ArrayPrototypeForEach(
+  NativeModule.getSchemeOnlyModuleNames(),
+  (lib) => ArrayPrototypePush(nodeSchemeBuiltinLibs, `node:${lib}`),
+);
 const domain = require('domain');
 let debug = require('internal/util/debuglog').debuglog('repl', (fn) => {
   debug = fn;

diff --git a/test/parallel/test-code-cache.js b/test/parallel/test-code-cache.js
@@ -16,7 +16,7 @@ const {
 } = internalBinding('native_module');
 
 for (const key of canBeRequired) {
-  require(key);
+  require(`node:${key}`);
 }
 
 // The computation has to be delayed until we have done loading modules

diff --git a/test/parallel/test-repl-tab-complete-import.js b/test/parallel/test-repl-tab-complete-import.js
@@ -53,14 +53,18 @@ testMe.complete("import\t( 'n", common.mustCall((error, data) => {
   assert.strictEqual(data[1], 'n');
   const completions = data[0];
   // import(...) completions include `node:` URL modules:
-  publicModules.forEach((lib, index) =>
-    assert.strictEqual(completions[index], `node:${lib}`));
-  assert.strictEqual(completions[publicModules.length], '');
+  let lastIndex = -1;
+
+  publicModules.forEach((lib, index) => {
+    lastIndex = completions.indexOf(`node:${lib}`);
+    assert.notStrictEqual(lastIndex, -1);
+  });
+  assert.strictEqual(completions[lastIndex + 1], '');
   // There is only one Node.js module that starts with n:
-  assert.strictEqual(completions[publicModules.length + 1], 'net');
-  assert.strictEqual(completions[publicModules.length + 2], '');
+  assert.strictEqual(completions[lastIndex + 2], 'net');
+  assert.strictEqual(completions[lastIndex + 3], '');
   // It's possible to pick up non-core modules too
-  completions.slice(publicModules.length + 3).forEach((completion) => {
+  completions.slice(lastIndex + 4).forEach((completion) => {
     assert.match(completion, /^n/);
   });
 }));

diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js
@@ -293,14 +293,18 @@ testMe.complete("require\t( 'n", common.mustCall(function(error, data) {
   assert.strictEqual(data.length, 2);
   assert.strictEqual(data[1], 'n');
   // require(...) completions include `node:`-prefixed modules:
-  publicModules.forEach((lib, index) =>
-    assert.strictEqual(data[0][index], `node:${lib}`));
-  assert.strictEqual(data[0][publicModules.length], '');
+  let lastIndex = -1;
+
+  publicModules.forEach((lib, index) => {
+    lastIndex = data[0].indexOf(`node:${lib}`);
+    assert.notStrictEqual(lastIndex, -1);
+  });
+  assert.strictEqual(data[0][lastIndex + 1], '');
   // There is only one Node.js module that starts with n:
-  assert.strictEqual(data[0][publicModules.length + 1], 'net');
-  assert.strictEqual(data[0][publicModules.length + 2], '');
+  assert.strictEqual(data[0][lastIndex + 2], 'net');
+  assert.strictEqual(data[0][lastIndex + 3], '');
   // It's possible to pick up non-core modules too
-  data[0].slice(publicModules.length + 3).forEach((completion) => {
+  data[0].slice(lastIndex + 4).forEach((completion) => {
     assert.match(completion, /^n/);
   });
 }));

diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js
@@ -0,0 +1,15 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+assert.throws(
+  () => require('test'),
+  common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
+);
+
+(async () => {
+  await assert.rejects(
+    async () => import('test'),
+    common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
+  );
+})().then(common.mustCall());