module: cache synchronous module jobs before linking

lib/internal/modules/esm/module_job.js

@@ -295,22 +295,33 @@ class ModuleJobSync extends ModuleJobBase {
   #loader = null;
   constructor(loader, url, importAttributes, moduleWrap, isMain, inspectBrk) {
     super(url, importAttributes, moduleWrap, isMain, inspectBrk, true);
-    assert(this.module instanceof ModuleWrap);
     this.#loader = loader;
-    const moduleRequests = this.module.getModuleRequests();
-    // Specifiers should be aligned with the moduleRequests array in order.
-    const specifiers = Array(moduleRequests.length);
-    const modules = Array(moduleRequests.length);
-    const jobs = Array(moduleRequests.length);
-    for (let i = 0; i < moduleRequests.length; ++i) {
-      const { specifier, attributes } = moduleRequests[i];
-      const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
-      specifiers[i] = specifier;
-      modules[i] = job.module;
-      jobs[i] = job;
+
+    assert(this.module instanceof ModuleWrap);
+    // Store itself into the cache first before linking in case there are circular
+    // references in the linking.
+    loader.loadCache.set(url, importAttributes.type, this);
+
+    try {
+      const moduleRequests = this.module.getModuleRequests();
+      // Specifiers should be aligned with the moduleRequests array in order.
+      const specifiers = Array(moduleRequests.length);
+      const modules = Array(moduleRequests.length);
+      const jobs = Array(moduleRequests.length);
+      for (let i = 0; i < moduleRequests.length; ++i) {
+        const { specifier, attributes } = moduleRequests[i];
+        const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
+        specifiers[i] = specifier;
+        modules[i] = job.module;
+        jobs[i] = job;
+      }
+      this.module.link(specifiers, modules);
+      this.linked = jobs;
+    } finally {
+      // Restore it - if it succeeds, we'll reset in the caller; Otherwise it's
+      // not cached and if the error is caught, subsequent attempt would still fail.
+      loader.loadCache.delete(url, importAttributes.type);
     }
-    this.module.link(specifiers, modules);
-    this.linked = jobs;
   }
 
   get modulePromise() {

lib/internal/modules/esm/module_map.js

@@ -114,6 +114,12 @@ class LoadCache extends SafeMap {
     validateString(type, 'type');
     return super.get(url)?.[type] !== undefined;
   }
+  delete(url, type = kImplicitAssertType) {
+    const cached = super.get(url);
+    if (cached) {
+      cached[type] = undefined;
+    }
+  }
 }
 
 module.exports = {

test/common/index.js

@@ -32,6 +32,7 @@ const net = require('net');
 const path = require('path');
 const { inspect } = require('util');
 const { isMainThread } = require('worker_threads');
+const { isModuleNamespaceObject } = require('util/types');
 
 const tmpdir = require('./tmpdir');
 const bits = ['arm64', 'loong64', 'mips', 'mipsel', 'ppc64', 'riscv64', 's390x', 'x64']
@@ -938,6 +939,18 @@ function getPrintedStackTrace(stderr) {
   return result;
 }
 
+/**
+ * Check the exports of require(esm).
+ * TODO(joyeecheung): use it in all the test-require-module-* tests to minimize changes
+ * if/when we change the layout of the result returned by require(esm).
+ * @param {object} mod result returned by require()
+ * @param {object} expectation shape of expected namespace.
+ */
+function expectRequiredModule(mod, expectation) {
+  assert(isModuleNamespaceObject(mod));
+  assert.deepStrictEqual({ ...mod }, { ...expectation });
+}
+
 const common = {
   allowGlobals,
   buildType,
@@ -946,6 +959,7 @@ const common = {
   createZeroFilledFile,
   defaultAutoSelectFamilyAttemptTimeout,
   expectsError,
+  expectRequiredModule,
   expectWarning,
   gcUntil,
   getArrayBufferViews,

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

@@ -0,0 +1,8 @@
+// Flags: --experimental-require-module
+'use strict';
+
+// This tests that ESM <-> ESM cycle is allowed in a require()-d graph.
+const common = require('../common');
+const cycle = require('../fixtures/es-modules/cjs-esm-esm-cycle/c.cjs');
+
+common.expectRequiredModule(cycle, { b: 5 });

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

@@ -0,0 +1 @@
+export { b } from './b.mjs';

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

@@ -0,0 +1,2 @@
+import './a.mjs'
+export const b = 5;

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

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