Skip to content

Commit

Permalink
module: add prefix-only modules to module.builtinModules
Browse files Browse the repository at this point in the history
Fixes #42785
  • Loading branch information
ljharb committed Dec 8, 2024
1 parent 549037f commit a806bee
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 21 deletions.
2 changes: 1 addition & 1 deletion doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ added:
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`.
Note: the list also contains [prefix-only modules][] like `node:test`.

`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
4 changes: 3 additions & 1 deletion doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,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 @@ -527,6 +527,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 @@ -320,14 +320,17 @@ class BuiltinModule {
);
}

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

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

static getAllBuiltinModuleIds() {
return [
...canBeRequiredByUsersWithoutSchemeList,
...ArrayFrom(schemelessBlockList, (x) => `node:${x}`),
];
}

// 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
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
8 changes: 5 additions & 3 deletions test/parallel/test-process-get-builtin.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ for (const id of publicBuiltins) {
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);
}
}
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 @@ -32,10 +32,12 @@ testMe._domain.on('error', assert.ifError);
testMe.complete('import(\'', common.mustCall((error, data) => {
assert.strictEqual(error, null);
publicModules.forEach((lib) => {
assert(
data[0].includes(lib) && data[0].includes(`node:${lib}`),
`${lib} not found`,
);
if (!lib.startsWith('node:')) {
assert(
data[0].includes(lib) && data[0].includes(`node:${lib}`),
`${lib} not found`,
);
}
});
const newModule = 'foobar';
assert(!builtinModules.includes(newModule));
Expand All @@ -56,8 +58,10 @@ testMe.complete("import\t( 'n", common.mustCall((error, data) => {
let lastIndex = -1;

publicModules.forEach((lib, index) => {
lastIndex = completions.indexOf(`node:${lib}`);
assert.notStrictEqual(lastIndex, -1);
if (!lib.startsWith('node:')) {
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:
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

0 comments on commit a806bee

Please sign in to comment.