Skip to content

Commit

Permalink
src: allow embedders to disable esm loader
Browse files Browse the repository at this point in the history
PR-URL: #34060
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
  • Loading branch information
codebytere authored and addaleax committed Sep 23, 2020
1 parent 4cd7f5f commit 5e28660
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 4 deletions.
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 @@ -64,7 +67,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 @@ -859,6 +859,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 @@ -1063,6 +1063,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 @@ -396,7 +396,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 @@ -994,6 +994,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

0 comments on commit 5e28660

Please sign in to comment.