From d4a8a4910d95cbef4b056d1af4cb6ecfe1ed4e92 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Sat, 4 May 2024 00:04:11 +0200
Subject: [PATCH 1/4] benchmark: add require-esm benchmark

---
 benchmark/esm/require-esm.js | 79 ++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 benchmark/esm/require-esm.js

diff --git a/benchmark/esm/require-esm.js b/benchmark/esm/require-esm.js
new file mode 100644
index 00000000000000..8734d0d0bc09e9
--- /dev/null
+++ b/benchmark/esm/require-esm.js
@@ -0,0 +1,79 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const tmpdir = require('../../test/common/tmpdir');
+const path = require('path');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  type: ['all', 'access', 'load'],
+  exports: ['default', 'named'],
+  n: [1000],
+}, {
+  flags: ['--experimental-require-module', '--no-warnings'],
+});
+
+function prepare(count, useDefault) {
+  tmpdir.refresh();
+  const dir = tmpdir.resolve('modules');
+  fs.mkdirSync(dir, { recursive: true });
+  let mainSource = '';
+  let useSource = 'exports.access = function() { return 0';
+  for (let i = 0; i < count; ++i) {
+    let modSource = `const value${i} = 1;\n`;
+    if (useDefault) {
+      modSource += `export default { value${i} }\n`;
+    } else {
+      modSource += `export { value${i} };\n`;
+    }
+    const filename = `mod${i}.mjs`;
+    fs.writeFileSync(
+      path.resolve(dir, filename),
+      modSource,
+      'utf8',
+    );
+    mainSource += `const mod${i} = require('./modules/${filename}');\n`;
+    if (useDefault) {
+      useSource += ` + mod${i}.default.value${i}`;
+    } else {
+      useSource += ` + mod${i}.value${i}`;
+    }
+  }
+  useSource += '; };\n';
+  const script = tmpdir.resolve('main.js');
+  fs.writeFileSync(script, mainSource + useSource, 'utf8');
+  return script;
+}
+
+function main({ n, exports, type }) {
+  const script = prepare(n, exports === 'default');
+  switch (type) {
+    case 'all': {
+      bench.start();
+      const result = require(script).access();
+      bench.end(n);
+      assert.strictEqual(result, n);
+      break;
+    }
+    case 'access': {
+      const { access } = require(script);
+      bench.start();
+      let result = access();
+      for (let i = 0; i < 99; ++i) {
+        result = access();
+      }
+      bench.end(n * 100);
+      assert.strictEqual(result, n);
+      break;
+    }
+    case 'load': {
+      bench.start();
+      const { access } = require(script);
+      bench.end(n);
+      const result = access();
+      assert.strictEqual(result, n);
+      break;
+    }
+  }
+}

From 1035f58d46d69cfd5eb43a014ff16fa806b40f36 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 20 Mar 2024 21:41:22 +0100
Subject: [PATCH 2/4] module: add __esModule to require()'d ESM

Tooling in the ecosystem have been using the __esModule property to
recognize transpiled ESM in consuming code. For example, a 'log'
package written in ESM:

export function log(val) { console.log(val); }

Can be transpiled as:

exports.__esModule = true;
exports.default = function log(val) { console.log(val); }

The consuming code may be written like this in ESM:

import log from 'log'

Which gets transpiled to:

const _mod = require('log');
const log = _mod.__esModule ? _mod.default : _mod;

So to allow transpiled consuming code to recognize require()'d real ESM
as ESM and pick up the default exports, we add a __esModule property by
building a source text module facade for any module that has a default
export and add .__esModule = true to the exports. We don't do this to
modules that don't have default exports to avoid the unnecessary
overhead. This maintains the enumerability of the re-exported names
and the live binding of the exports.

The source of the facade is defined as a constant per-isolate property
required_module_facade_source_string, which looks like this

export * from 'original';
export { default } from 'original';
export const __esModule = true;

And the 'original' module request is always resolved by
createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping
over the original module.
---
 lib/internal/modules/cjs/loader.js            | 54 ++++++++++++--
 lib/internal/modules/esm/loader.js            |  6 +-
 src/env.h                                     |  2 +
 src/env_properties.h                          |  6 ++
 src/module_wrap.cc                            | 70 +++++++++++++++++++
 src/module_wrap.h                             |  3 +
 test/common/index.js                          |  9 ++-
 .../test-require-module-default-extension.js  |  6 +-
 .../test-require-module-dynamic-import-1.js   |  3 +-
 .../test-require-module-dynamic-import-2.js   |  3 +-
 .../test-require-module-dynamic-import-3.js   |  3 +-
 .../test-require-module-dynamic-import-4.js   |  3 +-
 .../es-module/test-require-module-implicit.js |  6 +-
 .../test-require-module-with-detection.js     | 10 +--
 test/es-module/test-require-module.js         | 18 ++---
 test/parallel/test-runner-module-mocking.js   |  2 +-
 16 files changed, 158 insertions(+), 46 deletions(-)

diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index 61286fe760b229..d13c4c623c6ed0 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -41,6 +41,7 @@ const {
   ObjectFreeze,
   ObjectGetOwnPropertyDescriptor,
   ObjectGetPrototypeOf,
+  ObjectHasOwn,
   ObjectKeys,
   ObjectPrototype,
   ObjectPrototypeHasOwnProperty,
@@ -71,7 +72,7 @@ const {
   },
 } = internalBinding('util');
 
-const { kEvaluated } = internalBinding('module_wrap');
+const { kEvaluated, createRequiredModuleFacade } = internalBinding('module_wrap');
 
 // Internal properties for Module instances.
 /**
@@ -1333,9 +1334,54 @@ function loadESMFromCJS(mod, filename) {
     // ESM won't be accessible via process.mainModule.
     setOwnProperty(process, 'mainModule', undefined);
   } else {
-    // TODO(joyeecheung): we may want to invent optional special handling for default exports here.
-    // For now, it's good enough to be identical to what `import()` returns.
-    mod.exports = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, mod[kModuleParent]);
+    const {
+      wrap,
+      namespace,
+    } = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, mod[kModuleParent]);
+    // Tooling in the ecosystem have been using the __esModule property to recognize
+    // transpiled ESM in consuming code. For example, a 'log' package written in ESM:
+    //
+    // export default function log(val) { console.log(val); }
+    //
+    // Can be transpiled as:
+    //
+    // exports.__esModule = true;
+    // exports.default = function log(val) { console.log(val); }
+    //
+    // The consuming code may be written like this in ESM:
+    //
+    // import log from 'log'
+    //
+    // Which gets transpiled to:
+    //
+    // const _mod = require('log');
+    // const log = _mod.__esModule ? _mod.default : _mod;
+    //
+    // So to allow transpiled consuming code to recognize require()'d real ESM
+    // as ESM and pick up the default exports, we add a __esModule property by
+    // building a source text module facade for any module that has a default
+    // export and add .__esModule = true to the exports. We don't do this to
+    // modules that don't have default exports to avoid the unnecessary
+    // overhead. This maintains the enumerability of the re-exported names
+    // and the live binding of the exports.
+    //
+    // The source of the facade is defined as a constant per-isolate property
+    // required_module_default_facade_source_string, which looks like this
+    //
+    // export * from 'original';
+    // export { default } from 'original';
+    // export const __esModule = true;
+    //
+    // And the 'original' module request is always resolved by
+    // createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping
+    // over the original module.
+    const hasDefault = ObjectHasOwn(namespace, 'default');
+    if (namespace.__esModule || !hasDefault) {
+      // There is no need to add .__esModule, just return the original namespace.
+      mod.exports = namespace;
+    } else {
+      mod.exports = createRequiredModuleFacade(wrap, hasDefault);
+    }
   }
 }
 
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index 231f7308e652a6..92491e088ee054 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -280,7 +280,7 @@ class ModuleLoader {
    * @param {string} source Source code. TODO(joyeecheung): pass the raw buffer.
    * @param {string} isMain Whether this module is a main module.
    * @param {CJSModule|undefined} parent Parent module, if any.
-   * @returns {{ModuleWrap}}
+   * @returns {{wrap: ModuleWrap, namespace: ModuleNamespaceObject}}
    */
   importSyncForRequire(mod, filename, source, isMain, parent) {
     const url = pathToFileURL(filename).href;
@@ -305,7 +305,7 @@ class ModuleLoader {
         }
         throw new ERR_REQUIRE_CYCLE_MODULE(message);
       }
-      return job.module.getNamespaceSync();
+      return { wrap: job.module, namespace: job.module.getNamespaceSync() };
     }
     // TODO(joyeecheung): refactor this so that we pre-parse in C++ and hit the
     // cache here, or use a carrier object to carry the compiled module script
