diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index df7a26c9337c39..c6ff8932ff3b6f 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -308,7 +308,7 @@ class ModuleLoader { * @param {string} specifier Specifier of the the imported module. * @param {string} parentURL Where the import comes from. * @param {object} importAttributes import attributes from the import statement. - * @returns {ModuleWrap} + * @returns {ModuleJobBase} */ getModuleWrapForRequire(specifier, parentURL, importAttributes) { assert(getOptionValue('--experimental-require-module')); @@ -347,7 +347,7 @@ class ModuleLoader { // completed (e.g. the require call is lazy) so it's okay. We will return the // module now and check asynchronicity of the entire graph later, after the // graph is instantiated. - return job.module; + return job; } defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync; @@ -391,7 +391,7 @@ class ModuleLoader { job = new ModuleJobSync(this, url, importAttributes, wrap, isMain, inspectBrk); this.loadCache.set(url, importAttributes.type, job); - return job.module; + return job; } /** diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 43d9e141e04725..296dafe3ad20d6 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -19,6 +19,9 @@ const { StringPrototypeStartsWith, globalThis, } = primordials; +let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { + debug = fn; +}); const { ModuleWrap, kEvaluated } = internalBinding('module_wrap'); const { @@ -53,8 +56,7 @@ const isCommonJSGlobalLikeNotDefinedError = (errorMessage) => ); class ModuleJobBase { - constructor(loader, url, importAttributes, moduleWrapMaybePromise, isMain, inspectBrk) { - this.loader = loader; + constructor(url, importAttributes, moduleWrapMaybePromise, isMain, inspectBrk) { this.importAttributes = importAttributes; this.isMain = isMain; this.inspectBrk = inspectBrk; @@ -67,11 +69,13 @@ class ModuleJobBase { /* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of * its dependencies, over time. */ class ModuleJob extends ModuleJobBase { + #loader = null; // `loader` is the Loader instance used for loading dependencies. constructor(loader, url, importAttributes = { __proto__: null }, moduleProvider, isMain, inspectBrk, sync = false) { const modulePromise = ReflectApply(moduleProvider, loader, [url, isMain]); - super(loader, url, importAttributes, modulePromise, isMain, inspectBrk); + super(url, importAttributes, modulePromise, isMain, inspectBrk); + this.#loader = loader; // Expose the promise to the ModuleWrap directly for linking below. // `this.module` is also filled in below. this.modulePromise = modulePromise; @@ -94,7 +98,8 @@ class ModuleJob extends ModuleJobBase { // these `link` callbacks depending on each other. const dependencyJobs = []; const promises = this.module.link(async (specifier, attributes) => { - const job = await this.loader.getModuleJob(specifier, url, attributes); + const job = await this.#loader.getModuleJob(specifier, url, attributes); + debug(`async link() ${this.url} -> ${specifier}`, job); ArrayPrototypePush(dependencyJobs, job); return job.modulePromise; }); @@ -126,6 +131,8 @@ class ModuleJob extends ModuleJobBase { async _instantiate() { const jobsInGraph = new SafeSet(); const addJobsToDependencyGraph = async (moduleJob) => { + debug(`async addJobsToDependencyGraph() ${this.url}`, moduleJob); + if (jobsInGraph.has(moduleJob)) { return; } @@ -161,7 +168,7 @@ class ModuleJob extends ModuleJobBase { const { 1: childSpecifier, 2: name } = RegExpPrototypeExec( /module '(.*)' does not provide an export named '(.+)'/, e.message); - const { url: childFileURL } = await this.loader.resolve( + const { url: childFileURL } = await this.#loader.resolve( childSpecifier, parentFileUrl, kEmptyObject, @@ -172,7 +179,7 @@ class ModuleJob extends ModuleJobBase { // in the import attributes and some formats require them; but we only // care about CommonJS for the purposes of this error message. ({ format } = - await this.loader.load(childFileURL)); + await this.#loader.load(childFileURL)); } catch { // Continue regardless of error. } @@ -265,18 +272,27 @@ class ModuleJob extends ModuleJobBase { // All the steps are ensured to be synchronous and it throws on instantiating // an asynchronous graph. class ModuleJobSync extends ModuleJobBase { + #loader = null; constructor(loader, url, importAttributes, moduleWrap, isMain, inspectBrk) { - super(loader, url, importAttributes, moduleWrap, isMain, inspectBrk, true); + super(url, importAttributes, moduleWrap, isMain, inspectBrk, true); assert(this.module instanceof ModuleWrap); + this.#loader = loader; const moduleRequests = this.module.getModuleRequestsSync(); + const linked = []; for (let i = 0; i < moduleRequests.length; ++i) { const { 0: specifier, 1: attributes } = moduleRequests[i]; - const wrap = this.loader.getModuleWrapForRequire(specifier, url, attributes); + const job = this.#loader.getModuleWrapForRequire(specifier, url, attributes); const isLast = (i === moduleRequests.length - 1); // TODO(joyeecheung): make the resolution callback deal with both promisified // an raw module wraps, then we don't need to wrap it with a promise here. - this.module.cacheResolvedWrapsSync(specifier, PromiseResolve(wrap), isLast); + this.module.cacheResolvedWrapsSync(specifier, PromiseResolve(job.module), isLast); + ArrayPrototypePush(linked, job); } + this.linked = linked; + } + + get modulePromise() { + return PromiseResolve(this.module); } async run() { diff --git a/test/es-module/test-require-module-dynamic-import-3.js b/test/es-module/test-require-module-dynamic-import-3.js new file mode 100644 index 00000000000000..7a5fbf1a137f96 --- /dev/null +++ b/test/es-module/test-require-module-dynamic-import-3.js @@ -0,0 +1,14 @@ +// Flags: --experimental-require-module +'use strict'; + +// This tests that previously synchronously loaded submodule can still +// be loaded by dynamic import(). + +const common = require('../common'); +const assert = require('assert'); + +(async () => { + const required = require('../fixtures/es-modules/require-and-import/load.cjs'); + const imported = await import('../fixtures/es-modules/require-and-import/load.mjs'); + assert.deepStrictEqual({ ...required }, { ...imported }); +})().then(common.mustCall()); diff --git a/test/es-module/test-require-module-dynamic-import-4.js b/test/es-module/test-require-module-dynamic-import-4.js new file mode 100644 index 00000000000000..414cd70d82d33a --- /dev/null +++ b/test/es-module/test-require-module-dynamic-import-4.js @@ -0,0 +1,14 @@ +// Flags: --experimental-require-module +'use strict'; + +// This tests that previously asynchronously loaded submodule can still +// be loaded by require(). + +const common = require('../common'); +const assert = require('assert'); + +(async () => { + const imported = await import('../fixtures/es-modules/require-and-import/load.mjs'); + const required = require('../fixtures/es-modules/require-and-import/load.cjs'); + assert.deepStrictEqual({ ...required }, { ...imported }); +})().then(common.mustCall()); diff --git a/test/fixtures/es-modules/require-and-import/load.cjs b/test/fixtures/es-modules/require-and-import/load.cjs new file mode 100644 index 00000000000000..ec9c535a04352d --- /dev/null +++ b/test/fixtures/es-modules/require-and-import/load.cjs @@ -0,0 +1,2 @@ +module.exports = require('dep'); + diff --git a/test/fixtures/es-modules/require-and-import/load.mjs b/test/fixtures/es-modules/require-and-import/load.mjs new file mode 100644 index 00000000000000..f5d2135d4dca44 --- /dev/null +++ b/test/fixtures/es-modules/require-and-import/load.mjs @@ -0,0 +1,2 @@ +export * from 'dep'; + diff --git a/test/fixtures/es-modules/require-and-import/node_modules/dep/mod.js b/test/fixtures/es-modules/require-and-import/node_modules/dep/mod.js new file mode 100644 index 00000000000000..0554fa3d5c3bfb --- /dev/null +++ b/test/fixtures/es-modules/require-and-import/node_modules/dep/mod.js @@ -0,0 +1,2 @@ +export const hello = 'world'; + diff --git a/test/fixtures/es-modules/require-and-import/node_modules/dep/package.json b/test/fixtures/es-modules/require-and-import/node_modules/dep/package.json new file mode 100644 index 00000000000000..7aeabf8c45119d --- /dev/null +++ b/test/fixtures/es-modules/require-and-import/node_modules/dep/package.json @@ -0,0 +1,5 @@ +{ + "type": "module", + "main": "mod.js" +} +