diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 51aa863dd61..cac4f603229 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/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}.`); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index dd0f74c5580..2b81a4d5da6 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -783,11 +783,10 @@ void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo& 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()) { diff --git a/test/es-module/test-import-preload-require-cycle.js b/test/es-module/test-import-preload-require-cycle.js new file mode 100644 index 00000000000..c47b64ba584 --- /dev/null +++ b/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/, + } +); diff --git a/test/fixtures/import-require-cycle/a.js b/test/fixtures/import-require-cycle/a.js new file mode 100644 index 00000000000..595a5085cf5 --- /dev/null +++ b/test/fixtures/import-require-cycle/a.js @@ -0,0 +1 @@ +module.exports.b = require('./b.js'); diff --git a/test/fixtures/import-require-cycle/b.js b/test/fixtures/import-require-cycle/b.js new file mode 100644 index 00000000000..869be257319 --- /dev/null +++ b/test/fixtures/import-require-cycle/b.js @@ -0,0 +1 @@ +module.exports.a = require('./a.js'); diff --git a/test/fixtures/import-require-cycle/c.js b/test/fixtures/import-require-cycle/c.js new file mode 100644 index 00000000000..39099ad7607 --- /dev/null +++ b/test/fixtures/import-require-cycle/c.js @@ -0,0 +1,3 @@ +const obj = require('./b.js'); + +console.log('cycle equality', obj.a.b === obj); diff --git a/test/fixtures/import-require-cycle/preload.mjs b/test/fixtures/import-require-cycle/preload.mjs new file mode 100644 index 00000000000..81eed70009e --- /dev/null +++ b/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); + }, +});