module: allow cycles in require() in the CJS handling in ESM loader

lib/internal/modules/esm/module_job.js

@@ -26,6 +26,7 @@ const {
   ModuleWrap,
   kErrored,
   kEvaluated,
+  kEvaluating,
   kEvaluationPhase,
   kInstantiated,
   kUninstantiated,
@@ -338,8 +339,14 @@ class ModuleJob extends ModuleJobBase {
         return { __proto__: null, module: this.module, namespace };
       }
       throw this.module.getError();
-
-    } else if (status === kEvaluated) {
+    } else if (status === kEvaluating || status === kEvaluated) {
+      // kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
+      // Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
+      // detected earlier during the linking phase, though the CJS handling in the ESM
+      // loader won't be able to emit warnings on pending circular exports like what
+      // the CJS loader does.
+      // TODO(joyeecheung): remove the re-invented require() in the ESM loader and
+      // always handle CJS using the CJS loader to eliminate the quirks.
       return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
     }
     assert.fail(`Unexpected module status ${status}.`);

src/module_wrap.cc

@@ -783,11 +783,10 @@ void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
       return realm->env()->ThrowError(
           "Cannot get namespace, module has not been instantiated");
     case Module::Status::kInstantiated:
+    case Module::Status::kEvaluating:
     case Module::Status::kEvaluated:
     case Module::Status::kErrored:
       break;
-    case Module::Status::kEvaluating:
-      UNREACHABLE();
   }
 
   if (module->IsGraphAsync()) {

test/es-module/test-import-preload-require-cycle.js

@@ -0,0 +1,20 @@
+'use strict';
+
+// This tests that --import preload does not break CJS entry points that contains
+// require cycles.
+
+require('../common');
+const fixtures = require('../common/fixtures');
+const { spawnSyncAndAssert } = require('../common/child_process');
+
+spawnSyncAndAssert(
+  process.execPath,
+  [
+    '--import',
+    fixtures.fileURL('import-require-cycle/preload.mjs'),
+    fixtures.path('import-require-cycle/c.js'),
+  ],
+  {
+    stdout: /cycle equality true/,
+  }
+);

test/fixtures/import-require-cycle/a.js

@@ -0,0 +1 @@
+module.exports.b = require('./b.js');

test/fixtures/import-require-cycle/b.js

@@ -0,0 +1 @@
+module.exports.a = require('./a.js');

test/fixtures/import-require-cycle/c.js

@@ -0,0 +1,3 @@
+const obj = require('./b.js');
+
+console.log('cycle equality', obj.a.b === obj);

test/fixtures/import-require-cycle/preload.mjs

@@ -0,0 +1,7 @@
+import * as mod from "module";
+
+mod.registerHooks({
+  load(url, context, nextLoad) {
+    return nextLoad(url, context);
+  },
+});