From 95bef5af8884a11dd4ba36b665d8b786ef7596b0 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 29 Aug 2025 13:17:18 +0100 Subject: [PATCH] vm: sync-ify SourceTextModule linkage Split `module.link(linker)` into two synchronous step `sourceTextModule.linkRequests()` and `sourceTextModule.instantiate()`. This allows creating vm modules and resolving the dependencies in a complete synchronous procedure. This also makes `syntheticModule.link()` redundant. The link step for a SyntheticModule is no-op and is already taken care in the constructor by initializing the binding slots with the given export names. PR-URL: https://github.com/nodejs/node/pull/59000 Refs: https://github.com/nodejs/node/issues/37648 Reviewed-By: Joyee Cheung --- doc/api/errors.md | 7 + doc/api/vm.md | 231 ++++++++++++------ lib/internal/bootstrap/realm.js | 1 + lib/internal/errors.js | 1 + lib/internal/vm/module.js | 42 +++- src/module_wrap.cc | 51 ++++ src/module_wrap.h | 3 + src/node_errors.h | 1 + test/parallel/test-internal-module-wrap.js | 59 +++-- test/parallel/test-vm-module-basic.js | 2 +- test/parallel/test-vm-module-instantiate.js | 99 ++++++++ ...t-vm-module-linkmodulerequests-circular.js | 72 ++++++ .../test-vm-module-linkmodulerequests-deep.js | 34 +++ .../test-vm-module-linkmodulerequests.js | 67 +++++ .../parallel/test-vm-module-modulerequests.js | 17 +- test/parallel/test-vm-module-synthetic.js | 10 +- 16 files changed, 599 insertions(+), 98 deletions(-) create mode 100644 test/parallel/test-vm-module-instantiate.js create mode 100644 test/parallel/test-vm-module-linkmodulerequests-circular.js create mode 100644 test/parallel/test-vm-module-linkmodulerequests-deep.js create mode 100644 test/parallel/test-vm-module-linkmodulerequests.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 475cba0e8f..a492c0f514 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2285,6 +2285,13 @@ The V8 platform used by this instance of Node.js does not support creating Workers. This is caused by lack of embedder support for Workers. In particular, this error will not occur with standard builds of Node.js. + + +### `ERR_MODULE_LINK_MISMATCH` + +A module can not be linked because the same module requests in it are not +resolved to the same module. + ### `ERR_MODULE_NOT_FOUND` diff --git a/doc/api/vm.md b/doc/api/vm.md index a478b3022b..95f9f1ff99 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -419,9 +419,7 @@ class that closely mirrors [Module Record][]s as defined in the ECMAScript specification. Unlike `vm.Script` however, every `vm.Module` object is bound to a context from -its creation. Operations on `vm.Module` objects are intrinsically asynchronous, -in contrast with the synchronous nature of `vm.Script` objects. The use of -'async' functions can help with manipulating `vm.Module` objects. +its creation. Using a `vm.Module` object requires three distinct steps: creation/parsing, linking, and evaluation. These three steps are illustrated in the following @@ -449,7 +447,7 @@ const contextifiedObject = vm.createContext({ // Here, we attempt to obtain the default export from the module "foo", and // put it into local binding "secret". -const bar = new vm.SourceTextModule(` +const rootModule = new vm.SourceTextModule(` import s from 'foo'; s; print(s); @@ -459,39 +457,48 @@ const bar = new vm.SourceTextModule(` // // "Link" the imported dependencies of this Module to it. // -// The provided linking callback (the "linker") accepts two arguments: the -// parent module (`bar` in this case) and the string that is the specifier of -// the imported module. The callback is expected to return a Module that -// corresponds to the provided specifier, with certain requirements documented -// in `module.link()`. -// -// If linking has not started for the returned Module, the same linker -// callback will be called on the returned Module. +// Obtain the requested dependencies of a SourceTextModule by +// `sourceTextModule.moduleRequests` and resolve them. // // Even top-level Modules without dependencies must be explicitly linked. The -// callback provided would never be called, however. +// array passed to `sourceTextModule.linkRequests(modules)` can be +// empty, however. // -// The link() method returns a Promise that will be resolved when all the -// Promises returned by the linker resolve. -// -// Note: This is a contrived example in that the linker function creates a new -// "foo" module every time it is called. In a full-fledged module system, a -// cache would probably be used to avoid duplicated modules. +// Note: This is a contrived example in that the resolveAndLinkDependencies +// creates a new "foo" module every time it is called. In a full-fledged +// module system, a cache would probably be used to avoid duplicated modules. -async function linker(specifier, referencingModule) { - if (specifier === 'foo') { - return new vm.SourceTextModule(` - // The "secret" variable refers to the global variable we added to - // "contextifiedObject" when creating the context. - export default secret; - `, { context: referencingModule.context }); +const moduleMap = new Map([ + ['root', rootModule], +]); - // Using `contextifiedObject` instead of `referencingModule.context` - // here would work as well. - } - throw new Error(`Unable to resolve dependency: ${specifier}`); +function resolveAndLinkDependencies(module) { + const requestedModules = module.moduleRequests.map((request) => { + // In a full-fledged module system, the resolveAndLinkDependencies would + // resolve the module with the module cache key `[specifier, attributes]`. + // In this example, we just use the specifier as the key. + const specifier = request.specifier; + + let requestedModule = moduleMap.get(specifier); + if (requestedModule === undefined) { + requestedModule = new vm.SourceTextModule(` + // The "secret" variable refers to the global variable we added to + // "contextifiedObject" when creating the context. + export default secret; + `, { context: referencingModule.context }); + moduleMap.set(specifier, linkedModule); + // Resolve the dependencies of the new module as well. + resolveAndLinkDependencies(requestedModule); + } + + return requestedModule; + }); + + module.linkRequests(requestedModules); } -await bar.link(linker); + +resolveAndLinkDependencies(rootModule); +rootModule.instantiate(); // Step 3 // @@ -499,7 +506,7 @@ await bar.link(linker); // resolve after the module has finished evaluating. // Prints 42. -await bar.evaluate(); +await rootModule.evaluate(); ``` ```cjs @@ -521,7 +528,7 @@ const contextifiedObject = vm.createContext({ // Here, we attempt to obtain the default export from the module "foo", and // put it into local binding "secret". - const bar = new vm.SourceTextModule(` + const rootModule = new vm.SourceTextModule(` import s from 'foo'; s; print(s); @@ -531,39 +538,48 @@ const contextifiedObject = vm.createContext({ // // "Link" the imported dependencies of this Module to it. // - // The provided linking callback (the "linker") accepts two arguments: the - // parent module (`bar` in this case) and the string that is the specifier of - // the imported module. The callback is expected to return a Module that - // corresponds to the provided specifier, with certain requirements documented - // in `module.link()`. - // - // If linking has not started for the returned Module, the same linker - // callback will be called on the returned Module. + // Obtain the requested dependencies of a SourceTextModule by + // `sourceTextModule.moduleRequests` and resolve them. // // Even top-level Modules without dependencies must be explicitly linked. The - // callback provided would never be called, however. + // array passed to `sourceTextModule.linkRequests(modules)` can be + // empty, however. // - // The link() method returns a Promise that will be resolved when all the - // Promises returned by the linker resolve. - // - // Note: This is a contrived example in that the linker function creates a new - // "foo" module every time it is called. In a full-fledged module system, a - // cache would probably be used to avoid duplicated modules. + // Note: This is a contrived example in that the resolveAndLinkDependencies + // creates a new "foo" module every time it is called. In a full-fledged + // module system, a cache would probably be used to avoid duplicated modules. - async function linker(specifier, referencingModule) { - if (specifier === 'foo') { - return new vm.SourceTextModule(` - // The "secret" variable refers to the global variable we added to - // "contextifiedObject" when creating the context. - export default secret; - `, { context: referencingModule.context }); + const moduleMap = new Map([ + ['root', rootModule], + ]); - // Using `contextifiedObject` instead of `referencingModule.context` - // here would work as well. - } - throw new Error(`Unable to resolve dependency: ${specifier}`); + function resolveAndLinkDependencies(module) { + const requestedModules = module.moduleRequests.map((request) => { + // In a full-fledged module system, the resolveAndLinkDependencies would + // resolve the module with the module cache key `[specifier, attributes]`. + // In this example, we just use the specifier as the key. + const specifier = request.specifier; + + let requestedModule = moduleMap.get(specifier); + if (requestedModule === undefined) { + requestedModule = new vm.SourceTextModule(` + // The "secret" variable refers to the global variable we added to + // "contextifiedObject" when creating the context. + export default secret; + `, { context: referencingModule.context }); + moduleMap.set(specifier, linkedModule); + // Resolve the dependencies of the new module as well. + resolveAndLinkDependencies(requestedModule); + } + + return requestedModule; + }); + + module.linkRequests(requestedModules); } - await bar.link(linker); + + resolveAndLinkDependencies(rootModule); + rootModule.instantiate(); // Step 3 // @@ -571,7 +587,7 @@ const contextifiedObject = vm.createContext({ // resolve after the module has finished evaluating. // Prints 42. - await bar.evaluate(); + await rootModule.evaluate(); })(); ``` @@ -660,6 +676,10 @@ changes: Link module dependencies. This method must be called before evaluation, and can only be called once per module. +Use [`sourceTextModule.linkRequests(modules)`][] and +[`sourceTextModule.instantiate()`][] to link modules either synchronously or +asynchronously. + The function is expected to return a `Module` object or a `Promise` that eventually resolves to a `Module` object. The returned `Module` must satisfy the following two invariants: @@ -805,8 +825,9 @@ const module = new vm.SourceTextModule( meta.prop = {}; }, }); -// Since module has no dependencies, the linker function will never be called. -await module.link(() => {}); +// The module has an empty `moduleRequests` array. +module.linkRequests([]); +module.instantiate(); await module.evaluate(); // Now, Object.prototype.secret will be equal to 42. @@ -832,8 +853,9 @@ const contextifiedObject = vm.createContext({ secret: 42 }); meta.prop = {}; }, }); - // Since module has no dependencies, the linker function will never be called. - await module.link(() => {}); + // The module has an empty `moduleRequests` array. + module.linkRequests([]); + module.instantiate(); await module.evaluate(); // Now, Object.prototype.secret will be equal to 42. // @@ -898,6 +920,69 @@ to disallow any changes to it. Corresponds to the `[[RequestedModules]]` field of [Cyclic Module Record][]s in the ECMAScript specification. +### `sourceTextModule.instantiate()` + + + +* Returns: {undefined} + +Instantiate the module with the linked requested modules. + +This resolves the imported bindings of the module, including re-exported +binding names. When there are any bindings that cannot be resolved, +an error would be thrown synchronously. + +If the requested modules include cyclic dependencies, the +[`sourceTextModule.linkRequests(modules)`][] method must be called on all +modules in the cycle before calling this method. + +### `sourceTextModule.linkRequests(modules)` + + + +* `modules` {vm.Module\[]} Array of `vm.Module` objects that this module depends on. + The order of the modules in the array is the order of + [`sourceTextModule.moduleRequests`][]. +* Returns: {undefined} + +Link module dependencies. This method must be called before evaluation, and +can only be called once per module. + +The order of the module instances in the `modules` array should correspond to the order of +[`sourceTextModule.moduleRequests`][] being resolved. If two module requests have the same +specifier and import attributes, they must be resolved with the same module instance or an +`ERR_MODULE_LINK_MISMATCH` would be thrown. For example, when linking requests for this +module: + + + +```mjs +import foo from 'foo'; +import source Foo from 'foo'; +``` + + + +The `modules` array must contain two references to the same instance, because the two +module requests are identical but in two phases. + +If the module has no dependencies, the `modules` array can be empty. + +Users can use `sourceTextModule.moduleRequests` to implement the host-defined +[HostLoadImportedModule][] abstract operation in the ECMAScript specification, +and using `sourceTextModule.linkRequests()` to invoke specification defined +[FinishLoadingImportedModule][], on the module with all dependencies in a batch. + +It's up to the creator of the `SourceTextModule` to determine if the resolution +of the dependencies is synchronous or asynchronous. + +After each module in the `modules` array is linked, call +[`sourceTextModule.instantiate()`][]. + ### `sourceTextModule.moduleRequests` * `name` {string} Name of the export to set. * `value` {any} The value to set the export to. -This method is used after the module is linked to set the values of exports. If -it is called before the module is linked, an [`ERR_VM_MODULE_STATUS`][] error -will be thrown. +This method sets the module export binding slots with the given value. ```mjs import vm from 'node:vm'; @@ -1033,7 +1121,6 @@ const m = new vm.SyntheticModule(['x'], () => { m.setExport('x', 1); }); -await m.link(() => {}); await m.evaluate(); assert.strictEqual(m.namespace.x, 1); @@ -1045,7 +1132,6 @@ const vm = require('node:vm'); const m = new vm.SyntheticModule(['x'], () => { m.setExport('x', 1); }); - await m.link(() => {}); await m.evaluate(); assert.strictEqual(m.namespace.x, 1); })(); @@ -2037,7 +2123,9 @@ const { Script, SyntheticModule } = require('node:vm'); [Cyclic Module Record]: https://tc39.es/ecma262/#sec-cyclic-module-records [ECMAScript Module Loader]: esm.md#modules-ecmascript-modules [Evaluate() concrete method]: https://tc39.es/ecma262/#sec-moduleevaluation +[FinishLoadingImportedModule]: https://tc39.es/ecma262/#sec-FinishLoadingImportedModule [GetModuleNamespace]: https://tc39.es/ecma262/#sec-getmodulenamespace +[HostLoadImportedModule]: https://tc39.es/ecma262/#sec-HostLoadImportedModule [HostResolveImportedModule]: https://tc39.es/ecma262/#sec-hostresolveimportedmodule [ImportDeclaration]: https://tc39.es/ecma262/#prod-ImportDeclaration [Link() concrete method]: https://tc39.es/ecma262/#sec-moduledeclarationlinking @@ -2049,13 +2137,14 @@ const { Script, SyntheticModule } = require('node:vm'); [WithClause]: https://tc39.es/ecma262/#prod-WithClause [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing -[`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status [`Error`]: errors.md#class-error [`URL`]: url.md#class-url [`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval [`optionsExpression`]: https://tc39.es/proposal-import-attributes/#sec-evaluate-import-call [`script.runInContext()`]: #scriptrunincontextcontextifiedobject-options [`script.runInThisContext()`]: #scriptruninthiscontextoptions +[`sourceTextModule.instantiate()`]: #sourcetextmoduleinstantiate +[`sourceTextModule.linkRequests(modules)`]: #sourcetextmodulelinkrequestsmodules [`sourceTextModule.moduleRequests`]: #sourcetextmodulemodulerequests [`url.origin`]: url.md#urlorigin [`vm.compileFunction()`]: #vmcompilefunctioncode-params-options diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js index f49f0814bb..9055588a31 100644 --- a/lib/internal/bootstrap/realm.js +++ b/lib/internal/bootstrap/realm.js @@ -359,6 +359,7 @@ class BuiltinModule { this.setExport('default', builtin.exports); }); // Ensure immediate sync execution to capture exports now + this.module.link([]); this.module.instantiate(); this.module.evaluate(-1, false); return this.module; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a4a670797a..2453d8b0e9 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1602,6 +1602,7 @@ E('ERR_MISSING_ARGS', return `${msg} must be specified`; }, TypeError); E('ERR_MISSING_OPTION', '%s is required', TypeError); +E('ERR_MODULE_LINK_MISMATCH', '%s', TypeError); E('ERR_MODULE_NOT_FOUND', function(path, base, exactUrl) { if (exactUrl) { lazyInternalUtil().setOwnProperty(this, 'url', `${exactUrl}`); diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index aba5c2872d..5cdd1850a3 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -38,6 +38,7 @@ const { ERR_VM_MODULE_DIFFERENT_CONTEXT, ERR_VM_MODULE_CANNOT_CREATE_CACHED_DATA, ERR_VM_MODULE_LINK_FAILURE, + ERR_MODULE_LINK_MISMATCH, ERR_VM_MODULE_NOT_MODULE, ERR_VM_MODULE_STATUS, } = require('internal/errors').codes; @@ -50,6 +51,7 @@ const { validateUint32, validateString, validateThisInternalField, + validateArray, } = require('internal/validators'); const binding = internalBinding('module_wrap'); @@ -369,6 +371,37 @@ class SourceTextModule extends Module { } } + linkRequests(modules) { + validateThisInternalField(this, kWrap, 'SourceTextModule'); + if (this.status !== 'unlinked') { + throw new ERR_VM_MODULE_STATUS('must be unlinked'); + } + validateArray(modules, 'modules'); + if (modules.length !== this.#moduleRequests.length) { + throw new ERR_MODULE_LINK_MISMATCH( + `Expected ${this.#moduleRequests.length} modules, got ${modules.length}`, + ); + } + const moduleWraps = ArrayPrototypeMap(modules, (module) => { + if (!isModule(module)) { + throw new ERR_VM_MODULE_NOT_MODULE(); + } + if (module.context !== this.context) { + throw new ERR_VM_MODULE_DIFFERENT_CONTEXT(); + } + return module[kWrap]; + }); + this[kWrap].link(moduleWraps); + } + + instantiate() { + validateThisInternalField(this, kWrap, 'SourceTextModule'); + if (this.status !== 'unlinked') { + throw new ERR_VM_MODULE_STATUS('must be unlinked'); + } + this[kWrap].instantiate(); + } + get dependencySpecifiers() { this.#dependencySpecifiers ??= ObjectFreeze( ArrayPrototypeMap(this.#moduleRequests, (request) => request.specifier)); @@ -435,10 +468,15 @@ class SyntheticModule extends Module { context, identifier, }); + // A synthetic module does not have dependencies. + this[kWrap].link([]); + this[kWrap].instantiate(); } - [kLink]() { - /** nothing to do for synthetic modules */ + link() { + validateThisInternalField(this, kWrap, 'SyntheticModule'); + // No-op for synthetic modules + // Do not invoke super.link() as it will throw an error. } setExport(name, value) { diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 72d910fa4a..ccd3ded24f 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -72,6 +72,25 @@ void ModuleCacheKey::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("import_attributes", import_attributes); } +std::string ModuleCacheKey::ToString() const { + std::string result = "ModuleCacheKey(\"" + specifier + "\""; + if (!import_attributes.empty()) { + result += ", {"; + bool first = true; + for (const auto& attr : import_attributes) { + if (first) { + first = false; + } else { + result += ", "; + } + result += attr.first + ": " + attr.second; + } + result += "}"; + } + result += ")"; + return result; +} + template ModuleCacheKey ModuleCacheKey::From(Local context, Local specifier, @@ -605,6 +624,8 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo& args) { // moduleWrap.link(moduleWraps) void ModuleWrap::Link(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); + Realm* realm = Realm::GetCurrent(args); + Local context = realm->context(); ModuleWrap* dependent; ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This()); @@ -616,6 +637,30 @@ void ModuleWrap::Link(const FunctionCallbackInfo& args) { Local modules = args[0].As(); CHECK_EQ(modules->Length(), static_cast(requests->Length())); + for (int i = 0; i < requests->Length(); i++) { + ModuleCacheKey module_cache_key = ModuleCacheKey::From( + context, requests->Get(context, i).As()); + DCHECK(dependent->resolve_cache_.contains(module_cache_key)); + + Local module_i; + Local module_cache_i; + uint32_t coalesced_index = dependent->resolve_cache_[module_cache_key]; + if (!modules->Get(context, i).ToLocal(&module_i) || + !modules->Get(context, coalesced_index).ToLocal(&module_cache_i) || + !module_i->StrictEquals(module_cache_i)) { + // If the module is different from the one of the same request, throw an + // error. + THROW_ERR_MODULE_LINK_MISMATCH( + realm->env(), + "Module request '%s' at index %d must be linked " + "to the same module requested at index %d", + module_cache_key.ToString(), + i, + coalesced_index); + return; + } + } + args.This()->SetInternalField(kLinkedRequestsSlot, modules); dependent->linked_ = true; } @@ -627,6 +672,12 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); Local context = obj->context(); Local module = obj->module_.Get(isolate); + + if (!obj->IsLinked()) { + THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked"); + return; + } + TryCatchScope try_catch(realm->env()); USE(module->InstantiateModule( context, ResolveModuleCallback, ResolveSourceCallback)); diff --git a/src/module_wrap.h b/src/module_wrap.h index 467a9af117..2d0747dcf0 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -59,6 +59,9 @@ struct ModuleCacheKey : public MemoryRetainer { SET_SELF_SIZE(ModuleCacheKey) void MemoryInfo(MemoryTracker* tracker) const override; + // Returns a string representation of the ModuleCacheKey. + std::string ToString() const; + template static ModuleCacheKey From(v8::Local context, v8::Local specifier, diff --git a/src/node_errors.h b/src/node_errors.h index 09fe51d6f7..8919307dd3 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -108,6 +108,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details); V(ERR_MISSING_PASSPHRASE, TypeError) \ V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \ V(ERR_MODULE_NOT_FOUND, Error) \ + V(ERR_MODULE_LINK_MISMATCH, TypeError) \ V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \ V(ERR_OPERATION_FAILED, TypeError) \ V(ERR_OPTIONS_BEFORE_BOOTSTRAPPING, Error) \ diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index bed940d136..8ba92413c6 100644 --- a/test/parallel/test-internal-module-wrap.js +++ b/test/parallel/test-internal-module-wrap.js @@ -1,4 +1,4 @@ -// Flags: --expose-internals +// Flags: --expose-internals --js-source-phase-imports 'use strict'; const common = require('../common'); const assert = require('assert'); @@ -6,25 +6,25 @@ const assert = require('assert'); const { internalBinding } = require('internal/test/binding'); const { ModuleWrap } = internalBinding('module_wrap'); -const unlinked = new ModuleWrap('unlinked', undefined, 'export * from "bar";', 0, 0); -assert.throws(() => { - unlinked.instantiate(); -}, { - code: 'ERR_VM_MODULE_LINK_FAILURE', -}); +async function testModuleWrap() { + const unlinked = new ModuleWrap('unlinked', undefined, 'export * from "bar";', 0, 0); + assert.throws(() => { + unlinked.instantiate(); + }, { + code: 'ERR_VM_MODULE_LINK_FAILURE', + }); -const dependsOnUnlinked = new ModuleWrap('dependsOnUnlinked', undefined, 'export * from "unlinked";', 0, 0); -dependsOnUnlinked.link([unlinked]); -assert.throws(() => { - dependsOnUnlinked.instantiate(); -}, { - code: 'ERR_VM_MODULE_LINK_FAILURE', -}); + const dependsOnUnlinked = new ModuleWrap('dependsOnUnlinked', undefined, 'export * from "unlinked";', 0, 0); + dependsOnUnlinked.link([unlinked]); + assert.throws(() => { + dependsOnUnlinked.instantiate(); + }, { + code: 'ERR_VM_MODULE_LINK_FAILURE', + }); -const foo = new ModuleWrap('foo', undefined, 'export * from "bar";', 0, 0); -const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0); + const foo = new ModuleWrap('foo', undefined, 'export * from "bar";', 0, 0); + const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0); -(async () => { const moduleRequests = foo.getModuleRequests(); assert.strictEqual(moduleRequests.length, 1); assert.strictEqual(moduleRequests[0].specifier, 'bar'); @@ -37,5 +37,30 @@ const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0); // Check that the module requests are the same after linking, instantiate, and evaluation. assert.deepStrictEqual(moduleRequests, foo.getModuleRequests()); +} +// Verify that linking two module with a same ModuleCacheKey throws an error. +function testLinkMismatch() { + const foo = new ModuleWrap('foo', undefined, ` + import source BarSource from 'bar'; + import bar from 'bar'; +`, 0, 0); + const bar1 = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0); + const bar2 = new ModuleWrap('bar', undefined, 'export const six = 6', 0, 0); + + const moduleRequests = foo.getModuleRequests(); + assert.strictEqual(moduleRequests.length, 2); + assert.strictEqual(moduleRequests[0].specifier, moduleRequests[1].specifier); + assert.throws(() => { + foo.link([bar1, bar2]); + }, { + code: 'ERR_MODULE_LINK_MISMATCH', + // Test that ModuleCacheKey::ToString() is used in the error message. + message: `Module request 'ModuleCacheKey("bar")' at index 0 must be linked to the same module requested at index 1` + }); +} + +(async () => { + await testModuleWrap(); + testLinkMismatch(); })().then(common.mustCall()); diff --git a/test/parallel/test-vm-module-basic.js b/test/parallel/test-vm-module-basic.js index a8f5b92b7a..f38bc39d70 100644 --- a/test/parallel/test-vm-module-basic.js +++ b/test/parallel/test-vm-module-basic.js @@ -99,7 +99,7 @@ const util = require('util'); assert.strictEqual( util.inspect(m), `SyntheticModule { - status: 'unlinked', + status: 'linked', identifier: 'vm:module(0)', context: { foo: 'bar' } }` diff --git a/test/parallel/test-vm-module-instantiate.js b/test/parallel/test-vm-module-instantiate.js new file mode 100644 index 0000000000..e96a78a8f6 --- /dev/null +++ b/test/parallel/test-vm-module-instantiate.js @@ -0,0 +1,99 @@ +'use strict'; + +// Flags: --experimental-vm-modules + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('simple module', () => { + const foo = new SourceTextModule(` + export const foo = 4 + export default 5; + `); + foo.linkRequests([]); + foo.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(foo.namespace), + ['default', 'foo', Symbol.toStringTag] + ); +}); + +test('linkRequests can not be skipped', () => { + const foo = new SourceTextModule(` + export const foo = 4 + export default 5; + `); + assert.throws(() => { + foo.instantiate(); + }, { + code: 'ERR_VM_MODULE_LINK_FAILURE', + }); +}); + +test('re-export simple name', () => { + const foo = new SourceTextModule(` + export { bar } from 'bar'; + `); + const bar = new SourceTextModule(` + export const bar = 42; + `); + foo.linkRequests([bar]); + foo.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(foo.namespace), + ['bar', Symbol.toStringTag] + ); +}); + +test('re-export-star', () => { + const foo = new SourceTextModule(` + export * from 'bar'; + `); + const bar = new SourceTextModule(` + export const bar = 42; + `); + foo.linkRequests([bar]); + foo.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(foo.namespace), + ['bar', Symbol.toStringTag] + ); +}); + +test('deep re-export-star', () => { + let stackTop = new SourceTextModule(` + export const foo = 4; + `); + stackTop.linkRequests([]); + for (let i = 0; i < 10; i++) { + const mod = new SourceTextModule(` + export * from 'stack?top'; + `); + mod.linkRequests([stackTop]); + stackTop = mod; + } + stackTop.instantiate(); + + assert.deepStrictEqual( + Reflect.ownKeys(stackTop.namespace), + ['foo', Symbol.toStringTag] + ); +}); + +test('should throw if the module is not linked', () => { + const foo = new SourceTextModule(` + import { bar } from 'bar'; + `); + assert.throws(() => { + foo.instantiate(); + }, { + code: 'ERR_VM_MODULE_LINK_FAILURE', + }); +}); diff --git a/test/parallel/test-vm-module-linkmodulerequests-circular.js b/test/parallel/test-vm-module-linkmodulerequests-circular.js new file mode 100644 index 0000000000..f7992ef3a1 --- /dev/null +++ b/test/parallel/test-vm-module-linkmodulerequests-circular.js @@ -0,0 +1,72 @@ +'use strict'; + +// Flags: --experimental-vm-modules --js-source-phase-imports + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('basic circular linking', async function circular() { + const foo = new SourceTextModule(` + import getFoo from 'bar'; + export let foo = 42; + export default getFoo(); + `); + const bar = new SourceTextModule(` + import { foo } from 'foo'; + export default function getFoo() { + return foo; + } + `); + foo.linkRequests([bar]); + bar.linkRequests([foo]); + + foo.instantiate(); + assert.strictEqual(foo.status, 'linked'); + assert.strictEqual(bar.status, 'linked'); + + await foo.evaluate(); + assert.strictEqual(foo.namespace.default, 42); +}); + +test('circular linking graph', async function circular2() { + const sourceMap = { + 'root': ` + import * as a from './a.mjs'; + import * as b from './b.mjs'; + if (!('fromA' in a)) + throw new Error(); + if (!('fromB' in a)) + throw new Error(); + if (!('fromA' in b)) + throw new Error(); + if (!('fromB' in b)) + throw new Error(); + `, + './a.mjs': ` + export * from './b.mjs'; + export let fromA; + `, + './b.mjs': ` + export * from './a.mjs'; + export let fromB; + ` + }; + const moduleMap = new Map(); + for (const [specifier, source] of Object.entries(sourceMap)) { + moduleMap.set(specifier, new SourceTextModule(source, { + identifier: new URL(specifier, 'file:///').href, + })); + } + for (const mod of moduleMap.values()) { + mod.linkRequests(mod.moduleRequests.map((request) => { + return moduleMap.get(request.specifier); + })); + } + const rootModule = moduleMap.get('root'); + rootModule.instantiate(); + await rootModule.evaluate(); +}); diff --git a/test/parallel/test-vm-module-linkmodulerequests-deep.js b/test/parallel/test-vm-module-linkmodulerequests-deep.js new file mode 100644 index 0000000000..c73b5afddf --- /dev/null +++ b/test/parallel/test-vm-module-linkmodulerequests-deep.js @@ -0,0 +1,34 @@ +'use strict'; + +// Flags: --experimental-vm-modules --js-source-phase-imports + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('deep linking', async function depth() { + const foo = new SourceTextModule('export default 5'); + foo.linkRequests([]); + foo.instantiate(); + + function getProxy(parentName, parentModule) { + const mod = new SourceTextModule(` + import ${parentName} from '${parentName}'; + export default ${parentName}; + `); + mod.linkRequests([parentModule]); + mod.instantiate(); + return mod; + } + + const bar = getProxy('foo', foo); + const baz = getProxy('bar', bar); + const barz = getProxy('baz', baz); + + await barz.evaluate(); + + assert.strictEqual(barz.namespace.default, 5); +}); diff --git a/test/parallel/test-vm-module-linkmodulerequests.js b/test/parallel/test-vm-module-linkmodulerequests.js new file mode 100644 index 0000000000..6d9a4324e5 --- /dev/null +++ b/test/parallel/test-vm-module-linkmodulerequests.js @@ -0,0 +1,67 @@ +'use strict'; + +// Flags: --experimental-vm-modules --js-source-phase-imports + +require('../common'); + +const assert = require('assert'); + +const { SourceTextModule } = require('vm'); +const test = require('node:test'); + +test('simple graph', async function simple() { + const foo = new SourceTextModule('export default 5;'); + foo.linkRequests([]); + + globalThis.fiveResult = undefined; + const bar = new SourceTextModule('import five from "foo"; fiveResult = five'); + + bar.linkRequests([foo]); + bar.instantiate(); + + await bar.evaluate(); + assert.strictEqual(globalThis.fiveResult, 5); + delete globalThis.fiveResult; +}); + +test('invalid link values', () => { + const invalidValues = [ + undefined, + null, + {}, + SourceTextModule.prototype, + ]; + + for (const value of invalidValues) { + const module = new SourceTextModule('import "foo"'); + assert.throws(() => module.linkRequests([value]), { + code: 'ERR_VM_MODULE_NOT_MODULE', + }); + } +}); + +test('mismatch linkage', () => { + const foo = new SourceTextModule('export default 5;'); + foo.linkRequests([]); + + // Link with more modules than requested. + const bar = new SourceTextModule('import foo from "foo";'); + assert.throws(() => bar.linkRequests([foo, foo]), { + code: 'ERR_MODULE_LINK_MISMATCH', + }); + + // Link with fewer modules than requested. + const baz = new SourceTextModule('import foo from "foo"; import bar from "bar";'); + assert.throws(() => baz.linkRequests([foo]), { + code: 'ERR_MODULE_LINK_MISMATCH', + }); + + // Link a same module cache key with different instances. + const qux = new SourceTextModule(` + import foo from "foo"; + import source Foo from "foo"; + `); + assert.throws(() => qux.linkRequests([foo, bar]), { + code: 'ERR_MODULE_LINK_MISMATCH', + }); +}); diff --git a/test/parallel/test-vm-module-modulerequests.js b/test/parallel/test-vm-module-modulerequests.js index 87fb34cae3..aaa892cbab 100644 --- a/test/parallel/test-vm-module-modulerequests.js +++ b/test/parallel/test-vm-module-modulerequests.js @@ -12,15 +12,21 @@ const test = require('node:test'); test('SourceTextModule.moduleRequests should return module requests', (t) => { const m = new SourceTextModule(` import { foo } from './foo.js'; + import * as FooDuplicate from './foo.js'; import { bar } from './bar.json' with { type: 'json' }; + import * as BarDuplicate from './bar.json' with { type: 'json' }; import { quz } from './quz.js' with { attr1: 'quz' }; + import * as QuzDuplicate from './quz.js' with { attr1: 'quz' }; import { quz as quz2 } from './quz.js' with { attr2: 'quark', attr3: 'baz' }; + import * as Quz2Duplicate from './quz.js' with { attr2: 'quark', attr3: 'baz' }; import source Module from './source-module'; + import source Module2 from './source-module'; + import * as SourceModule from './source-module'; export { foo, bar, quz, quz2 }; `); const requests = m.moduleRequests; - assert.strictEqual(requests.length, 5); + assert.strictEqual(requests.length, 6); assert.deepStrictEqual(requests[0], { __proto__: null, specifier: './foo.js', @@ -65,6 +71,14 @@ test('SourceTextModule.moduleRequests should return module requests', (t) => { }, phase: 'source', }); + assert.deepStrictEqual(requests[5], { + __proto__: null, + specifier: './source-module', + attributes: { + __proto__: null, + }, + phase: 'evaluation', + }); // Check the deprecated dependencySpecifiers property. // The dependencySpecifiers items are not unique. @@ -74,6 +88,7 @@ test('SourceTextModule.moduleRequests should return module requests', (t) => { './quz.js', './quz.js', './source-module', + './source-module', ]); }); diff --git a/test/parallel/test-vm-module-synthetic.js b/test/parallel/test-vm-module-synthetic.js index 831387ddbd..c32a99ede1 100644 --- a/test/parallel/test-vm-module-synthetic.js +++ b/test/parallel/test-vm-module-synthetic.js @@ -58,12 +58,10 @@ const assert = require('assert'); } { - const s = new SyntheticModule([], () => {}); - assert.throws(() => { - s.setExport('name', 'value'); - }, { - code: 'ERR_VM_MODULE_STATUS', - }); + const s = new SyntheticModule(['name'], () => {}); + // Exports of SyntheticModule can be immediately set after creation. + // No link is required. + s.setExport('name', 'value'); } for (const value of [null, {}, SyntheticModule.prototype]) {