module: disallow CJS <-> ESM edges in a cycle from require(esm)

This patch disallows CJS <-> ESM edges when they come from
require(esm) requested in ESM evalaution.

Drive-by: don't reuse the cache for imported CJS modules to stash
source code of required ESM because the former is also used for
cycle detection.

PR-URL: https://github.com/nodejs/node/pull/52264
Fixes: https://github.com/nodejs/node/issues/52145
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

doc/api/errors.md

@@ -2489,6 +2489,21 @@ Accessing `Object.prototype.__proto__` has been forbidden using
 [`Object.setPrototypeOf`][] should be used to get and set the prototype of an
 object.
 
+<a id="ERR_REQUIRE_CYCLE_MODULE"></a>
+
+### `ERR_REQUIRE_CYCLE_MODULE`
+
+> Stability: 1 - Experimental
+
+When trying to `require()` a [ES Module][] under `--experimental-require-module`,
+a CommonJS to ESM or ESM to CommonJS edge participates in an immediate cycle.
+This is not allowed because ES Modules cannot be evaluated while they are
+already being evaluated.
+
+To avoid the cycle, the `require()` call involved in a cycle should not happen
+at the top-level of either a ES Module (via `createRequire()`) or a CommonJS
+module, and should be done lazily in an inner function.
+
 <a id="ERR_REQUIRE_ASYNC_MODULE"></a>
 
 ### `ERR_REQUIRE_ASYNC_MODULE`

lib/internal/errors.js

