Skip to content

Commit

Permalink
module,repl: support 'node:'-only core modules
Browse files Browse the repository at this point in the history
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: #42325
Backport-PR-URL: #43904
Refs: #40954
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
cjihrig authored and targos committed Jul 31, 2022
1 parent 9a53083 commit 3d851d6
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 16 deletions.
16 changes: 16 additions & 0 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
/* global process, getLinkedBinding, getInternalBinding, primordials */

const {
ArrayFrom,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeSlice,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 9 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 7 additions & 1 deletion lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ const {
globalThis,
} = primordials;

const { NativeModule } = require('internal/bootstrap/loaders');
const {
makeRequireFunction,
addBuiltinLibsToObject
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-code-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions test/parallel/test-repl-tab-complete-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
}));
Expand Down
16 changes: 10 additions & 6 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
}));
Expand Down
15 changes: 15 additions & 0 deletions test/parallel/test-runner-import-no-scheme.js
Original file line number Diff line number Diff line change
@@ -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());

0 comments on commit 3d851d6

Please sign in to comment.