@@ -317,7 +317,7 @@ class ModuleLoader {
     job = new ModuleJobSync(this, url, kEmptyObject, wrap, isMain, inspectBrk);
     this.loadCache.set(url, kImplicitTypeAttribute, job);
     mod[kRequiredModuleSymbol] = job.module;
-    return job.runSync().namespace;
+    return { wrap: job.module, namespace: job.runSync().namespace };
   }
 
   /**
diff --git a/src/env.h b/src/env.h
index a864da46fb25b3..4a944e1483f005 100644
--- a/src/env.h
+++ b/src/env.h
@@ -1055,6 +1055,8 @@ class Environment : public MemoryRetainer {
   std::vector<std::string> supported_hash_algorithms;
 #endif  // HAVE_OPENSSL
 
+  v8::Global<v8::Module> temporary_required_module_facade_original;
+
  private:
   inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>,
                                                      v8::Local<v8::Value>),
diff --git a/src/env_properties.h b/src/env_properties.h
index eb5c51b6e2b98d..9df54e532b8630 100644
--- a/src/env_properties.h
+++ b/src/env_properties.h
@@ -256,6 +256,7 @@
   V(openssl_error_stack, "opensslErrorStack")                                  \
   V(options_string, "options")                                                 \
   V(order_string, "order")                                                     \
+  V(original_string, "original")                                               \
   V(output_string, "output")                                                   \
   V(overlapped_string, "overlapped")                                           \
   V(parse_error_string, "Parse Error")                                         \
@@ -289,6 +290,11 @@
   V(regexp_string, "regexp")                                                   \
   V(rename_string, "rename")                                                   \
   V(replacement_string, "replacement")                                         \
+  V(required_module_facade_url_string,                                         \
+    "node:internal/require_module_default_facade")                             \
+  V(required_module_facade_source_string,                                      \
+    "export * from 'original'; export { default } from 'original'; export "    \
+    "const __esModule = true;")                                                \
   V(require_string, "require")                                                 \
   V(resource_string, "resource")                                               \
   V(retry_string, "retry")                                                     \
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index f744ebec3828a9..e730423a19a3f6 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -1019,6 +1019,70 @@ void ModuleWrap::CreateCachedData(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+// This v8::Module::ResolveModuleCallback simply links `import 'original'`
+// to the env->temporary_required_module_facade_original() which is stashed
+// right before this callback is called and will be restored as soon as
+// v8::Module::Instantiate() returns.
+MaybeLocal<Module> LinkRequireFacadeWithOriginal(
+    Local<Context> context,
+    Local<String> specifier,
+    Local<FixedArray> import_attributes,
+    Local<Module> referrer) {
+  Environment* env = Environment::GetCurrent(context);
+  Isolate* isolate = context->GetIsolate();
+  CHECK(specifier->Equals(context, env->original_string()).ToChecked());
+  CHECK(!env->temporary_required_module_facade_original.IsEmpty());
+  return env->temporary_required_module_facade_original.Get(isolate);
+}
+
+// Wraps an existing source text module with a facade that adds
+// .__esModule = true to the exports.
+// See env->required_module_facade_source_string() for the source.
+void ModuleWrap::CreateRequiredModuleFacade(
+    const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+  Environment* env = Environment::GetCurrent(context);
+  CHECK(args[0]->IsObject());  // original module
+  CHECK(args[1]->IsTrue());    // hasDefault
+  Local<Object> wrap = args[0].As<Object>();
+  ModuleWrap* original;
+  ASSIGN_OR_RETURN_UNWRAP(&original, wrap);
+
+  // Use the same facade source and URL to hit the compilation cache.
+  ScriptOrigin origin(env->required_module_facade_url_string(),
+                      0,               // line offset
+                      0,               // column offset
+                      true,            // is cross origin
+                      -1,              // script id
+                      Local<Value>(),  // source map URL
+                      false,           // is opaque (?)
+                      false,           // is WASM
+                      true);           // is ES Module
+  ScriptCompiler::Source source(env->required_module_facade_source_string(),
+                                origin);
+
+  // The module facade instantiation simply links `import 'original'` in the
+  // facade with the original module and should never fail.
+  Local<Module> facade =
+      ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+  // Stash the original module in temporary_required_module_facade_original
+  // for the LinkRequireFacadeWithOriginal() callback to pick it up.
+  CHECK(env->temporary_required_module_facade_original.IsEmpty());
+  env->temporary_required_module_facade_original.Reset(
+      isolate, original->module_.Get(isolate));
+  CHECK(facade->InstantiateModule(context, LinkRequireFacadeWithOriginal)
+            .IsJust());
+  env->temporary_required_module_facade_original.Reset();
+
+  // The evaluation of the facade is synchronous.
+  Local<Value> evaluated = facade->Evaluate(context).ToLocalChecked();
+  CHECK(evaluated->IsPromise());
+  CHECK_EQ(evaluated.As<Promise>()->State(), Promise::PromiseState::kFulfilled);
+
+  args.GetReturnValue().Set(facade->GetModuleNamespace());
+}
+
 void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
                                             Local<ObjectTemplate> target) {
   Isolate* isolate = isolate_data->isolate();
@@ -1051,6 +1115,10 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
             target,
             "setInitializeImportMetaObjectCallback",
             SetInitializeImportMetaObjectCallback);
+  SetMethod(isolate,
+            target,
+            "createRequiredModuleFacade",
+            CreateRequiredModuleFacade);
 }
 
 void ModuleWrap::CreatePerContextProperties(Local<Object> target,
@@ -1091,6 +1159,8 @@ void ModuleWrap::RegisterExternalReferences(
   registry->Register(GetStatus);
   registry->Register(GetError);
 
+  registry->Register(CreateRequiredModuleFacade);
+
   registry->Register(SetImportModuleDynamicallyCallback);
   registry->Register(SetInitializeImportMetaObjectCallback);
 }
diff --git a/src/module_wrap.h b/src/module_wrap.h
index 671f87f76eb3b3..83b5793013cbc4 100644
--- a/src/module_wrap.h
+++ b/src/module_wrap.h
@@ -88,6 +88,9 @@ class ModuleWrap : public BaseObject {
       std::optional<v8::ScriptCompiler::CachedData*> user_cached_data,
       bool* cache_rejected);
 
+  static void CreateRequiredModuleFacade(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
+
  private:
   ModuleWrap(Realm* realm,
              v8::Local<v8::Object> object,
diff --git a/test/common/index.js b/test/common/index.js
index ea21d15c104ac3..71bd772f9fbe3d 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -965,8 +965,13 @@ function getPrintedStackTrace(stderr) {
  * @param {object} expectation shape of expected namespace.
  */
 function expectRequiredModule(mod, expectation) {
-  assert(isModuleNamespaceObject(mod));
-  assert.deepStrictEqual({ ...mod }, { ...expectation });
+  const clone = { ...mod };
+  if (Object.hasOwn(mod, 'default')) {
+    assert.strictEqual(mod.__esModule, true);
+    delete clone.__esModule;
+  }
+  isModuleNamespaceObject(mod);
+  assert.deepStrictEqual(clone, { ...expectation });
 }
 
 const common = {
diff --git a/test/es-module/test-require-module-default-extension.js b/test/es-module/test-require-module-default-extension.js
index 7c49e21aba9a15..c126affe983264 100644
--- a/test/es-module/test-require-module-default-extension.js
+++ b/test/es-module/test-require-module-default-extension.js
@@ -1,13 +1,11 @@
 // Flags: --experimental-require-module
 'use strict';
 
-require('../common');
+const { expectRequiredModule } = require('../common');
 const assert = require('assert');
-const { isModuleNamespaceObject } = require('util/types');
 
 const mod = require('../fixtures/es-modules/package-default-extension/index.mjs');
-assert.deepStrictEqual({ ...mod }, { entry: 'mjs' });
-assert(isModuleNamespaceObject(mod));
+expectRequiredModule(mod, { entry: 'mjs' });
 
 assert.throws(() => {
   const mod = require('../fixtures/es-modules/package-default-extension');
diff --git a/test/es-module/test-require-module-dynamic-import-1.js b/test/es-module/test-require-module-dynamic-import-1.js
index 000e31485f559e..939a2cdbcc93bf 100644
--- a/test/es-module/test-require-module-dynamic-import-1.js
+++ b/test/es-module/test-require-module-dynamic-import-1.js
@@ -21,8 +21,7 @@ const { pathToFileURL } = require('url');
     const url = pathToFileURL(path.resolve(__dirname, id));
     const imported = await import(url);
     const required = require(id);
-    assert.strictEqual(imported, required,
-                       `import()'ed and require()'ed result of ${id} was not reference equal`);
+    common.expectRequiredModule(required, imported);
   }
 
   const id = '../fixtures/es-modules/data-import.mjs';
diff --git a/test/es-module/test-require-module-dynamic-import-2.js b/test/es-module/test-require-module-dynamic-import-2.js
index 6c31c04f0b2e77..a3e24a800c1803 100644
--- a/test/es-module/test-require-module-dynamic-import-2.js
+++ b/test/es-module/test-require-module-dynamic-import-2.js
@@ -21,8 +21,7 @@ const path = require('path');
     const url = pathToFileURL(path.resolve(__dirname, id));
     const required = require(id);
     const imported = await import(url);
-    assert.strictEqual(imported, required,
-                       `import()'ed and require()'ed result of ${id} was not reference equal`);
+    common.expectRequiredModule(required, imported);
   }
 
   const id = '../fixtures/es-modules/data-import.mjs';
diff --git a/test/es-module/test-require-module-dynamic-import-3.js b/test/es-module/test-require-module-dynamic-import-3.js
index 7a5fbf1a137f96..53fcdc48c8c552 100644
--- a/test/es-module/test-require-module-dynamic-import-3.js
+++ b/test/es-module/test-require-module-dynamic-import-3.js
@@ -5,10 +5,9 @@
 // be loaded by dynamic import().
 
 const common = require('../common');
-const assert = require('assert');
 
 (async () => {
   const required = require('../fixtures/es-modules/require-and-import/load.cjs');
   const imported = await import('../fixtures/es-modules/require-and-import/load.mjs');
-  assert.deepStrictEqual({ ...required }, { ...imported });
+  common.expectRequiredModule(required, imported);
 })().then(common.mustCall());
diff --git a/test/es-module/test-require-module-dynamic-import-4.js b/test/es-module/test-require-module-dynamic-import-4.js
index 414cd70d82d33a..88c565d2ba2e47 100644
--- a/test/es-module/test-require-module-dynamic-import-4.js
+++ b/test/es-module/test-require-module-dynamic-import-4.js
@@ -5,10 +5,9 @@
 // be loaded by require().
 
 const common = require('../common');
-const assert = require('assert');
 
 (async () => {
   const imported = await import('../fixtures/es-modules/require-and-import/load.mjs');
   const required = require('../fixtures/es-modules/require-and-import/load.cjs');
-  assert.deepStrictEqual({ ...required }, { ...imported });
+  common.expectRequiredModule(required, imported);
 })().then(common.mustCall());
diff --git a/test/es-module/test-require-module-implicit.js b/test/es-module/test-require-module-implicit.js
index 5b5a4a4bbb47b0..2e3a5d94352dbb 100644
--- a/test/es-module/test-require-module-implicit.js
+++ b/test/es-module/test-require-module-implicit.js
@@ -3,9 +3,8 @@
 
 // Tests that require()ing modules without explicit module type information
 // warns and errors.
-require('../common');
+const common = require('../common');
 const assert = require('assert');
-const { isModuleNamespaceObject } = require('util/types');
 
 assert.throws(() => {
   require('../fixtures/es-modules/package-without-type/noext-esm');
@@ -28,6 +27,5 @@ assert.throws(() => {
     code: 'MODULE_NOT_FOUND'
   });
   const mod = require(`${id}.mjs`);
-  assert.deepStrictEqual({ ...mod }, { hello: 'world' });
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { hello: 'world' });
 }
diff --git a/test/es-module/test-require-module-with-detection.js b/test/es-module/test-require-module-with-detection.js
index 36da19f3b96270..baf993d3253ec7 100644
--- a/test/es-module/test-require-module-with-detection.js
+++ b/test/es-module/test-require-module-with-detection.js
@@ -1,18 +1,14 @@
 // Flags: --experimental-require-module --experimental-detect-module
 'use strict';
 
-require('../common');
-const assert = require('assert');
-const { isModuleNamespaceObject } = require('util/types');
+const common = require('../common');
 
 {
   const mod = require('../fixtures/es-modules/loose.js');
-  assert.deepStrictEqual({ ...mod }, { default: 'module' });
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { default: 'module' });
 }
 
 {
   const mod = require('../fixtures/es-modules/package-without-type/noext-esm');
-  assert.deepStrictEqual({ ...mod }, { default: 'module' });
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { default: 'module' });
 }
