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

sea: allow requiring core modules with the "node:" prefix #47779

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
14 changes: 14 additions & 0 deletions lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,20 @@ class BuiltinModule {
return canBeRequiredByUsersWithoutSchemeList.has(id);
}

static normalizeRequirableId(id) {
let normalizedId = id;
if (StringPrototypeStartsWith(id, 'node:')) {
normalizedId = StringPrototypeSlice(id, 5);
}

if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
(id === normalizedId && !BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
RaisinTen marked this conversation as resolved.
Show resolved Hide resolved
return undefined;
}

return normalizedId;
}

static isBuiltin(id) {
return BuiltinModule.canBeRequiredWithoutScheme(id) || (
typeof id === 'string' &&
Expand Down
13 changes: 3 additions & 10 deletions lib/internal/main/mksnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ const {
ObjectSetPrototypeOf,
SafeArrayIterator,
SafeSet,
StringPrototypeStartsWith,
StringPrototypeSlice,
} = primordials;

const binding = internalBinding('mksnapshot');
const { BuiltinModule } = require('internal/bootstrap/realm');
const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
const {
getEmbedderEntryFunction,
compileSerializeMain,
Expand Down Expand Up @@ -98,13 +96,8 @@ function supportedInUserSnapshot(id) {
}

function requireForUserSnapshot(id) {
let normalizedId = id;
if (StringPrototypeStartsWith(id, 'node:')) {
normalizedId = StringPrototypeSlice(id, 5);
}
if (!BuiltinModule.canBeRequiredByUsers(normalizedId) ||
(id !== normalizedId &&
!BuiltinModule.canBeRequiredWithoutScheme(normalizedId))) {
const normalizedId = normalizeRequirableId(id);
if (!normalizedId) {
// eslint-disable-next-line no-restricted-syntax
const err = new Error(
`Cannot find module '${id}'. `,
Expand Down
11 changes: 6 additions & 5 deletions lib/internal/util/embedding.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const { codes: { ERR_UNKNOWN_BUILTIN_MODULE } } = require('internal/errors');
const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm');
const { Module, wrapSafe } = require('internal/modules/cjs/loader');

// This is roughly the same as:
Expand Down Expand Up @@ -36,12 +37,12 @@ function embedderRunCjs(contents) {
customDirname);
}

function embedderRequire(path) {
if (!Module.isBuiltin(path)) {
throw new ERR_UNKNOWN_BUILTIN_MODULE(path);
function embedderRequire(id) {
const normalizedId = normalizeRequirableId(id);
if (!normalizedId) {
throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
}

return require(path);
return require(normalizedId);
}

module.exports = { embedderRequire, embedderRunCjs };
17 changes: 16 additions & 1 deletion test/fixtures/sea.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,23 @@ expectWarning('ExperimentalWarning',
'Single executable application is an experimental feature and ' +
'might change at any time');

// Should be possible to require core modules that optionally require the
// "node:" scheme.
const { deepStrictEqual, strictEqual, throws } = require('assert');
const { dirname } = require('path');
const { dirname } = require('node:path');

// Should be possible to require a core module that requires using the "node:"
// scheme.
{
const { test } = require('node:test');
strictEqual(typeof test, 'function');
}

// Should not be possible to require a core module without the "node:" scheme if
// it requires using the "node:" scheme.
throws(() => require('test'), {
code: 'ERR_UNKNOWN_BUILTIN_MODULE',
});

deepStrictEqual(process.argv, [process.execPath, process.execPath, '-a', '--b=c', 'd']);

Expand Down