diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index cdf92206f54190..02ec3183bc596b 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -17,7 +17,6 @@ const { const { kIsExecuting, kRequiredModuleSymbol, - Module: CJSModule, } = require('internal/modules/cjs/loader'); const { imported_cjs_symbol } = internalBinding('symbols'); @@ -560,19 +559,6 @@ class ModuleLoader { throw new ERR_UNKNOWN_MODULE_FORMAT(format, url); } - // Populate the CJS cache with a facade for ESM in case subsequent require(esm) is - // looking it up from the cache. The parent module of the CJS cache entry would be the - // first CJS module that loads it with require(). This is an approximation, because - // ESM caches more and it does not get re-loaded and updated every time an `import` is - // encountered, unlike CJS require(), and we only use the parent entry to provide - // more information in error messages. - if (format === 'module') { - const parentFilename = urlToFilename(parentURL); - const parent = parentFilename ? CJSModule._cache[parentFilename] : undefined; - const cjsModule = lazyLoadTranslators().cjsEmplaceModuleCacheEntryForURL(url, parent); - debug('cjsEmplaceModuleCacheEntryForURL', url, parent, cjsModule); - } - const result = FunctionPrototypeCall(translator, this, url, source, parentURL === undefined); assert(result instanceof ModuleWrap); return result; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 7c8e20caf0182c..147d96bda8098f 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -154,9 +154,22 @@ function loadCJSModule(module, source, url, filename, isMain) { // requires a separate cache to be populated as well as introducing several quirks. This is not ideal. const job = cascadedLoader.getModuleJobForRequireInImportedCJS(specifier, url, importAttributes); job.runSync(); - const mod = cjsCache.get(job.url); - assert(mod, `Imported CJS module ${url} failed to load module ${job.url} using require()`); - if (!job.module.synthetic && !mod.loaded) { + let mod = cjsCache.get(job.url); + assert(job.module, `Imported CJS module ${url} failed to load module ${job.url} using require() due to race condition`); + + if (job.module.synthetic) { + assert(mod, `Imported CJS module ${url} failed to load module ${job.url} using require() due to missed cache`); + return mod.exports; + } + + // The module being required is a source text module. + if (!mod) { + mod = cjsEmplaceModuleCacheEntry(job.url); + cjsCache.set(job.url, mod); + } + // The module has been cached by the re-invented require. Update the exports object + // from the namespace object and return the evaluated exports. + if (!mod.loaded) { debug('populateCJSExportsFromESM from require(esm) in imported CJS', url, mod, job.module); populateCJSExportsFromESM(mod, job.module, job.module.getNamespace()); mod.loaded = true; diff --git a/test/es-module/test-esm-in-require-cache-2.mjs b/test/es-module/test-esm-in-require-cache-2.mjs new file mode 100644 index 00000000000000..b751883029b934 --- /dev/null +++ b/test/es-module/test-esm-in-require-cache-2.mjs @@ -0,0 +1,26 @@ +// This tests the behavior of ESM in require.cache when it's loaded from import. + +import '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; +const filename = fixtures.path('es-modules', 'esm-in-require-cache', 'esm.mjs'); +import { Module } from 'node:module'; + +// Imported ESM should not be in the require cache. +let { name } = await import('../fixtures/es-modules/esm-in-require-cache/import-esm.mjs'); +assert.strictEqual(name, 'esm'); +assert(!Module._cache[filename]); + +({ name } = await import('../fixtures/es-modules/esm-in-require-cache/esm.mjs')); +assert.strictEqual(name, 'esm'); +assert(!Module._cache[filename]); + +// Requiring ESM indirectly should not put it in the cache. +({ name } = await import('../fixtures/es-modules/esm-in-require-cache/require-import-esm.cjs')); +assert.strictEqual(name, 'esm'); +assert(!Module._cache[filename]); + +// After being required directly, it should be in the cache. +({ name } = await import('../fixtures/es-modules/esm-in-require-cache/import-require-esm.mjs')); +assert.strictEqual(name, 'esm'); +assert(Module._cache[filename]); diff --git a/test/es-module/test-esm-in-require-cache.js b/test/es-module/test-esm-in-require-cache.js new file mode 100644 index 00000000000000..d3365ce14f80b6 --- /dev/null +++ b/test/es-module/test-esm-in-require-cache.js @@ -0,0 +1,25 @@ +// This tests the behavior of ESM in require.cache when it's loaded from require. +'use strict'; +require('../common'); +const assert = require('node:assert'); +const fixtures = require('../common/fixtures'); +const filename = fixtures.path('es-modules', 'esm-in-require-cache', 'esm.mjs'); + +// Requiring ESM indirectly should not put it in the cache. +let { name } = require('../fixtures/es-modules/esm-in-require-cache/import-esm.mjs'); +assert.strictEqual(name, 'esm'); +assert(!require.cache[filename]); + +({ name } = require('../fixtures/es-modules/esm-in-require-cache/require-import-esm.cjs')); +assert.strictEqual(name, 'esm'); +assert(!require.cache[filename]); + +// After being required directly, it should be in the cache. +({ name } = require('../fixtures/es-modules/esm-in-require-cache/esm.mjs')); +assert.strictEqual(name, 'esm'); +assert(require.cache[filename]); +delete require.cache[filename]; + +({ name } = require('../fixtures/es-modules/esm-in-require-cache/import-require-esm.mjs')); +assert.strictEqual(name, 'esm'); +assert(require.cache[filename]); diff --git a/test/fixtures/es-modules/esm-in-require-cache/esm.mjs b/test/fixtures/es-modules/esm-in-require-cache/esm.mjs new file mode 100644 index 00000000000000..787bbe86d3e771 --- /dev/null +++ b/test/fixtures/es-modules/esm-in-require-cache/esm.mjs @@ -0,0 +1 @@ +export const name = 'esm'; diff --git a/test/fixtures/es-modules/esm-in-require-cache/import-esm.mjs b/test/fixtures/es-modules/esm-in-require-cache/import-esm.mjs new file mode 100644 index 00000000000000..b70b27fe1620b9 --- /dev/null +++ b/test/fixtures/es-modules/esm-in-require-cache/import-esm.mjs @@ -0,0 +1 @@ +export { name } from './esm.mjs'; diff --git a/test/fixtures/es-modules/esm-in-require-cache/import-require-esm.mjs b/test/fixtures/es-modules/esm-in-require-cache/import-require-esm.mjs new file mode 100644 index 00000000000000..1d8e14adb2f5ed --- /dev/null +++ b/test/fixtures/es-modules/esm-in-require-cache/import-require-esm.mjs @@ -0,0 +1 @@ +export { name, cache } from './require-esm.cjs' diff --git a/test/fixtures/es-modules/esm-in-require-cache/require-esm.cjs b/test/fixtures/es-modules/esm-in-require-cache/require-esm.cjs new file mode 100644 index 00000000000000..52ead05a698255 --- /dev/null +++ b/test/fixtures/es-modules/esm-in-require-cache/require-esm.cjs @@ -0,0 +1,9 @@ +const path = require('path'); + +const name = require('./esm.mjs').name; +exports.name = name; + +const filename = path.join(__dirname, 'esm.mjs'); +const cache = require.cache[filename]; +exports.cache = require.cache[filename]; + diff --git a/test/fixtures/es-modules/esm-in-require-cache/require-import-esm.cjs b/test/fixtures/es-modules/esm-in-require-cache/require-import-esm.cjs new file mode 100644 index 00000000000000..ddf16383f7a4df --- /dev/null +++ b/test/fixtures/es-modules/esm-in-require-cache/require-import-esm.cjs @@ -0,0 +1 @@ +exports.name = require('./import-esm.mjs').name;