Skip to content

Commit f13589f

Browse files
legendecasmarco-ippolito
legendecas
authored and
marco-ippolito
committedFeb 11, 2025
lib,src: iterate module requests of a module wrap in JS
Avoid repetitively calling into JS callback from C++ in `ModuleWrap::Link`. This removes the convoluted callback style of the internal `ModuleWrap` link step. PR-URL: #52058 Backport-PR-URL: #56927 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Refs: #52697
·
v20.19.0
1 parent b07ad39 commit f13589f

File tree

9 files changed

+219
-229
lines changed

9 files changed

+219
-229
lines changed
 

Diff for: ‎lib/internal/modules/esm/loader.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ class ModuleLoader {
318318
* @param {object} importAttributes import attributes from the import statement.
319319
* @returns {ModuleJobBase}
320320
*/
321-
getModuleWrapForRequire(specifier, parentURL, importAttributes) {
321+
getModuleJobForRequire(specifier, parentURL, importAttributes) {
322322
assert(getOptionValue('--experimental-require-module'));
323323

324324
if (canParse(specifier)) {

Diff for: ‎lib/internal/modules/esm/module_job.js

+57-35
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
'use strict';
22

33
const {
4+
Array,
45
ArrayPrototypeJoin,
5-
ArrayPrototypePush,
66
ArrayPrototypeSome,
77
FunctionPrototype,
88
ObjectSetPrototypeOf,
@@ -82,31 +82,8 @@ class ModuleJob extends ModuleJobBase {
8282
this.modulePromise = PromiseResolve(this.modulePromise);
8383
}
8484

85-
// Wait for the ModuleWrap instance being linked with all dependencies.
86-
const link = async () => {
87-
this.module = await this.modulePromise;
88-
assert(this.module instanceof ModuleWrap);
89-
90-
// Explicitly keeping track of dependency jobs is needed in order
91-
// to flatten out the dependency graph below in `_instantiate()`,
92-
// so that circular dependencies can't cause a deadlock by two of
93-
// these `link` callbacks depending on each other.
94-
const dependencyJobs = [];
95-
const promises = this.module.link(async (specifier, attributes) => {
96-
const job = await this.#loader.getModuleJob(specifier, url, attributes);
97-
debug(`async link() ${this.url} -> ${specifier}`, job);
98-
ArrayPrototypePush(dependencyJobs, job);
99-
return job.modulePromise;
100-
});
101-
102-
if (promises !== undefined) {
103-
await SafePromiseAllReturnVoid(promises);
104-
}
105-
106-
return SafePromiseAllReturnArrayLike(dependencyJobs);
107-
};
10885
// Promise for the list of all dependencyJobs.
109-
this.linked = link();
86+
this.linked = this._link();
11087
// This promise is awaited later anyway, so silence
11188
// 'unhandled rejection' warnings.
11289
PromisePrototypeThen(this.linked, undefined, noop);
@@ -116,6 +93,49 @@ class ModuleJob extends ModuleJobBase {
11693
this.instantiated = undefined;
11794
}
11895

96+
/**
97+
* Iterates the module requests and links with the loader.
98+
* @returns {Promise<ModuleJob[]>} Dependency module jobs.
99+
*/
100+
async _link() {
101+
this.module = await this.modulePromise;
102+
assert(this.module instanceof ModuleWrap);
103+
104+
const moduleRequests = this.module.getModuleRequests();
105+
// Explicitly keeping track of dependency jobs is needed in order
106+
// to flatten out the dependency graph below in `_instantiate()`,
107+
// so that circular dependencies can't cause a deadlock by two of
108+
// these `link` callbacks depending on each other.
109+
// Create an ArrayLike to avoid calling into userspace with `.then`
110+
// when returned from the async function.
111+
const dependencyJobs = Array(moduleRequests.length);
112+
ObjectSetPrototypeOf(dependencyJobs, null);
113+
114+
// Specifiers should be aligned with the moduleRequests array in order.
115+
const specifiers = Array(moduleRequests.length);
116+
const modulePromises = Array(moduleRequests.length);
117+
// Iterate with index to avoid calling into userspace with `Symbol.iterator`.
118+
for (let idx = 0; idx < moduleRequests.length; idx++) {
119+
const { specifier, attributes } = moduleRequests[idx];
120+
121+
const dependencyJobPromise = this.#loader.getModuleJob(
122+
specifier, this.url, attributes,
123+
);
124+
const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => {
125+
debug(`async link() ${this.url} -> ${specifier}`, job);
126+
dependencyJobs[idx] = job;
127+
return job.modulePromise;
128+
});
129+
modulePromises[idx] = modulePromise;
130+
specifiers[idx] = specifier;
131+
}
132+
133+
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
134+
this.module.link(specifiers, modules);
135+
136+
return dependencyJobs;
137+
}
138+
119139
instantiate() {
120140
if (this.instantiated === undefined) {
121141
this.instantiated = this._instantiate();
@@ -269,18 +289,20 @@ class ModuleJobSync extends ModuleJobBase {
269289
super(url, importAttributes, moduleWrap, isMain, inspectBrk, true);
270290
assert(this.module instanceof ModuleWrap);
271291
this.#loader = loader;
272-
const moduleRequests = this.module.getModuleRequestsSync();
273-
const linked = [];
292+
const moduleRequests = this.module.getModuleRequests();
293+
// Specifiers should be aligned with the moduleRequests array in order.
294+
const specifiers = Array(moduleRequests.length);
295+
const modules = Array(moduleRequests.length);
296+
const jobs = Array(moduleRequests.length);
274297
for (let i = 0; i < moduleRequests.length; ++i) {
275-
const { 0: specifier, 1: attributes } = moduleRequests[i];
276-
const job = this.#loader.getModuleWrapForRequire(specifier, url, attributes);
277-
const isLast = (i === moduleRequests.length - 1);
278-
// TODO(joyeecheung): make the resolution callback deal with both promisified
279-
// an raw module wraps, then we don't need to wrap it with a promise here.
280-
this.module.cacheResolvedWrapsSync(specifier, PromiseResolve(job.module), isLast);
281-
ArrayPrototypePush(linked, job);
298+
const { specifier, attributes } = moduleRequests[i];
299+
const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
300+
specifiers[i] = specifier;
301+
modules[i] = job.module;
302+
jobs[i] = job;
282303
}
283-
this.linked = linked;
304+
this.module.link(specifiers, modules);
305+
this.linked = jobs;
284306
}
285307

286308
get modulePromise() {

Diff for: ‎lib/internal/vm/module.js

+56-35
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@
22

33
const assert = require('internal/assert');
44
const {
5+
Array,
56
ArrayIsArray,
67
ArrayPrototypeForEach,
78
ArrayPrototypeIndexOf,
9+
ArrayPrototypeMap,
810
ArrayPrototypeSome,
911
ObjectDefineProperty,
1012
ObjectGetPrototypeOf,
1113
ObjectPrototypeHasOwnProperty,
1214
ObjectSetPrototypeOf,
15+
PromiseResolve,
16+
PromisePrototypeThen,
1317
ReflectApply,
14-
SafePromiseAllReturnVoid,
18+
SafePromiseAllReturnArrayLike,
1519
Symbol,
1620
SymbolToStringTag,
1721
TypeError,
@@ -293,44 +297,61 @@ class SourceTextModule extends Module {
293297
importModuleDynamically,
294298
});
295299

296-
this[kLink] = async (linker) => {
297-
this.#statusOverride = 'linking';
300+
this[kDependencySpecifiers] = undefined;
301+
}
298302

299-
const promises = this[kWrap].link(async (identifier, attributes) => {
300-
const module = await linker(identifier, this, { attributes, assert: attributes });
301-
if (!isModule(module)) {
302-
throw new ERR_VM_MODULE_NOT_MODULE();
303-
}
304-
if (module.context !== this.context) {
305-
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
306-
}
307-
if (module.status === 'errored') {
308-
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${identifier}' resolved to an errored module`, module.error);
309-
}
310-
if (module.status === 'unlinked') {
311-
await module[kLink](linker);
312-
}
313-
return module[kWrap];
303+
async [kLink](linker) {
304+
this.#statusOverride = 'linking';
305+
306+
const moduleRequests = this[kWrap].getModuleRequests();
307+
// Iterates the module requests and links with the linker.
308+
// Specifiers should be aligned with the moduleRequests array in order.
309+
const specifiers = Array(moduleRequests.length);
310+
const modulePromises = Array(moduleRequests.length);
311+
// Iterates with index to avoid calling into userspace with `Symbol.iterator`.
312+
for (let idx = 0; idx < moduleRequests.length; idx++) {
313+
const { specifier, attributes } = moduleRequests[idx];
314+
315+
const linkerResult = linker(specifier, this, {
316+
attributes,
317+
assert: attributes,
314318
});
319+
const modulePromise = PromisePrototypeThen(
320+
PromiseResolve(linkerResult), async (module) => {
321+
if (!isModule(module)) {
322+
throw new ERR_VM_MODULE_NOT_MODULE();
323+
}
324+
if (module.context !== this.context) {
325+
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
326+
}
327+
if (module.status === 'errored') {
328+
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${specifier}' resolved to an errored module`, module.error);
329+
}
330+
if (module.status === 'unlinked') {
331+
await module[kLink](linker);
332+
}
333+
return module[kWrap];
334+
});
335+
modulePromises[idx] = modulePromise;
336+
specifiers[idx] = specifier;
337+
}
315338

316-
try {
317-
if (promises !== undefined) {
318-
await SafePromiseAllReturnVoid(promises);
319-
}
320-
} catch (e) {
321-
this.#error = e;
322-
throw e;
323-
} finally {
324-
this.#statusOverride = undefined;
325-
}
326-
};
327-
328-
this[kDependencySpecifiers] = undefined;
339+
try {
340+
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
341+
this[kWrap].link(specifiers, modules);
342+
} catch (e) {
343+
this.#error = e;
344+
throw e;
345+
} finally {
346+
this.#statusOverride = undefined;
347+
}
329348
}
330349

331350
get dependencySpecifiers() {
332351
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
333-
this[kDependencySpecifiers] ??= this[kWrap].getStaticDependencySpecifiers();
352+
// TODO(legendecas): add a new getter to expose the import attributes as the value type
353+
// of [[RequestedModules]] is changed in https://tc39.es/proposal-import-attributes/#table-cyclic-module-fields.
354+
this[kDependencySpecifiers] ??= ArrayPrototypeMap(this[kWrap].getModuleRequests(), (request) => request.specifier);
334355
return this[kDependencySpecifiers];
335356
}
336357

@@ -392,10 +413,10 @@ class SyntheticModule extends Module {
392413
context,
393414
identifier,
394415
});
416+
}
395417

396-
this[kLink] = () => this[kWrap].link(() => {
397-
assert.fail('link callback should not be called');
398-
});
418+
[kLink]() {
419+
/** nothing to do for synthetic modules */
399420
}
400421

401422
setExport(name, value) {

Diff for: ‎src/env_properties.h

+3
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
V(args_string, "args") \
7272
V(asn1curve_string, "asn1Curve") \
7373
V(async_ids_stack_string, "async_ids_stack") \
74+
V(attributes_string, "attributes") \
7475
V(base_string, "base") \
7576
V(bits_string, "bits") \
7677
V(block_list_string, "blockList") \
@@ -303,6 +304,7 @@
303304
V(sni_context_string, "sni_context") \
304305
V(source_string, "source") \
305306
V(source_map_url_string, "sourceMapURL") \
307+
V(specifier_string, "specifier") \
306308
V(stack_string, "stack") \
307309
V(standard_name_string, "standardName") \
308310
V(start_time_string, "startTime") \
@@ -377,6 +379,7 @@
377379
V(intervalhistogram_constructor_template, v8::FunctionTemplate) \
378380
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
379381
V(message_port_constructor_template, v8::FunctionTemplate) \
382+
V(module_wrap_constructor_template, v8::FunctionTemplate) \
380383
V(microtask_queue_ctor_template, v8::FunctionTemplate) \
381384
V(pipe_constructor_template, v8::FunctionTemplate) \
382385
V(promise_wrap_template, v8::ObjectTemplate) \

Diff for: ‎src/module_wrap.cc

+72-142
Original file line numberDiff line numberDiff line change
@@ -343,136 +343,105 @@ static Local<Object> createImportAttributesContainer(
343343
Local<FixedArray> raw_attributes,
344344
const int elements_per_attribute) {
345345
CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0);
346-
Local<Object> attributes =
347-
Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
346+
size_t num_attributes = raw_attributes->Length() / elements_per_attribute;
347+
std::vector<Local<v8::Name>> names(num_attributes);
348+
std::vector<Local<v8::Value>> values(num_attributes);
349+
348350
for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) {
349-
attributes
350-
->Set(realm->context(),
351-
raw_attributes->Get(realm->context(), i).As<String>(),
352-
raw_attributes->Get(realm->context(), i + 1).As<Value>())
353-
.ToChecked();
351+
int idx = i / elements_per_attribute;
352+
names[idx] = raw_attributes->Get(realm->context(), i).As<v8::Name>();
353+
values[idx] = raw_attributes->Get(realm->context(), i + 1).As<Value>();
354354
}
355355

356-
return attributes;
356+
return Object::New(
357+
isolate, v8::Null(isolate), names.data(), values.data(), num_attributes);
357358
}
358359

359-
void ModuleWrap::GetModuleRequestsSync(
360-
const FunctionCallbackInfo<Value>& args) {
361-
Realm* realm = Realm::GetCurrent(args);
362-
Isolate* isolate = args.GetIsolate();
363-
364-
Local<Object> that = args.This();
365-
366-
ModuleWrap* obj;
367-
ASSIGN_OR_RETURN_UNWRAP(&obj, that);
368-
369-
CHECK(!obj->linked_);
360+
static Local<Array> createModuleRequestsContainer(
361+
Realm* realm, Isolate* isolate, Local<FixedArray> raw_requests) {
362+
std::vector<Local<Value>> requests(raw_requests->Length());
370363

371-
Local<Module> module = obj->module_.Get(isolate);
372-
Local<FixedArray> module_requests = module->GetModuleRequests();
373-
const int module_requests_length = module_requests->Length();
374-
375-
std::vector<Local<Value>> requests;
376-
requests.reserve(module_requests_length);
377-
// call the dependency resolve callbacks
378-
for (int i = 0; i < module_requests_length; i++) {
364+
for (int i = 0; i < raw_requests->Length(); i++) {
379365
Local<ModuleRequest> module_request =
380-
module_requests->Get(realm->context(), i).As<ModuleRequest>();
381-
Local<FixedArray> raw_attributes = module_request->GetImportAssertions();
382-
std::vector<Local<Value>> request = {
383-
module_request->GetSpecifier(),
384-
createImportAttributesContainer(realm, isolate, raw_attributes, 3),
385-
};
386-
requests.push_back(Array::New(isolate, request.data(), request.size()));
387-
}
366+
raw_requests->Get(realm->context(), i).As<ModuleRequest>();
388367

389-
args.GetReturnValue().Set(
390-
Array::New(isolate, requests.data(), requests.size()));
391-
}
392-
393-
void ModuleWrap::CacheResolvedWrapsSync(
394-
const FunctionCallbackInfo<Value>& args) {
395-
Isolate* isolate = args.GetIsolate();
368+
Local<String> specifier = module_request->GetSpecifier();
396369

397-
CHECK_EQ(args.Length(), 3);
398-
CHECK(args[0]->IsString());
399-
CHECK(args[1]->IsPromise());
400-
CHECK(args[2]->IsBoolean());
370+
// Contains the import assertions for this request in the form:
371+
// [key1, value1, source_offset1, key2, value2, source_offset2, ...].
372+
Local<FixedArray> raw_attributes = module_request->GetImportAssertions();
373+
Local<Object> attributes =
374+
createImportAttributesContainer(realm, isolate, raw_attributes, 3);
401375

402-
ModuleWrap* dependent;
403-
ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
376+
Local<v8::Name> names[] = {
377+
realm->isolate_data()->specifier_string(),
378+
realm->isolate_data()->attributes_string(),
379+
};
380+
Local<Value> values[] = {
381+
specifier,
382+
attributes,
383+
};
384+
DCHECK_EQ(arraysize(names), arraysize(values));
404385

405-
Utf8Value specifier(isolate, args[0]);
406-
dependent->resolve_cache_[specifier.ToString()].Reset(isolate,
407-
args[1].As<Promise>());
386+
Local<Object> request = Object::New(
387+
isolate, v8::Null(isolate), names, values, arraysize(names));
408388

409-
if (args[2].As<v8::Boolean>()->Value()) {
410-
dependent->linked_ = true;
389+
requests[i] = request;
411390
}
391+
392+
return Array::New(isolate, requests.data(), requests.size());
412393
}
413394

414-
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
395+
void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
415396
Realm* realm = Realm::GetCurrent(args);
416397
Isolate* isolate = args.GetIsolate();
417-
418-
CHECK_EQ(args.Length(), 1);
419-
CHECK(args[0]->IsFunction());
420-
421398
Local<Object> that = args.This();
422399

423400
ModuleWrap* obj;
424401
ASSIGN_OR_RETURN_UNWRAP(&obj, that);
425402

426-
if (obj->linked_)
427-
return;
428-
obj->linked_ = true;
403+
Local<Module> module = obj->module_.Get(isolate);
404+
args.GetReturnValue().Set(createModuleRequestsContainer(
405+
realm, isolate, module->GetModuleRequests()));
406+
}
429407

430-
Local<Function> resolver_arg = args[0].As<Function>();
408+
// moduleWrap.link(specifiers, moduleWraps)
409+
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
410+
Realm* realm = Realm::GetCurrent(args);
411+
Isolate* isolate = args.GetIsolate();
412+
Local<Context> context = realm->context();
431413

432-
Local<Context> mod_context = obj->context();
433-
Local<Module> module = obj->module_.Get(isolate);
414+
ModuleWrap* dependent;
415+
ASSIGN_OR_RETURN_UNWRAP(&dependent, args.This());
434416

435-
Local<FixedArray> module_requests = module->GetModuleRequests();
436-
const int module_requests_length = module_requests->Length();
437-
MaybeStackBuffer<Local<Value>, 16> promises(module_requests_length);
417+
CHECK_EQ(args.Length(), 2);
438418

439-
// call the dependency resolve callbacks
440-
for (int i = 0; i < module_requests_length; i++) {
441-
Local<ModuleRequest> module_request =
442-
module_requests->Get(realm->context(), i).As<ModuleRequest>();
443-
Local<String> specifier = module_request->GetSpecifier();
444-
Utf8Value specifier_utf8(realm->isolate(), specifier);
445-
std::string specifier_std(*specifier_utf8, specifier_utf8.length());
419+
Local<Array> specifiers = args[0].As<Array>();
420+
Local<Array> modules = args[1].As<Array>();
421+
CHECK_EQ(specifiers->Length(), modules->Length());
446422

447-
Local<FixedArray> raw_attributes = module_request->GetImportAssertions();
448-
Local<Object> attributes =
449-
createImportAttributesContainer(realm, isolate, raw_attributes, 3);
423+
std::vector<v8::Global<Value>> specifiers_buffer;
424+
if (FromV8Array(context, specifiers, &specifiers_buffer).IsNothing()) {
425+
return;
426+
}
427+
std::vector<v8::Global<Value>> modules_buffer;
428+
if (FromV8Array(context, modules, &modules_buffer).IsNothing()) {
429+
return;
430+
}
450431

451-
Local<Value> argv[] = {
452-
specifier,
453-
attributes,
454-
};
432+
for (uint32_t i = 0; i < specifiers->Length(); i++) {
433+
Local<String> specifier_str =
434+
specifiers_buffer[i].Get(isolate).As<String>();
435+
Local<Object> module_object = modules_buffer[i].Get(isolate).As<Object>();
455436

456-
MaybeLocal<Value> maybe_resolve_return_value =
457-
resolver_arg->Call(mod_context, that, arraysize(argv), argv);
458-
if (maybe_resolve_return_value.IsEmpty()) {
459-
return;
460-
}
461-
Local<Value> resolve_return_value =
462-
maybe_resolve_return_value.ToLocalChecked();
463-
if (!resolve_return_value->IsPromise()) {
464-
THROW_ERR_VM_MODULE_LINK_FAILURE(
465-
realm, "request for '%s' did not return promise", specifier_std);
466-
return;
467-
}
468-
Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
469-
obj->resolve_cache_[specifier_std].Reset(isolate, resolve_promise);
437+
CHECK(
438+
realm->isolate_data()->module_wrap_constructor_template()->HasInstance(
439+
module_object));
470440

471-
promises[i] = resolve_promise;
441+
Utf8Value specifier(isolate, specifier_str);
442+
dependent->resolve_cache_[specifier.ToString()].Reset(isolate,
443+
module_object);
472444
}
473-
474-
args.GetReturnValue().Set(
475-
Array::New(isolate, promises.out(), promises.length()));
476445
}
477446

478447
void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
@@ -733,29 +702,6 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
733702
args.GetReturnValue().Set(module->GetStatus());
734703
}
735704

736-
void ModuleWrap::GetStaticDependencySpecifiers(
737-
const FunctionCallbackInfo<Value>& args) {
738-
Realm* realm = Realm::GetCurrent(args);
739-
ModuleWrap* obj;
740-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
741-
742-
Local<Module> module = obj->module_.Get(realm->isolate());
743-
744-
Local<FixedArray> module_requests = module->GetModuleRequests();
745-
int count = module_requests->Length();
746-
747-
MaybeStackBuffer<Local<Value>, 16> specifiers(count);
748-
749-
for (int i = 0; i < count; i++) {
750-
Local<ModuleRequest> module_request =
751-
module_requests->Get(realm->context(), i).As<ModuleRequest>();
752-
specifiers[i] = module_request->GetSpecifier();
753-
}
754-
755-
args.GetReturnValue().Set(
756-
Array::New(realm->isolate(), specifiers.out(), count));
757-
}
758-
759705
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
760706
Isolate* isolate = args.GetIsolate();
761707
ModuleWrap* obj;
@@ -793,16 +739,8 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
793739
return MaybeLocal<Module>();
794740
}
795741

796-
Local<Promise> resolve_promise =
742+
Local<Object> module_object =
797743
dependent->resolve_cache_[specifier_std].Get(isolate);
798-
799-
if (resolve_promise->State() != Promise::kFulfilled) {
800-
THROW_ERR_VM_MODULE_LINK_FAILURE(
801-
env, "request for '%s' is not yet fulfilled", specifier_std);
802-
return MaybeLocal<Module>();
803-
}
804-
805-
Local<Object> module_object = resolve_promise->Result().As<Object>();
806744
if (module_object.IsEmpty() || !module_object->IsObject()) {
807745
THROW_ERR_VM_MODULE_LINK_FAILURE(
808746
env, "request for '%s' did not return an object", specifier_std);
@@ -1029,9 +967,7 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
1029967
ModuleWrap::kInternalFieldCount);
1030968

1031969
SetProtoMethod(isolate, tpl, "link", Link);
1032-
SetProtoMethod(isolate, tpl, "getModuleRequestsSync", GetModuleRequestsSync);
1033-
SetProtoMethod(
1034-
isolate, tpl, "cacheResolvedWrapsSync", CacheResolvedWrapsSync);
970+
SetProtoMethod(isolate, tpl, "getModuleRequests", GetModuleRequests);
1035971
SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync);
1036972
SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
1037973
SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync);
@@ -1043,12 +979,8 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
1043979
SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
1044980
SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
1045981
SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
1046-
SetProtoMethodNoSideEffect(isolate,
1047-
tpl,
1048-
"getStaticDependencySpecifiers",
1049-
GetStaticDependencySpecifiers);
1050-
1051982
SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
983+
isolate_data->set_module_wrap_constructor_template(tpl);
1052984

1053985
SetMethod(isolate,
1054986
target,
@@ -1086,8 +1018,7 @@ void ModuleWrap::RegisterExternalReferences(
10861018
registry->Register(New);
10871019

10881020
registry->Register(Link);
1089-
registry->Register(GetModuleRequestsSync);
1090-
registry->Register(CacheResolvedWrapsSync);
1021+
registry->Register(GetModuleRequests);
10911022
registry->Register(InstantiateSync);
10921023
registry->Register(EvaluateSync);
10931024
registry->Register(GetNamespaceSync);
@@ -1098,7 +1029,6 @@ void ModuleWrap::RegisterExternalReferences(
10981029
registry->Register(GetNamespace);
10991030
registry->Register(GetStatus);
11001031
registry->Register(GetError);
1101-
registry->Register(GetStaticDependencySpecifiers);
11021032

11031033
registry->Register(SetImportModuleDynamicallyCallback);
11041034
registry->Register(SetInitializeImportMetaObjectCallback);

Diff for: ‎src/module_wrap.h

+2-7
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,7 @@ class ModuleWrap : public BaseObject {
9797
~ModuleWrap() override;
9898

9999
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
100-
static void GetModuleRequestsSync(
101-
const v8::FunctionCallbackInfo<v8::Value>& args);
102-
static void CacheResolvedWrapsSync(
100+
static void GetModuleRequests(
103101
const v8::FunctionCallbackInfo<v8::Value>& args);
104102
static void InstantiateSync(const v8::FunctionCallbackInfo<v8::Value>& args);
105103
static void EvaluateSync(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -111,8 +109,6 @@ class ModuleWrap : public BaseObject {
111109
static void GetNamespace(const v8::FunctionCallbackInfo<v8::Value>& args);
112110
static void GetStatus(const v8::FunctionCallbackInfo<v8::Value>& args);
113111
static void GetError(const v8::FunctionCallbackInfo<v8::Value>& args);
114-
static void GetStaticDependencySpecifiers(
115-
const v8::FunctionCallbackInfo<v8::Value>& args);
116112

117113
static void SetImportModuleDynamicallyCallback(
118114
const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -132,10 +128,9 @@ class ModuleWrap : public BaseObject {
132128
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
133129

134130
v8::Global<v8::Module> module_;
135-
std::unordered_map<std::string, v8::Global<v8::Promise>> resolve_cache_;
131+
std::unordered_map<std::string, v8::Global<v8::Object>> resolve_cache_;
136132
contextify::ContextifyContext* contextify_context_ = nullptr;
137133
bool synthetic_ = false;
138-
bool linked_ = false;
139134
int module_hash_;
140135
};
141136

Diff for: ‎src/util-inl.h

+16
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,22 @@ inline char* UncheckedCalloc(size_t n) { return UncheckedCalloc<char>(n); }
403403
// headers than we really need to.
404404
void ThrowErrStringTooLong(v8::Isolate* isolate);
405405

406+
v8::Maybe<void> FromV8Array(v8::Local<v8::Context> context,
407+
v8::Local<v8::Array> js_array,
408+
std::vector<v8::Global<v8::Value>>* out) {
409+
uint32_t count = js_array->Length();
410+
out->reserve(count);
411+
v8::Isolate* isolate = context->GetIsolate();
412+
for (size_t i = 0; i < count; ++i) {
413+
v8::Local<v8::Value> element;
414+
if (!js_array->Get(context, i).ToLocal(&element)) {
415+
return v8::Nothing<void>();
416+
}
417+
out->push_back(v8::Global<v8::Value>(isolate, element));
418+
}
419+
return v8::JustVoid();
420+
}
421+
406422
v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
407423
std::string_view str,
408424
v8::Isolate* isolate) {

Diff for: ‎src/util.h

+5
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,11 @@ struct FunctionDeleter {
702702
template <typename T, void (*function)(T*)>
703703
using DeleteFnPtr = typename FunctionDeleter<T, function>::Pointer;
704704

705+
// Mocking the FromV8Array from newer version of Node.js which relies
706+
// on V8 API that does not exist on v20.x to smooth out backports.
707+
inline v8::Maybe<void> FromV8Array(v8::Local<v8::Context> context,
708+
v8::Local<v8::Array> js_array,
709+
std::vector<v8::Global<v8::Value>>* out);
705710
std::vector<std::string_view> SplitString(const std::string_view in,
706711
const std::string_view delim);
707712

Diff for: ‎test/parallel/test-internal-module-wrap.js

+7-9
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,21 @@ const assert = require('assert');
55

66
const { internalBinding } = require('internal/test/binding');
77
const { ModuleWrap } = internalBinding('module_wrap');
8-
const { getPromiseDetails, isPromise } = internalBinding('util');
9-
const setTimeoutAsync = require('util').promisify(setTimeout);
108

119
const foo = new ModuleWrap('foo', undefined, 'export * from "bar";', 0, 0);
1210
const bar = new ModuleWrap('bar', undefined, 'export const five = 5', 0, 0);
1311

1412
(async () => {
15-
const promises = foo.link(() => setTimeoutAsync(1000).then(() => bar));
16-
assert.strictEqual(promises.length, 1);
17-
assert(isPromise(promises[0]));
18-
19-
await Promise.all(promises);
20-
21-
assert.strictEqual(getPromiseDetails(promises[0])[1], bar);
13+
const moduleRequests = foo.getModuleRequests();
14+
assert.strictEqual(moduleRequests.length, 1);
15+
assert.strictEqual(moduleRequests[0].specifier, 'bar');
2216

17+
foo.link(['bar'], [bar]);
2318
foo.instantiate();
2419

2520
assert.strictEqual(await foo.evaluate(-1, false), undefined);
2621
assert.strictEqual(foo.getNamespace().five, 5);
22+
23+
// Check that the module requests are the same after linking, instantiate, and evaluation.
24+
assert.deepStrictEqual(moduleRequests, foo.getModuleRequests());
2725
})().then(common.mustCall());

0 commit comments

Comments
 (0)
Please sign in to comment.