@@ -1683,6 +1683,7 @@ E('ERR_PARSE_ARGS_UNKNOWN_OPTION', (option, allowPositionals) => {
 E('ERR_PERFORMANCE_INVALID_TIMESTAMP',
   '%d is not a valid timestamp', TypeError);
 E('ERR_PERFORMANCE_MEASURE_INVALID_OPTIONS', '%s', TypeError);
+E('ERR_REQUIRE_CYCLE_MODULE', '%s', Error);
 E('ERR_REQUIRE_ESM',
   function(filename, hasEsmSyntax, parentPath = null, packageJsonPath = null) {
     hideInternalStackFrames(this);

lib/internal/modules/cjs/loader.js

@@ -63,25 +63,34 @@ const {
   Symbol,
 } = primordials;
 
+const { kEvaluated } = internalBinding('module_wrap');
+
 // Map used to store CJS parsing data or for ESM loading.
-const cjsSourceCache = new SafeWeakMap();
+const importedCJSCache = new SafeWeakMap();
 /**
  * Map of already-loaded CJS modules to use.
  */
 const cjsExportsCache = new SafeWeakMap();
+const requiredESMSourceCache = new SafeWeakMap();
 
+const kIsMainSymbol = Symbol('kIsMainSymbol');
+const kIsCachedByESMLoader = Symbol('kIsCachedByESMLoader');
+const kRequiredModuleSymbol = Symbol('kRequiredModuleSymbol');
+const kIsExecuting = Symbol('kIsExecuting');
 // Set first due to cycle with ESM loader functions.
 module.exports = {
   cjsExportsCache,
-  cjsSourceCache,
+  importedCJSCache,
   initializeCJS,
   entryPointSource: undefined, // Set below.
   Module,
   wrapSafe,
+  kIsMainSymbol,
+  kIsCachedByESMLoader,
+  kRequiredModuleSymbol,
+  kIsExecuting,
 };
 
-const kIsMainSymbol = Symbol('kIsMainSymbol');
-
 const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   maybeCacheSourceMap,
@@ -138,6 +147,7 @@ const {
   codes: {
     ERR_INVALID_ARG_VALUE,
     ERR_INVALID_MODULE_SPECIFIER,
+    ERR_REQUIRE_CYCLE_MODULE,
     ERR_REQUIRE_ESM,
     ERR_UNKNOWN_BUILTIN_MODULE,
   },
@@ -942,6 +952,16 @@ const CircularRequirePrototypeWarningProxy = new Proxy({}, {
  * @param {Module} module The module instance
  */
 function getExportsForCircularRequire(module) {
+  const requiredESM = module[kRequiredModuleSymbol];
+  if (requiredESM && requiredESM.getStatus() !== kEvaluated) {
+    let message = `Cannot require() ES Module ${module.id} in a cycle.`;
+    const parent = moduleParentCache.get(module);
+    if (parent) {
+      message += ` (from ${parent.filename})`;
+    }
+    throw new ERR_REQUIRE_CYCLE_MODULE(message);
+  }
+
   if (module.exports &&
       !isProxy(module.exports) &&
       ObjectGetPrototypeOf(module.exports) === ObjectPrototype &&
@@ -1009,11 +1029,21 @@ Module._load = function(request, parent, isMain) {
   if (cachedModule !== undefined) {
     updateChildren(parent, cachedModule, true);
     if (!cachedModule.loaded) {
-      const parseCachedModule = cjsSourceCache.get(cachedModule);
-      if (!parseCachedModule || parseCachedModule.loaded) {
+      // If it's not cached by the ESM loader, the loading request
+      // comes from required CJS, and we can consider it a circular
+      // dependency when it's cached.
+      if (!cachedModule[kIsCachedByESMLoader]) {
         return getExportsForCircularRequire(cachedModule);
       }
-      parseCachedModule.loaded = true;
+      // If it's cached by the ESM loader as a way to indirectly pass
+      // the module in to avoid creating it twice, the loading request
+      // come from imported CJS. In that case use the importedCJSCache
+      // to determine if it's loading or not.
+      const importedCJSMetadata = importedCJSCache.get(cachedModule);
+      if (importedCJSMetadata.loading) {
+        return getExportsForCircularRequire(cachedModule);
+      }
+      importedCJSMetadata.loading = true;
     } else {
       return cachedModule.exports;
     }
@@ -1027,18 +1057,21 @@ Module._load = function(request, parent, isMain) {
   // Don't call updateChildren(), Module constructor already does.
   const module = cachedModule || new Module(filename, parent);
 
-  if (isMain) {
-    setOwnProperty(process, 'mainModule', module);
-    setOwnProperty(module.require, 'main', process.mainModule);
-    module.id = '.';
-    module[kIsMainSymbol] = true;
-  } else {
-    module[kIsMainSymbol] = false;
+  if (!cachedModule) {
+    if (isMain) {
+      setOwnProperty(process, 'mainModule', module);
+      setOwnProperty(module.require, 'main', process.mainModule);
+      module.id = '.';
+      module[kIsMainSymbol] = true;
+    } else {
+      module[kIsMainSymbol] = false;
+    }
+
+    reportModuleToWatchMode(filename);
+    Module._cache[filename] = module;
+    module[kIsCachedByESMLoader] = false;
   }
 
-  reportModuleToWatchMode(filename);
-
-  Module._cache[filename] = module;
   if (parent !== undefined) {
     relativeResolveCache[relResolveCacheIdentifier] = filename;
   }
@@ -1280,7 +1313,7 @@ function loadESMFromCJS(mod, filename) {
   const isMain = mod[kIsMainSymbol];
   // 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(filename, source, isMain);
+  mod.exports = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, moduleParentCache.get(mod));
 }
 
 /**
@@ -1373,7 +1406,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
   // Only modules being require()'d really need to avoid TLA.
   if (loadAsESM) {
     // Pass the source into the .mjs extension handler indirectly through the cache.
-    cjsSourceCache.set(this, { source: content });
+    requiredESMSourceCache.set(this, content);
     loadESMFromCJS(this, filename);
     return;
   }
@@ -1414,6 +1447,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
   const module = this;
   if (requireDepth === 0) { statCache = new SafeMap(); }
   setHasStartedUserCJSExecution();
+  this[kIsExecuting] = true;
   if (inspectorWrapper) {
     result = inspectorWrapper(compiledWrapper, thisValue, exports,
                               require, module, filename, dirname);
@@ -1421,6 +1455,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
     result = ReflectApply(compiledWrapper, thisValue,
                           [exports, require, module, filename, dirname]);
   }
+  this[kIsExecuting] = false;
   if (requireDepth === 0) { statCache = null; }
   return result;
 };
@@ -1432,7 +1467,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
  * @returns {string}
  */
 function getMaybeCachedSource(mod, filename) {
-  const cached = cjsSourceCache.get(mod);
+  const cached = importedCJSCache.get(mod);
   let content;
   if (cached?.source) {
     content = cached.source;
@@ -1440,7 +1475,7 @@ function getMaybeCachedSource(mod, filename) {
   } else {
     // TODO(joyeecheung): we can read a buffer instead to speed up
     // compilation.
-    content = fs.readFileSync(filename, 'utf8');
+    content = requiredESMSourceCache.get(mod) ?? fs.readFileSync(filename, 'utf8');
   }
   return content;
 }

lib/internal/modules/esm/load.js

@@ -152,6 +152,11 @@ async function defaultLoad(url, context = kEmptyObject) {
 
   validateAttributes(url, format, importAttributes);
 
+  // Use the synchronous commonjs translator which can deal with cycles.
+  if (format === 'commonjs' && getOptionValue('--experimental-require-module')) {
+    format = 'commonjs-sync';
+  }
+
   return {
     __proto__: null,
     format,
@@ -201,6 +206,11 @@ function defaultLoadSync(url, context = kEmptyObject) {
 
   validateAttributes(url, format, importAttributes);
 
+  // Use the synchronous commonjs translator which can deal with cycles.
+  if (format === 'commonjs' && getOptionValue('--experimental-require-module')) {
+    format = 'commonjs-sync';
+  }
+
   return {
     __proto__: null,
     format,

lib/internal/modules/esm/loader.js

@@ -1,7 +1,10 @@
 'use strict';
 
 // This is needed to avoid cycles in esm/resolve <-> cjs/loader
-require('internal/modules/cjs/loader');
+const {
+  kIsExecuting,
+  kRequiredModuleSymbol,
+} = require('internal/modules/cjs/loader');
 
 const {
   ArrayPrototypeJoin,
@@ -15,8 +18,11 @@ const {
   hardenRegExp,
 } = primordials;
 
+const { imported_cjs_symbol } = internalBinding('symbols');
+
 const assert = require('internal/assert');
 const {
+  ERR_REQUIRE_CYCLE_MODULE,
   ERR_REQUIRE_ESM,
   ERR_NETWORK_IMPORT_DISALLOWED,
   ERR_UNKNOWN_MODULE_FORMAT,
@@ -30,7 +36,10 @@ const {
 } = require('internal/modules/esm/utils');
 const { kImplicitAssertType } = require('internal/modules/esm/assert');
 const { canParse } = internalBinding('url');
-const { ModuleWrap } = internalBinding('module_wrap');
+const { ModuleWrap, kEvaluating, kEvaluated } = internalBinding('module_wrap');
+const {
+  urlToFilename,
+} = require('internal/modules/helpers');
 let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer;
 
 /**
@@ -248,17 +257,36 @@ class ModuleLoader {
   /**
    * This constructs (creates, instantiates and evaluates) a module graph that
    * is require()'d.
+   * @param {import('../cjs/loader.js').Module} mod CJS module wrapper of the ESM.
    * @param {string} filename Resolved filename of the module being require()'d
    * @param {string} source Source code. TODO(joyeecheung): pass the raw buffer.
    * @param {string} isMain Whether this module is a main module.
-   * @returns {ModuleNamespaceObject}
+   * @param {import('../cjs/loader.js').Module|undefined} parent Parent module, if any.
+   * @returns {{ModuleWrap}}
    */
-  importSyncForRequire(filename, source, isMain) {
+  importSyncForRequire(mod, filename, source, isMain, parent) {
     const url = pathToFileURL(filename).href;
     let job = this.loadCache.get(url, kImplicitAssertType);
-    // This module is already loaded, check whether it's synchronous and return the
-    // namespace.
+    // This module job is already created:
+    // 1. If it was loaded by `require()` before, at this point the instantiation
+    //    is already completed and we can check the whether it is in a cycle
+    //    (in that case the module status is kEvaluaing), and whether the
+    //    required graph is synchronous.
+    // 2. If it was loaded by `import` before, only allow it if it's already evaluated
+    //    to forbid cycles.
+    //    TODO(joyeecheung): ensure that imported synchronous graphs are evaluated
+    //    synchronously so that any previously imported synchronous graph is already
+    //    evaluated at this point.
     if (job !== undefined) {
+      mod[kRequiredModuleSymbol] = job.module;
+      if (job.module.getStatus() !== kEvaluated) {
+        const parentFilename = urlToFilename(parent?.filename);
+        let message = `Cannot require() ES Module ${filename} in a cycle.`;
+        if (parentFilename) {
+          message += ` (from ${parentFilename})`;
+        }
+        throw new ERR_REQUIRE_CYCLE_MODULE(message);
+      }
       return job.module.getNamespaceSync();
     }
     // TODO(joyeecheung): refactor this so that we pre-parse in C++ and hit the
@@ -270,6 +298,7 @@ class ModuleLoader {
     const { ModuleJobSync } = require('internal/modules/esm/module_job');
     job = new ModuleJobSync(this, url, kEmptyObject, wrap, isMain, inspectBrk);
     this.loadCache.set(url, kImplicitAssertType, job);
+    mod[kRequiredModuleSymbol] = job.module;
     return job.runSync().namespace;
   }
 
@@ -304,19 +333,29 @@ class ModuleLoader {
     const resolvedImportAttributes = resolveResult.importAttributes ?? importAttributes;
     let job = this.loadCache.get(url, resolvedImportAttributes.type);
     if (job !== undefined) {
-      // This module is previously imported before. We will return the module now and check
-      // asynchronicity of the entire graph later, after the graph is instantiated.
+      // This module is being evaluated, which means it's imported in a previous link
+      // in a cycle.
+      if (job.module.getStatus() === kEvaluating) {
+        const parentFilename = urlToFilename(parentURL);
+        let message = `Cannot import Module ${specifier} in a cycle.`;
+        if (parentFilename) {
+          message += ` (from ${parentFilename})`;
+        }
+        throw new ERR_REQUIRE_CYCLE_MODULE(message);
+      }
+      // Othersie the module could be imported before but the evaluation may be already
+      // completed (e.g. the require call is lazy) so it's okay. We will return the
+      // module now and check asynchronicity of the entire graph later, after the
+      // graph is instantiated.
       return job.module;
     }
 
     defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
     const loadResult = defaultLoadSync(url, { format, importAttributes });
     const { responseURL, source } = loadResult;
-    let { format: finalFormat } = loadResult;
+    const { format: finalFormat } = loadResult;
     this.validateLoadResult(url, finalFormat);
-    if (finalFormat === 'commonjs') {
-      finalFormat = 'commonjs-sync';
-    } else if (finalFormat === 'wasm') {
+    if (finalFormat === 'wasm') {
       assert.fail('WASM is currently unsupported by require(esm)');
     }
 
@@ -333,6 +372,20 @@ class ModuleLoader {
       process.send({ 'watch:import': [url] });
     }
 
+    const cjsModule = wrap[imported_cjs_symbol];
+    if (cjsModule) {
+      assert(finalFormat === 'commonjs-sync');
+      // Check if the ESM initiating import CJS is being required by the same CJS module.
+      if (cjsModule && cjsModule[kIsExecuting]) {
+        const parentFilename = urlToFilename(parentURL);
+        let message = `Cannot import CommonJS Module ${specifier} in a cycle.`;
+        if (parentFilename) {
+          message += ` (from ${parentFilename})`;
+        }
+        throw new ERR_REQUIRE_CYCLE_MODULE(message);
+      }
+    }
+
     const inspectBrk = (isMain && getOptionValue('--inspect-brk'));
     const { ModuleJobSync } = require('internal/modules/esm/module_job');
     job = new ModuleJobSync(this, url, importAttributes, wrap, isMain, inspectBrk);

lib/internal/modules/esm/translators.js

@@ -43,9 +43,10 @@ const {
   stripBOM,
 } = require('internal/modules/helpers');
 const {
-  Module: CJSModule,
-  cjsSourceCache,
   cjsExportsCache,
+  importedCJSCache,
+  kIsCachedByESMLoader,
+  Module: CJSModule,
 } = require('internal/modules/cjs/loader');
 const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
 let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
@@ -305,8 +306,7 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
       this.setExport(exportName, value);
     }
     this.setExport('default', exports);
-  });
-
+  }, module);
 }
 
 translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) {
@@ -315,7 +315,7 @@ translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) {
 
   return createCJSModuleWrap(url, source, isMain, (module, source, url, filename) => {
     assert(module === CJSModule._cache[filename]);
-    CJSModule._load(filename, null, false);
+    CJSModule._load(filename);
   });
 });
 
@@ -367,7 +367,7 @@ function cjsPreparseModuleExports(filename, source) {
   // TODO: Do we want to keep hitting the user mutable CJS loader here?
   let module = CJSModule._cache[filename];
   if (module) {
-    const cached = cjsSourceCache.get(module);
+    const cached = importedCJSCache.get(module);
     if (cached) {
       return { module, exportNames: cached.exportNames };
     }
@@ -377,6 +377,7 @@ function cjsPreparseModuleExports(filename, source) {
     module = new CJSModule(filename);
     module.filename = filename;
     module.paths = CJSModule._nodeModulePaths(module.path);
+    module[kIsCachedByESMLoader] = true;
     CJSModule._cache[filename] = module;
   }
 
@@ -391,7 +392,7 @@ function cjsPreparseModuleExports(filename, source) {
   const exportNames = new SafeSet(new SafeArrayIterator(exports));
 
   // Set first for cycles.
-  cjsSourceCache.set(module, { source, exportNames });
+  importedCJSCache.set(module, { source, exportNames });
 
   if (reexports.length) {
     module.filename = filename;

lib/internal/modules/helpers.js

@@ -353,6 +353,15 @@ function shouldRetryAsESM(errorMessage, source) {
   return false;
 }
 
+/**
+ * @param {string|undefined} url URL to convert to filename
+ */
+function urlToFilename(url) {
+  if (url && StringPrototypeStartsWith(url, 'file://')) {
+    return fileURLToPath(url);
+  }
+  return url;
+}
 
 // Whether we have started executing any user-provided CJS code.
 // This is set right before we call the wrapped CJS code (not after,
@@ -388,4 +397,5 @@ module.exports = {
   setHasStartedUserESMExecution() {
     _hasStartedUserESMExecution = true;
   },
+  urlToFilename,
 };

src/env_properties.h

@@ -41,6 +41,7 @@
   V(handle_onclose_symbol, "handle_onclose")                                   \
   V(no_message_symbol, "no_message_symbol")                                    \
   V(messaging_deserialize_symbol, "messaging_deserialize_symbol")              \
+  V(imported_cjs_symbol, "imported_cjs_symbol")                                \
   V(messaging_transfer_symbol, "messaging_transfer_symbol")                    \
   V(messaging_clone_symbol, "messaging_clone_symbol")                          \
   V(messaging_transfer_list_symbol, "messaging_transfer_list_symbol")          \

src/module_wrap.cc

@@ -144,8 +144,8 @@ v8::Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
 
 // new ModuleWrap(url, context, source, lineOffset, columnOffset, cachedData)
 // new ModuleWrap(url, context, source, lineOffset, columOffset,
-// hostDefinedOption) new ModuleWrap(url, context, exportNames,
-// syntheticExecutionFunction)
+//                hostDefinedOption)
+// new ModuleWrap(url, context, exportNames, evaluationCallback[, cjsModule])
 void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
   CHECK(args.IsConstructCall());
   CHECK_GE(args.Length(), 3);
@@ -179,7 +179,8 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
       PrimitiveArray::New(isolate, HostDefinedOptions::kLength);
   Local<Symbol> id_symbol;
   if (synthetic) {
-    // new ModuleWrap(url, context, exportNames, syntheticExecutionFunction)
+    // new ModuleWrap(url, context, exportNames, evaluationCallback[,
+    // cjsModule])
     CHECK(args[3]->IsFunction());
   } else {
     // new ModuleWrap(url, context, source, lineOffset, columOffset, cachedData)
@@ -294,6 +295,12 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  if (synthetic && args[4]->IsObject() &&
+      that->Set(context, realm->isolate_data()->imported_cjs_symbol(), args[4])
+          .IsNothing()) {
+    return;
+  }
+
   // Use the extras object as an object whose GetCreationContext() will be the
   // original `context`, since the `Context` itself strictly speaking cannot
   // be stored in an internal field.
@@ -653,14 +660,12 @@ void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
     case v8::Module::Status::kUninstantiated:
     case v8::Module::Status::kInstantiating:
       return realm->env()->ThrowError(
-          "cannot get namespace, module has not been instantiated");
-    case v8::Module::Status::kEvaluating:
-      return THROW_ERR_REQUIRE_ASYNC_MODULE(realm->env());
+          "Cannot get namespace, module has not been instantiated");
     case v8::Module::Status::kInstantiated:
     case v8::Module::Status::kEvaluated:
     case v8::Module::Status::kErrored:
       break;
-    default:
+    case v8::Module::Status::kEvaluating:
       UNREACHABLE();
   }
 

test/es-module/test-require-module-cycle-esm-cjs-esm-esm.js

@@ -0,0 +1,56 @@
+'use strict';
+
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+
+// a.mjs -> b.cjs -> c.mjs -> a.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-esm-cycle/a.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stderr: /Cannot import Module \.\/a\.mjs in a cycle\. \(from .*c\.mjs\)/,
+    }
+  );
+}
+
+// b.cjs -> c.mjs -> a.mjs -> b.cjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-esm-cycle/b.cjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stderr: /Cannot import CommonJS Module \.\/b\.cjs in a cycle\. \(from .*a\.mjs\)/,
+    }
+  );
+}
+
+// c.mjs -> a.mjs -> b.cjs -> c.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-esm-cycle/c.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stderr: /Cannot require\(\) ES Module .*c\.mjs in a cycle\. \(from .*b\.cjs\)/,
+    }
+  );
+}

test/es-module/test-require-module-cycle-esm-cjs-esm.js

@@ -0,0 +1,72 @@
+'use strict';
+
+require('../common');
+const { spawnSyncAndExit, spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+
+// require-a.cjs -> a.mjs -> b.cjs -> a.mjs.
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-cycle/require-a.cjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stderr: /Cannot require\(\) ES Module .*a\.mjs in a cycle\. \(from .*require-a\.cjs\)/,
+    }
+  );
+}
+
+// require-b.cjs -> b.cjs -> a.mjs -> b.cjs.
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-cycle/require-b.cjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stderr: /Cannot import CommonJS Module \.\/b\.cjs in a cycle\. \(from .*a\.mjs\)/,
+    }
+  );
+}
+
+// a.mjs -> b.cjs -> a.mjs
+{
+  spawnSyncAndExit(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-cycle/a.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      stderr: /Cannot require\(\) ES Module .*a\.mjs in a cycle\. \(from .*b\.cjs\)/,
+    }
+  );
+}
+
+// b.cjs -> a.mjs -> b.cjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-cjs-esm-cycle/b.cjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stderr: /Cannot import CommonJS Module \.\/b\.cjs in a cycle\. \(from .*a\.mjs\)/,
+    }
+  );
+}

test/es-module/test-require-module-cycle-esm-esm-cjs-esm-esm.js

@@ -0,0 +1,70 @@
+'use strict';
+
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+
+// a.mjs -> b.mjs -> c.cjs -> z.mjs -> a.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-esm-cycle/a.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      stderr: /Cannot import Module \.\/a\.mjs in a cycle\. \(from .*z\.mjs\)/,
+    }
+  );
+}
+
+// b.mjs -> c.cjs -> z.mjs -> a.mjs -> b.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-esm-cycle/b.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      stderr: /Cannot import Module \.\/b\.mjs in a cycle\. \(from .*a\.mjs\)/,
+    }
+  );
+}
+
+// c.cjs -> z.mjs -> a.mjs -> b.mjs -> c.cjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-esm-cycle/c.cjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      stderr: /Cannot import CommonJS Module \.\/c\.cjs in a cycle\. \(from .*b\.mjs\)/,
+    }
+  );
+}
+
+
+// z.mjs -> a.mjs -> b.mjs -> c.cjs -> z.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-esm-cycle/z.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      stderr: /Cannot require\(\) ES Module .*z\.mjs in a cycle\. \(from .*c\.cjs\)/,
+    }
+  );
+}

