diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index d7c6927049c42d..2f42909e0c6f82 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/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() { diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index ab1171eaa47b02..247bde93cabd70 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/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 = { diff --git a/test/common/index.js b/test/common/index.js index d2c68c0fafb01b..84805723e4be6a 100644 --- a/test/common/index.js +++ b/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, diff --git a/test/es-module/test-require-module-cycle-cjs-esm-esm.js b/test/es-module/test-require-module-cycle-cjs-esm-esm.js new file mode 100644 index 00000000000000..a83d7ee7a71bb2 --- /dev/null +++ b/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 }); diff --git a/test/fixtures/es-modules/cjs-esm-esm-cycle/a.mjs b/test/fixtures/es-modules/cjs-esm-esm-cycle/a.mjs new file mode 100644 index 00000000000000..798e86506da2a9 --- /dev/null +++ b/test/fixtures/es-modules/cjs-esm-esm-cycle/a.mjs @@ -0,0 +1 @@ +export { b } from './b.mjs'; diff --git a/test/fixtures/es-modules/cjs-esm-esm-cycle/b.mjs b/test/fixtures/es-modules/cjs-esm-esm-cycle/b.mjs new file mode 100644 index 00000000000000..9e909cd6856455 --- /dev/null +++ b/test/fixtures/es-modules/cjs-esm-esm-cycle/b.mjs @@ -0,0 +1,2 @@ +import './a.mjs' +export const b = 5; diff --git a/test/fixtures/es-modules/cjs-esm-esm-cycle/c.cjs b/test/fixtures/es-modules/cjs-esm-esm-cycle/c.cjs new file mode 100644 index 00000000000000..f9361ecd59d11e --- /dev/null +++ b/test/fixtures/es-modules/cjs-esm-esm-cycle/c.cjs @@ -0,0 +1 @@ +module.exports = require('./a.mjs');