Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: add prefix-only modules to module.builtinModules #56185

Merged
merged 1 commit into from
Dec 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ added:
- v9.3.0
- v8.10.0
- v6.13.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/56185
description: The list now also contains prefix-only modules.
-->

* {string\[]}

A list of the names of all modules provided by Node.js. Can be used to verify
if a module is maintained by a third party or not.

Note: the list doesn't contain [prefix-only modules][] like `node:test`.
ljharb marked this conversation as resolved.
Show resolved Hide resolved

`module` in this context isn't the same object that's provided
by the [module wrapper][]. To access it, require the `Module` module:

Expand Down Expand Up @@ -1724,7 +1726,6 @@ returned object contains the following keys:
[load hook]: #loadurl-context-nextload
[module compile cache]: #module-compile-cache
[module wrapper]: modules.md#the-module-wrapper
[prefix-only modules]: modules.md#built-in-modules-with-mandatory-node-prefix
[realm]: https://tc39.es/ecma262/#realm
[resolve hook]: #resolvespecifier-context-nextresolve
[source map include directives]: https://sourcemaps.info/spec.html#h.lmz475t4mvbx
Expand Down
4 changes: 3 additions & 1 deletion doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ Some built-in modules are always preferentially loaded if their identifier is
passed to `require()`. For instance, `require('http')` will always
return the built-in HTTP module, even if there is a file by that name. The list
of built-in modules that can be loaded without using the `node:` prefix is exposed
as [`module.builtinModules`][].
in [`module.builtinModules`][], listed without the prefix.

### Built-in modules with mandatory `node:` prefix

Expand All @@ -531,6 +531,8 @@ taken the name. Currently the built-in modules that requires the `node:` prefix
* [`node:test`][]
* [`node:test/reporters`][]

The list of these modules is exposed in [`module.builtinModules`][], including the prefix.

## Cycles

<!--type=misc-->
Expand Down
11 changes: 7 additions & 4 deletions lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const {
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypePushApply,
ArrayPrototypeSlice,
Error,
ObjectDefineProperty,
Expand Down Expand Up @@ -320,14 +321,16 @@ class BuiltinModule {
);
}

static getCanBeRequiredByUsersWithoutSchemeList() {
return ArrayFrom(canBeRequiredByUsersWithoutSchemeList);
}

static getSchemeOnlyModuleNames() {
return ArrayFrom(schemelessBlockList);
}

static getAllBuiltinModuleIds() {
const allBuiltins = ArrayFrom(canBeRequiredByUsersWithoutSchemeList);
ArrayPrototypePushApply(allBuiltins, ArrayFrom(schemelessBlockList, (x) => `node:${x}`));
return allBuiltins;
}

