diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index ce2350f6cd16..0f901c460340 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -17,6 +17,7 @@ const { PromiseReject, PromiseResolve, SafePromiseAllReturnArrayLike, + SafeSet, Symbol, SymbolToStringTag, TypeError, @@ -332,7 +333,10 @@ class SourceTextModule extends Module { })); } - async [kLink](linker) { + async [kLink](linker, linkedModules = new SafeSet()) { + if (linkedModules.has(this)) { + return; + } this.#statusOverride = 'linking'; // Iterates the module requests and links with the linker. @@ -358,7 +362,7 @@ class SourceTextModule extends Module { throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${specifier}' resolved to an errored module`, module.error); } if (module.status === 'unlinked') { - await module[kLink](linker); + await module[kLink](linker, linkedModules); } return module[kWrap]; }); @@ -368,6 +372,7 @@ class SourceTextModule extends Module { try { const modules = await SafePromiseAllReturnArrayLike(modulePromises); this[kWrap].link(modules); + linkedModules.add(this); } catch (e) { this.#error = e; throw e; diff --git a/test/parallel/test-vm-module-link-linked-deps.js b/test/parallel/test-vm-module-link-linked-deps.js new file mode 100644 index 000000000000..179ee0f8c4a4 --- /dev/null +++ b/test/parallel/test-vm-module-link-linked-deps.js @@ -0,0 +1,77 @@ +// Flags: --experimental-vm-modules + +'use strict'; + +const common = require('../common'); +const vm = require('vm'); +const assert = require('assert'); + +// This test verifies that `module.link(linker)` does not link a dependency +// again when it has already finished linking before another module +// requests it. + +// Regression of https://github.com/nodejs/node/issues/66938 + +const sources = { + './index.js': ` + import foo from "./foo.js"; + import shared from "./shared.js"; + export default { foo, shared }; + `, + './foo.js': ` + import shared from "./shared.js"; + export default "foo"; + `, + './shared.js': ` + import leaf from "./leaf.js"; + export default leaf; + `, + './leaf.js': ` + export default "leaf"; + `, +}; + +const moduleCache = new Map(); + +function getModuleInstance(identifier) { + let module = moduleCache.get(identifier); + + if (!module) { + module = new vm.SourceTextModule(sources[identifier], { + identifier, + }); + moduleCache.set(identifier, module); + } + + return module; +} + +async function test() { + const module = getModuleInstance('./index.js'); + const requests = []; + + await module.link(async (specifier, referrer) => { + requests.push([specifier, referrer.identifier]); + // Delay `./foo.js` so that `./shared.js` finishes linking before + // `./foo.js` requests it. + if (specifier === './foo.js') { + await new Promise((resolve) => setImmediate(resolve)); + } + return getModuleInstance(specifier); + }); + + await module.evaluate(); + assert.strictEqual(module.namespace.default.foo, 'foo'); + assert.strictEqual(module.namespace.default.shared, 'leaf'); + + // Assert that `./shared.js` is not linked again for `./foo.js`. + assert.deepStrictEqual(requests, [ + // [specifier, referrer] + ['./foo.js', './index.js'], + ['./shared.js', './index.js'], + ['./leaf.js', './shared.js'], + ['./shared.js', './foo.js'], + ]); +} + +test().then(common.mustCall());