lib: always initialize esm loader callbackMap

[codebytere]

Refs #34060.

We should instead unilaterally initialize the callbackMap and return early if an embedder is choosing to not use the Node.js esm loader is in an embedder context.

I also updated the corresponding tests to more directly check importModuleDynamically functionality affected by a decision to not use the esm loader provided by Node.js.

cc @addaleax@joyeecheung

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32158/

[addaleax]

Would it maybe make more sense to provide callbackMap unconditionally? e.g. this (untested):

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
index a51dbf05ec4f..8d4c44922ac0 100644
--- a/lib/internal/bootstrap/pre_execution.js+++ b/lib/internal/bootstrap/pre_execution.js@@ -60,9 +60,7 @@ function prepareMainThreadExecution(expandArgv1 = false) {
initializeWASI();
initializeCJSLoader();
- if (!shouldNotRegisterESMLoader) {- initializeESMLoader();- }+ initializeESMLoader();
const CJSLoader = require('internal/modules/cjs/loader');
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
@@ -406,6 +404,8 @@ function initializeESMLoader() {
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
internalBinding('module_wrap').callbackMap = new SafeWeakMap();
+ if (!shouldNotRegisterESMLoader) return;+
const {
setImportModuleDynamicallyCallback,
setInitializeImportMetaObjectCallback

That requires a smaller patch and would also make things easier in case we do figure out a good way to mix ESM loaders (e.g. by making them per-Context)?

[codebytere]

That also works! Will do :)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32207/

[codebytere]

Landed in 37fc587