// Used by user-land module loaders to compile and load builtins.
compileForPublicLoader() {
if (!BuiltinModule.canBeRequiredByUsers(this.id)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,8 @@ Module.isBuiltin = BuiltinModule.isBuiltin;
*/
function initializeCJS() {
// This need to be done at runtime in case --expose-internals is set.
const builtinModules = BuiltinModule.getCanBeRequiredByUsersWithoutSchemeList();
Module.builtinModules = ObjectFreeze(builtinModules);

Module.builtinModules = ObjectFreeze(BuiltinModule.getAllBuiltinModuleIds());

initializeCjsConditions();

Expand Down
2 changes: 1 addition & 1 deletion lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ const { shouldColorize } = require('internal/util/colors');
const CJSModule = require('internal/modules/cjs/loader').Module;
let _builtinLibs = ArrayPrototypeFilter(
CJSModule.builtinModules,
(e) => e[0] !== '_',
(e) => e[0] !== '_' && !StringPrototypeStartsWith(e, 'node:'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess exposing these to the REPL would actually be good, while it's also good to have it in a separate PR lands, to clearly separate the things from each other.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to do that as a followup.

);
const nodeSchemeBuiltinLibs = ArrayPrototypeMap(
_builtinLibs, (lib) => `node:${lib}`);
Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-internal-module-require.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ if (process.argv[2] === 'child') {
});
} else {
require(id);
if (!id.startsWith('node:')) {
require(`node:${id}`);
}
publicModules.add(id);
}
}
Expand Down
14 changes: 9 additions & 5 deletions test/parallel/test-process-get-builtin.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,19 @@ for (const id of publicBuiltins) {
}
// Check that import(id).default returns the same thing as process.getBuiltinModule(id).
for (const id of publicBuiltins) {
const imported = await import(`node:${id}`);
assert.strictEqual(process.getBuiltinModule(id), imported.default);
if (!id.startsWith('node:')) {
const imported = await import(`node:${id}`);
assert.strictEqual(process.getBuiltinModule(id), imported.default);
}
}

// publicBuiltins does not include 'test' which requires the node: prefix.
const ids = publicBuiltins.add('test');
// Check that import(id).default returns the same thing as process.getBuiltinModule(id).
for (const id of ids) {
const prefixed = `node:${id}`;
const imported = await import(prefixed);
assert.strictEqual(process.getBuiltinModule(prefixed), imported.default);
if (!id.startsWith('node:')) {
const prefixed = `node:${id}`;
const imported = await import(prefixed);
assert.strictEqual(process.getBuiltinModule(prefixed), imported.default);
}
}
6 changes: 3 additions & 3 deletions test/parallel/test-repl-tab-complete-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const ArrayStream = require('../common/arraystream');
const fixtures = require('../common/fixtures');
const assert = require('assert');
const { builtinModules } = require('module');
const publicModules = builtinModules.filter((lib) => !lib.startsWith('_'));
const publicUnprefixedModules = builtinModules.filter((lib) => !lib.startsWith('_') && !lib.startsWith('node:'));

if (!common.isMainThread)
common.skip('process.chdir is not available in Workers');
Expand All @@ -31,7 +31,7 @@ testMe._domain.on('error', assert.ifError);
// Tab complete provides built in libs for import()
testMe.complete('import(\'', common.mustCall((error, data) => {
assert.strictEqual(error, null);
publicModules.forEach((lib) => {
publicUnprefixedModules.forEach((lib) => {
assert(
data[0].includes(lib) && data[0].includes(`node:${lib}`),
`${lib} not found`,
Expand All @@ -55,7 +55,7 @@ testMe.complete("import\t( 'n", common.mustCall((error, data) => {
// import(...) completions include `node:` URL modules:
let lastIndex = -1;

publicModules.forEach((lib, index) => {
publicUnprefixedModules.forEach((lib, index) => {
lastIndex = completions.indexOf(`node:${lib}`);
assert.notStrictEqual(lastIndex, -1);
});
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ testMe.complete('require(\'', common.mustCall(function(error, data) {
assert.strictEqual(error, null);
publicModules.forEach((lib) => {
assert(
data[0].includes(lib) && data[0].includes(`node:${lib}`),
data[0].includes(lib) && (lib.startsWith('node:') || data[0].includes(`node:${lib}`)),
`${lib} not found`
);
});
Expand All @@ -295,7 +295,7 @@ testMe.complete("require\t( 'n", common.mustCall(function(error, data) {
// require(...) completions include `node:`-prefixed modules:
let lastIndex = -1;

publicModules.forEach((lib, index) => {
publicModules.filter((lib) => !lib.startsWith('node:')).forEach((lib, index) => {
lastIndex = data[0].indexOf(`node:${lib}`);
assert.notStrictEqual(lastIndex, -1);
});
Expand Down
7 changes: 3 additions & 4 deletions test/parallel/test-require-resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ require(fixtures.path('resolve-paths', 'default', 'verify-paths.js'));
// builtinModules.
builtinModules.forEach((mod) => {
assert.strictEqual(require.resolve.paths(mod), null);
});

builtinModules.forEach((mod) => {
assert.strictEqual(require.resolve.paths(`node:${mod}`), null);
if (!mod.startsWith('node:')) {
assert.strictEqual(require.resolve.paths(`node:${mod}`), null);
}
});

// node_modules.
Expand Down
Loading