test/es-module/test-require-module-cycle-esm-esm-cjs-esm.js

@@ -0,0 +1,82 @@
+'use strict';
+
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+
+// a.mjs -> b.mjs -> c.mjs -> d.mjs -> c.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-cycle/a.mjs'),
+    ],
+    {
+      signal: null,
+      status: 0,
+      trim: true,
+      stdout(output) {
+        assert.match(output, /Start c/);
+        assert.match(output, /dynamic import b\.mjs failed.*ERR_REQUIRE_CYCLE_MODULE/);
+        assert.match(output, /dynamic import d\.mjs failed.*ERR_REQUIRE_CYCLE_MODULE/);
+      }
+    }
+  );
+}
+
+// b.mjs -> c.mjs -> d.mjs -> c.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-cycle/b.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stdout: /Start c/,
+      stderr: /Cannot import Module \.\/c\.mjs in a cycle\. \(from .*d\.mjs\)/,
+    }
+  );
+}
+
+// c.mjs -> d.mjs -> c.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-cycle/c.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stdout: /Start c/,
+      stderr: /Cannot import Module \.\/c\.mjs in a cycle\. \(from .*d\.mjs\)/,
+    }
+  );
+}
+
+
+// d.mjs -> c.mjs -> d.mjs
+{
+  spawnSyncAndAssert(
+    process.execPath,
+    [
+      '--experimental-require-module',
+      fixtures.path('es-modules/esm-esm-cjs-esm-cycle/d.mjs'),
+    ],
+    {
+      signal: null,
+      status: 1,
+      trim: true,
+      stdout: /Start c/,
+      stderr: /Cannot require\(\) ES Module .*d\.mjs in a cycle\. \(from .*c\.mjs\)/,
+    }
+  );
+}

