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

src: allow embedders to disable esm loader #34060

Closed
Closed
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
10 changes: 8 additions & 2 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ const {
SafeWeakMap,
} = primordials;

const { getOptionValue } = require('internal/options');
const {
getOptionValue,
shouldNotRegisterESMLoader
} = require('internal/options');
const { Buffer } = require('buffer');
const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
const assert = require('internal/assert');
Expand Down Expand Up @@ -56,7 +59,10 @@ function prepareMainThreadExecution(expandArgv1 = false) {
initializeDeprecations();
initializeWASI();
initializeCJSLoader();
initializeESMLoader();

if (!shouldNotRegisterESMLoader) {
initializeESMLoader();
}

const CJSLoader = require('internal/modules/cjs/loader');
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/options.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const { getOptions } = internalBinding('options');
const { getOptions, shouldNotRegisterESMLoader } = internalBinding('options');
const { options, aliases } = getOptions();

let warnOnAllowUnauthorized = true;
Expand Down Expand Up @@ -32,4 +32,5 @@ module.exports = {
aliases,
getOptionValue,
getAllowUnauthorized,
shouldNotRegisterESMLoader
};
4 changes: 4 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,10 @@ inline bool Environment::is_main_thread() const {
return worker_context() == nullptr;
}

inline bool Environment::should_not_register_esm_loader() const {
return flags_ & EnvironmentFlags::kNoRegisterESMLoader;
}

inline bool Environment::owns_process_state() const {
return flags_ & EnvironmentFlags::kOwnsProcessState;
}
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,7 @@ class Environment : public MemoryRetainer {
inline void set_has_serialized_options(bool has_serialized_options);

inline bool is_main_thread() const;
inline bool should_not_register_esm_loader() const;
inline bool owns_process_state() const;
inline bool owns_inspector() const;
inline uint64_t thread_id() const;
Expand Down
6 changes: 5 additions & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,11 @@ enum Flags : uint64_t {
// Set if this Environment instance is associated with the global inspector
// handling code (i.e. listening on SIGUSR1).
// This is set when using kDefaultFlags.
kOwnsInspector = 1 << 2
kOwnsInspector = 1 << 2,
// Set if Node.js should not run its own esm loader. This is needed by some
// embedders, because it's possible for the Node.js esm loader to conflict
// with another one in an embedder environment, e.g. Blink's in Chromium.
kNoRegisterESMLoader = 1 << 3
};
} // namespace EnvironmentFlags

Expand Down
6 changes: 6 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,12 @@ void Initialize(Local<Object> target,
context, FIXED_ONE_BYTE_STRING(isolate, "envSettings"), env_settings)
.Check();

target
->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"),
Boolean::New(isolate, env->should_not_register_esm_loader()))
.Check();

Local<Object> types = Object::New(isolate);
NODE_DEFINE_CONSTANT(types, kNoOp);
NODE_DEFINE_CONSTANT(types, kV8Option);
Expand Down
50 changes: 50 additions & 0 deletions test/cctest/test_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,56 @@ class EnvironmentTest : public EnvironmentTestFixture {
}
};

TEST_F(EnvironmentTest, EnvironmentWithESMLoader) {
const v8::HandleScope handle_scope(isolate_);
Argv argv;
Env env {handle_scope, argv};

node::Environment* envi = *env;
envi->options()->experimental_vm_modules = true;

SetProcessExitHandler(*env, [&](node::Environment* env_, int exit_code) {
EXPECT_EQ(*env, env_);
EXPECT_EQ(exit_code, 0);
node::Stop(*env);
});

node::LoadEnvironment(
*env,
"const { SourceTextModule } = require('vm');"
"try {"
"new SourceTextModule('export const a = 1;');"
"process.exit(0);"
"} catch {"
"process.exit(42);"
"}");
}

TEST_F(EnvironmentTest, EnvironmentWithNoESMLoader) {
const v8::HandleScope handle_scope(isolate_);
Argv argv;
Env env {handle_scope, argv, node::EnvironmentFlags::kNoRegisterESMLoader};

node::Environment* envi = *env;
envi->options()->experimental_vm_modules = true;

SetProcessExitHandler(*env, [&](node::Environment* env_, int exit_code) {
EXPECT_EQ(*env, env_);
EXPECT_EQ(exit_code, 42);
node::Stop(*env);
});

node::LoadEnvironment(
*env,
"const { SourceTextModule } = require('vm');"
"try {"
"new SourceTextModule('export const a = 1;');"
"process.exit(0);"
"} catch {"
"process.exit(42);"
"}");
}

TEST_F(EnvironmentTest, PreExecutionPreparation) {
const v8::HandleScope handle_scope(isolate_);
const Argv argv;
Expand Down