diff --git a/test/es-module/test-require-module.js b/test/es-module/test-require-module.js
index 631f5d731a5c86..2cc7d0198837df 100644
--- a/test/es-module/test-require-module.js
+++ b/test/es-module/test-require-module.js
@@ -3,7 +3,6 @@
 
 const common = require('../common');
 const assert = require('assert');
-const { isModuleNamespaceObject } = require('util/types');
 
 common.expectWarning(
   'ExperimentalWarning',
@@ -14,22 +13,19 @@ common.expectWarning(
 // Test named exports.
 {
   const mod = require('../fixtures/es-module-loaders/module-named-exports.mjs');
-  assert.deepStrictEqual({ ...mod }, { foo: 'foo', bar: 'bar' });
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { foo: 'foo', bar: 'bar' });
 }
 
 // Test ESM that import ESM.
 {
   const mod = require('../fixtures/es-modules/import-esm.mjs');
-  assert.deepStrictEqual({ ...mod }, { hello: 'world' });
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { hello: 'world' });
 }
 
 // Test ESM that import CJS.
 {
   const mod = require('../fixtures/es-modules/cjs-exports.mjs');
-  assert.deepStrictEqual({ ...mod }, {});
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { });
 }
 
 // Test ESM that require() CJS.
@@ -39,24 +35,20 @@ common.expectWarning(
   // re-export everything from the CJS version.
   assert.strictEqual(common.mustCall, mjs.mustCall);
   assert.strictEqual(common.localIPv6Hosts, mjs.localIPv6Hosts);
-  assert(!isModuleNamespaceObject(common));
-  assert(isModuleNamespaceObject(mjs));
 }
 
 // Test "type": "module" and "main" field in package.json.
 // Also, test default export.
 {
   const mod = require('../fixtures/es-modules/package-type-module');
-  assert.deepStrictEqual({ ...mod }, { default: 'package-type-module' });
-  assert(isModuleNamespaceObject(mod));
+  common.expectRequiredModule(mod, { default: 'package-type-module' });
 }
 
 // Test data: import.
 {
   const mod = require('../fixtures/es-modules/data-import.mjs');
-  assert.deepStrictEqual({ ...mod }, {
+  common.expectRequiredModule(mod, {
     data: { hello: 'world' },
     id: 'data:text/javascript,export default %7B%20hello%3A%20%22world%22%20%7D'
   });
-  assert(isModuleNamespaceObject(mod));
 }
diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js
index 7b7a743ed15ba6..13b614f6e9fe45 100644
--- a/test/parallel/test-runner-module-mocking.js
+++ b/test/parallel/test-runner-module-mocking.js
@@ -338,7 +338,7 @@ test('ESM mocking with namedExports option', async (t) => {
     assert.strictEqual(mocked.default, 'mock default');
     assert.strictEqual(mocked.val1, 'mock value');
     t.mock.reset();
-    assert.strictEqual(original, require(fixture));
+    common.expectRequiredModule(require(fixture), original);
   });
 
   await t.test('throws if named exports cannot be applied to defaultExport as CJS', async (t) => {

From 911bbb3c2d570bd2fbd5d1025cf66f15d9c06a78 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 5 Jul 2024 18:23:56 +0200
Subject: [PATCH 3/4] fixup! module: add __esModule to require()'d ESM

---
 doc/api/modules.md                            | 36 ++++++++++++++-----
 lib/internal/modules/cjs/loader.js            | 19 +++++-----
 src/module_wrap.cc                            |  1 -
 test/common/index.js                          |  6 ++--
 .../test-require-module-defined-esmodule.js   | 12 +++++++
 .../test-require-module-transpiled.js         | 30 ++++++++++++++++
 test/fixtures/es-modules/export-es-module.mjs |  2 ++
 .../dist/import-both.cjs                      | 27 ++++++++++++++
 .../dist/import-default.cjs                   |  7 ++++
 .../dist/import-named.cjs                     |  4 +++
 .../node_modules/logger/logger.mjs            |  2 ++
 .../node_modules/logger/package.json          |  4 +++
 .../src/import-both.mjs                       |  2 ++
 .../src/import-default.mjs                    |  2 ++
 .../src/import-named.mjs                      |  2 ++
 .../transpile.cjs                             | 23 ++++++++++++
 16 files changed, 157 insertions(+), 22 deletions(-)
 create mode 100644 test/es-module/test-require-module-defined-esmodule.js
 create mode 100644 test/es-module/test-require-module-transpiled.js
 create mode 100644 test/fixtures/es-modules/export-es-module.mjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-both.cjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-default.cjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-named.cjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/logger.mjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/package.json
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/src/import-both.mjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/src/import-default.mjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/src/import-named.mjs
 create mode 100644 test/fixtures/es-modules/transpiled-cjs-require-module/transpile.cjs

diff --git a/doc/api/modules.md b/doc/api/modules.md
index b772632d71256f..70f4e3cf6af651 100644
--- a/doc/api/modules.md
+++ b/doc/api/modules.md
@@ -188,33 +188,51 @@ loaded by `require()` meets the following requirements:
      `"type": "commonjs"`, and `--experimental-detect-module` is enabled.
 
 `require()` will load the requested module as an ES Module, and return
-the module name space object. In this case it is similar to dynamic
+the module namespace object. In this case it is similar to dynamic
 `import()` but is run synchronously and returns the name space object
 directly.
 
+With the following ES Modules:
+
 ```mjs
-// point.mjs
+// distance.mjs
 export function distance(a, b) { return (b.x - a.x) ** 2 + (b.y - a.y) ** 2; }
+```
+
+```mjs
+// point.mjs
 class Point {
   constructor(x, y) { this.x = x; this.y = y; }
 }
 export default Point;
 ```
 
+A CommonJS module can load them with `require()` under `--experimental-detect-module`:
+
 ```cjs
-const required = require('./point.mjs');
+const distance = require('./distance.mjs');
+console.log(distance);
 // [Module: null prototype] {
-//   default: [class Point],
 //   distance: [Function: distance]
 // }
-console.log(required);
 
-(async () => {
-  const imported = await import('./point.mjs');
-  console.log(imported === required);  // true
-})();
+const point = require('./point.mjs');
+console.log(point);
+// [Module: null prototype] {
+//   default: [class Point],
+//   __esModule: true,
+// }
 ```
 
+For interoperability with existing tools that convert ES Modules into CommonJS,
+which could then load real ES Modules through `require()`, the returned namespace
+would contain a `__esModule: true` property if it has a `default` export so that
+consuming code generated by tools can recognize the default exports in real
+ES Modules. If the namespace already defines `__esModule`, this would not be added.
+This property is experimental and can change in the future. It should only be used
+by tools converting ES modules into CommonJS modules, following existing ecosystem
+conventions. Code authored directly in CommonJS should avoid depending on it.
+
 If the module being `require()`'d contains top-level `await`, or the module
 graph it `import`s contains top-level `await`,
 [`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index d13c4c623c6ed0..91e8571887383d 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -1360,10 +1360,9 @@ function loadESMFromCJS(mod, filename) {
     // So to allow transpiled consuming code to recognize require()'d real ESM
     // as ESM and pick up the default exports, we add a __esModule property by
     // building a source text module facade for any module that has a default
-    // export and add .__esModule = true to the exports. We don't do this to
-    // modules that don't have default exports to avoid the unnecessary
-    // overhead. This maintains the enumerability of the re-exported names
-    // and the live binding of the exports.
+    // export and add .__esModule = true to the exports. This maintains the
+    // enumerability of the re-exported names and the live binding of the exports,
+    // without incurring a non-trivial per-access overhead on the exports.
     //
     // The source of the facade is defined as a constant per-isolate property
     // required_module_default_facade_source_string, which looks like this
@@ -1373,14 +1372,16 @@ function loadESMFromCJS(mod, filename) {
     // export const __esModule = true;
     //
     // And the 'original' module request is always resolved by
-    // createRequiredModuleFacade() to wrap which is a ModuleWrap wrapping
+    // createRequiredModuleFacade() to `wrap` which is a ModuleWrap wrapping
     // over the original module.
-    const hasDefault = ObjectHasOwn(namespace, 'default');
-    if (namespace.__esModule || !hasDefault) {
-      // There is no need to add .__esModule, just return the original namespace.
+
+    // We don't do this to modules that don't have default exports to avoid
+    // the unnecessary overhead. If __esModule is already defined, we will
+    // also skip the extension to allow users to override it.
+    if (!ObjectHasOwn(namespace, 'default') || ObjectHasOwn(namespace, '__esModule')) {
       mod.exports = namespace;
     } else {
-      mod.exports = createRequiredModuleFacade(wrap, hasDefault);
+      mod.exports = createRequiredModuleFacade(wrap);
     }
   }
 }
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index e730423a19a3f6..48b61e8b760070 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -1044,7 +1044,6 @@ void ModuleWrap::CreateRequiredModuleFacade(
   Local<Context> context = isolate->GetCurrentContext();
   Environment* env = Environment::GetCurrent(context);
   CHECK(args[0]->IsObject());  // original module
-  CHECK(args[1]->IsTrue());    // hasDefault
   Local<Object> wrap = args[0].As<Object>();
   ModuleWrap* original;
   ASSIGN_OR_RETURN_UNWRAP(&original, wrap);
diff --git a/test/common/index.js b/test/common/index.js
index 71bd772f9fbe3d..02ca35c5c55659 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -964,13 +964,13 @@ function getPrintedStackTrace(stderr) {
  * @param {object} mod result returned by require()
  * @param {object} expectation shape of expected namespace.
  */
-function expectRequiredModule(mod, expectation) {
+function expectRequiredModule(mod, expectation, checkESModule = true) {
   const clone = { ...mod };
-  if (Object.hasOwn(mod, 'default')) {
+  if (Object.hasOwn(mod, 'default') && checkESModule) {
     assert.strictEqual(mod.__esModule, true);
     delete clone.__esModule;
   }
-  isModuleNamespaceObject(mod);
+  assert(isModuleNamespaceObject(mod));
   assert.deepStrictEqual(clone, { ...expectation });
 }
 
diff --git a/test/es-module/test-require-module-defined-esmodule.js b/test/es-module/test-require-module-defined-esmodule.js
new file mode 100644
index 00000000000000..4831be2850cfc2
--- /dev/null
+++ b/test/es-module/test-require-module-defined-esmodule.js
@@ -0,0 +1,12 @@
+// Flags: --experimental-require-module
+'use strict';
+const common = require('../common');
+const mod = require('../fixtures/es-modules/export-es-module.mjs');
+
+// If an ESM already defines __esModule to be something else,
+// require(esm) should allow the user override.
+common.expectRequiredModule(
+  mod,
+  { default: { hello: 'world' }, __esModule: 'test' },
+  false,
+);
diff --git a/test/es-module/test-require-module-transpiled.js b/test/es-module/test-require-module-transpiled.js
new file mode 100644
index 00000000000000..b927507b876370
--- /dev/null
+++ b/test/es-module/test-require-module-transpiled.js
@@ -0,0 +1,30 @@
+'use strict';
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+
+// This is a minimum integration test for CJS transpiled from ESM that tries to load real ESM.
+
+spawnSyncAndAssert(process.execPath, [
+  '--experimental-require-module',
+  fixtures.path('es-modules', 'transpiled-cjs-require-module', 'dist', 'import-both.cjs'),
+], {
+  trim: true,
+  stdout: 'import both',
+});
+
+spawnSyncAndAssert(process.execPath, [
+  '--experimental-require-module',
+  fixtures.path('es-modules', 'transpiled-cjs-require-module', 'dist', 'import-named.cjs'),
+], {
+  trim: true,
+  stdout: 'import named',
+});
+
+spawnSyncAndAssert(process.execPath, [
+  '--experimental-require-module',
+  fixtures.path('es-modules', 'transpiled-cjs-require-module', 'dist', 'import-default.cjs'),
+], {
+  trim: true,
+  stdout: 'import default',
+});
diff --git a/test/fixtures/es-modules/export-es-module.mjs b/test/fixtures/es-modules/export-es-module.mjs
new file mode 100644
index 00000000000000..a85dea6c5e3d00
--- /dev/null
+++ b/test/fixtures/es-modules/export-es-module.mjs
@@ -0,0 +1,2 @@
+export const __esModule = 'test';
+export default { hello: 'world' };
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-both.cjs b/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-both.cjs
new file mode 100644
index 00000000000000..62c00a7f046304
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-both.cjs
@@ -0,0 +1,27 @@
+"use strict";
+var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
+    if (k2 === undefined) k2 = k;
+    var desc = Object.getOwnPropertyDescriptor(m, k);
+    if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
+      desc = { enumerable: true, get: function() { return m[k]; } };
+    }
+    Object.defineProperty(o, k2, desc);
+}) : (function(o, m, k, k2) {
+    if (k2 === undefined) k2 = k;
+    o[k2] = m[k];
+}));
+var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) {
+    Object.defineProperty(o, "default", { enumerable: true, value: v });
+}) : function(o, v) {
+    o["default"] = v;
+});
+var __importStar = (this && this.__importStar) || function (mod) {
+    if (mod && mod.__esModule) return mod;
+    var result = {};
+    if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
+    __setModuleDefault(result, mod);
+    return result;
+};
+Object.defineProperty(exports, "__esModule", { value: true });
+var logger_1 = __importStar(require("logger"));
+(0, logger_1.log)(new logger_1.default(), 'import both');
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-default.cjs b/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-default.cjs
new file mode 100644
index 00000000000000..9880e20de5c91a
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-default.cjs
@@ -0,0 +1,7 @@
+"use strict";
+var __importDefault = (this && this.__importDefault) || function (mod) {
+    return (mod && mod.__esModule) ? mod : { "default": mod };
+};
+Object.defineProperty(exports, "__esModule", { value: true });
+var logger_1 = __importDefault(require("logger"));
+new logger_1.default().log('import default');
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-named.cjs b/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-named.cjs
new file mode 100644
index 00000000000000..9daf6c92ff566d
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/dist/import-named.cjs
@@ -0,0 +1,4 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+var logger_1 = require("logger");
+(0, logger_1.log)(console, 'import named');
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/logger.mjs b/test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/logger.mjs
new file mode 100644
index 00000000000000..abcac43fff428f
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/logger.mjs
@@ -0,0 +1,2 @@
+export default class Logger { log(val) { console.log(val); } }
+export function log(logger, val) { logger.log(val) };
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/package.json b/test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/package.json
new file mode 100644
index 00000000000000..56c237df953d7a
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/node_modules/logger/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "logger",
+  "main": "logger.mjs"
+}
\ No newline at end of file
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-both.mjs b/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-both.mjs
new file mode 100644
index 00000000000000..7773ccb2bc1d20
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-both.mjs
@@ -0,0 +1,2 @@
+import Logger, { log } from 'logger';
+log(new Logger(), 'import both');
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-default.mjs b/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-default.mjs
new file mode 100644
index 00000000000000..16c123bbccb0f9
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-default.mjs
@@ -0,0 +1,2 @@
+import Logger from 'logger';
+new Logger().log('import default');
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-named.mjs b/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-named.mjs
new file mode 100644
index 00000000000000..489d0886e542f7
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/src/import-named.mjs
@@ -0,0 +1,2 @@
+import { log } from 'logger';
+log(console, 'import named');
diff --git a/test/fixtures/es-modules/transpiled-cjs-require-module/transpile.cjs b/test/fixtures/es-modules/transpiled-cjs-require-module/transpile.cjs
new file mode 100644
index 00000000000000..da9978164b7c6b
--- /dev/null
+++ b/test/fixtures/es-modules/transpiled-cjs-require-module/transpile.cjs
@@ -0,0 +1,23 @@
+'use strict';
+
+// This script is used to transpile ESM fixtures from the src/ directory
+// to CJS modules in dist/. The transpiled CJS files are used to test
+// integration of transpiled CJS modules loading real ESM.
+
+const { readFileSync, writeFileSync, readdirSync } = require('node:fs');
+
+// We use typescript.js because it's already in the code base as a fixture.
+// Most ecosystem tools follow a similar pattern, and this produces a bare
+// minimum integration test for existing patterns.
+const ts = require('../../snapshot/typescript');
+const { join } = require('node:path');
+const sourceDir = join(__dirname, 'src');
+const files = readdirSync(sourceDir);
+for (const filename of files) {
+  const filePath = join(sourceDir, filename);
+  const source = readFileSync(filePath, 'utf8');
+  const { outputText } = ts.transpileModule(source, {
+    compilerOptions: { module: ts.ModuleKind.NodeNext }
+  });
+  writeFileSync(join(__dirname, 'dist', filename.replace('.mjs', '.cjs')), outputText, 'utf8');
+}

From ae54d52b1282ef633736c4b4b938fdffc3be8eaa Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 10 Jul 2024 16:15:27 +0200
Subject: [PATCH 4/4] fixup! fixup! module: add __esModule to require()'d ESM

---
 .../test-require-module-defined-esmodule.js   | 23 ++++++++++++++-----
 .../es-modules/export-es-module-2.mjs         |  2 ++
 2 files changed, 19 insertions(+), 6 deletions(-)
 create mode 100644 test/fixtures/es-modules/export-es-module-2.mjs

diff --git a/test/es-module/test-require-module-defined-esmodule.js b/test/es-module/test-require-module-defined-esmodule.js
index 4831be2850cfc2..68225ebdbd93cd 100644
--- a/test/es-module/test-require-module-defined-esmodule.js
+++ b/test/es-module/test-require-module-defined-esmodule.js
@@ -1,12 +1,23 @@
 // Flags: --experimental-require-module
 'use strict';
 const common = require('../common');
-const mod = require('../fixtures/es-modules/export-es-module.mjs');
 
 // If an ESM already defines __esModule to be something else,
 // require(esm) should allow the user override.
-common.expectRequiredModule(
-  mod,
-  { default: { hello: 'world' }, __esModule: 'test' },
-  false,
-);
+{
+  const mod = require('../fixtures/es-modules/export-es-module.mjs');
+  common.expectRequiredModule(
+    mod,
+    { default: { hello: 'world' }, __esModule: 'test' },
+    false,
+  );
+}
+
+{
+  const mod = require('../fixtures/es-modules/export-es-module-2.mjs');
+  common.expectRequiredModule(
+    mod,
+    { default: { hello: 'world' }, __esModule: false },
+    false,
+  );
+}
diff --git a/test/fixtures/es-modules/export-es-module-2.mjs b/test/fixtures/es-modules/export-es-module-2.mjs
new file mode 100644
index 00000000000000..81f61095ce75af
--- /dev/null
+++ b/test/fixtures/es-modules/export-es-module-2.mjs
@@ -0,0 +1,2 @@
+export const __esModule = false;
+export default { hello: 'world' };