test/fixtures/es-modules/esm-cjs-esm-cycle/a.mjs

@@ -0,0 +1,3 @@
+import result from './b.cjs';
+export default 'hello';
+console.log('import b.cjs from a.mjs', result);

test/fixtures/es-modules/esm-cjs-esm-cycle/b.cjs

@@ -0,0 +1,3 @@
+const result = require('./a.mjs');
+module.exports = result;
+console.log('require a.mjs in b.cjs', result.default);

test/fixtures/es-modules/esm-cjs-esm-cycle/require-a.cjs

@@ -0,0 +1 @@
+require('./a.mjs');

test/fixtures/es-modules/esm-cjs-esm-cycle/require-b.cjs

@@ -0,0 +1 @@
+require('./b.cjs');

test/fixtures/es-modules/esm-cjs-esm-esm-cycle/a.mjs

@@ -0,0 +1 @@
+import './b.cjs'

test/fixtures/es-modules/esm-cjs-esm-esm-cycle/b.cjs

@@ -0,0 +1 @@
+require('./c.mjs')

test/fixtures/es-modules/esm-cjs-esm-esm-cycle/c.mjs

@@ -0,0 +1 @@
+import './a.mjs'

test/fixtures/es-modules/esm-esm-cjs-esm-cycle/a.mjs

@@ -0,0 +1,15 @@
+// a.mjs
+
+try {
+  await import('./b.mjs');
+  console.log('dynamic import b.mjs did not fail');
+} catch (err) {
+  console.log('dynamic import b.mjs failed', err);
+}
+
+try {
+  await import('./d.mjs');
+  console.log('dynamic import d.mjs did not fail');
+} catch (err) {
+  console.log('dynamic import d.mjs failed', err);
+}

test/fixtures/es-modules/esm-esm-cjs-esm-cycle/b.mjs

@@ -0,0 +1,3 @@
+// b.mjs
+import "./c.mjs";
+console.log("Execute b");

test/fixtures/es-modules/esm-esm-cjs-esm-cycle/c.mjs

@@ -0,0 +1,5 @@
+// c.mjs
+import { createRequire } from "module";
+console.log("Start c");
+createRequire(import.meta.url)("./d.mjs");
+throw new Error("Error from c");

test/fixtures/es-modules/esm-esm-cjs-esm-cycle/d.mjs

@@ -0,0 +1,3 @@
+// d.mjs
+import "./c.mjs";
+console.log("Execute d");

test/fixtures/es-modules/esm-esm-cjs-esm-esm-cycle/a.mjs

@@ -0,0 +1 @@
+import './b.mjs'

test/fixtures/es-modules/esm-esm-cjs-esm-esm-cycle/b.mjs

@@ -0,0 +1 @@
+import './c.cjs'

test/fixtures/es-modules/esm-esm-cjs-esm-esm-cycle/c.cjs

@@ -0,0 +1 @@
+require('./z.mjs')

test/fixtures/es-modules/esm-esm-cjs-esm-esm-cycle/z.mjs

@@ -0,0 +1 @@
+import './a.mjs'