FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

esm: use index-based resolution callbacks · nodejs/node@6f3b16d · GitHub

/ node Public

Commit 6f3b16d

Browse files
joyeecheung
authored and
aduh95
committed
esm: use index-based resolution callbacks
This makes use of a new module resolution V8 API that passes in an index to the module request array to identify the module request, which simplifies the module linking process. PR-URL: #59396 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/6804466 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 1bbcdf9 commit 6f3b16d

3 files changed

Lines changed: 104 additions & 98 deletions

File tree

‎src/module_wrap.cc‎

Lines changed: 91 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,6 @@ ModuleWrap::ModuleWrap(Realm* realm,
165165
}
166166
MakeWeak();
167167
module_.SetWeak();
168-
169-
HandleScope scope(realm->isolate());
170-
Local<Context> context = realm->context();
171-
Local<FixedArray> requests = module->GetModuleRequests();
172-
for (int i = 0; i < requests->Length(); i++) {
173-
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
174-
context, requests->Get(context, i).As<ModuleRequest>());
175-
resolve_cache_[module_cache_key] = i;
176-
}
177168
}
178169

179170
ModuleWrap::~ModuleWrap() {
@@ -194,30 +185,6 @@ Local<Context> ModuleWrap::context() const {
194185
return obj.As<Object>()->GetCreationContextChecked();
195186
}
196187

197-
ModuleWrap* ModuleWrap::GetLinkedRequest(uint32_t index) {
198-
DCHECK(IsLinked());
199-
Isolate* isolate = env()->isolate();
200-
EscapableHandleScope scope(isolate);
201-
Local<Data> linked_requests_data =
202-
object()->GetInternalField(kLinkedRequestsSlot);
203-
DCHECK(linked_requests_data->IsValue() &&
204-
linked_requests_data.As<Value>()->IsArray());
205-
Local<Array> requests = linked_requests_data.As<Array>();
206-
207-
CHECK_LT(index, requests->Length());
208-
209-
Local<Value> module_value;
210-
if (!requests->Get(context(), index).ToLocal(&module_value)) {
211-
return nullptr;
212-
}
213-
CHECK(module_value->IsObject());
214-
Local<Object> module_object = module_value.As<Object>();
215-
216-
ModuleWrap* module_wrap;
217-
ASSIGN_OR_RETURN_UNWRAP(&module_wrap, module_object, nullptr);
218-
return module_wrap;
219-
}
220-
221188
ModuleWrap* ModuleWrap::GetFromModule(Environment* env,
222189
Local<Module> module) {
223190
auto range = env->hash_to_module_map.equal_range(module->GetIdentityHash());
@@ -653,6 +620,7 @@ void ModuleWrap::GetModuleRequests(const FunctionCallbackInfo<Value>& args) {
653620
// moduleWrap.link(moduleWraps)
654621
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
655622
Isolate* isolate = args.GetIsolate();
623+
HandleScope handle_scope(isolate);
656624
Realm* realm = Realm::GetCurrent(args);
657625
Local<Context> context = realm->context();
658626

@@ -664,33 +632,70 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
664632
Local<FixedArray> requests =
665633
dependent->module_.Get(isolate)->GetModuleRequests();
666634
Local<Array> modules = args[0].As<Array>();
667-
CHECK_EQ(modules->Length(), static_cast<uint32_t>(requests->Length()));
668-
669-
for (int i = 0; i < requests->Length(); i++) {
635+
std::vector<Global<Value>> modules_vector;
636+
if (FromV8Array(context, modules, &modules_vector).IsEmpty()) {
637+
return;
638+
}
639+
size_t request_count = static_cast<size_t>(requests->Length());
640+
CHECK_EQ(modules_vector.size(), request_count);
641+
std::vector<ModuleWrap*> linked_module_wraps(request_count);
642+
643+
// Track the duplicated module requests. For example if a modulelooks like
644+
// this:
645+
//
646+
// import { foo } from 'mod' with { type: 'json' };
647+
// import source ModSource from 'mod' with { type: 'json' };
648+
// import { baz } from 'mod2';
649+
//
650+
// The first two module requests are identical. The map would look like
651+
// { mod_key: 0, mod2_key: 2 } in this case, so that module request 0 and
652+
// module request 1 would be mapped to mod_key and both should resolve to the
653+
// module identified by module request 0 (the first one with this identity),
654+
// and module request 2 should resolve the module identified by index 2.
655+
std::unordered_map<ModuleCacheKey, size_t, ModuleCacheKey::Hash>
656+
module_request_map;
657+
658+
for (size_t i = 0; i < request_count; i++) {
659+
// TODO(joyeecheung): merge this with the serializeKey() in module_map.js.
660+
// This currently doesn't sort the import attributes.
661+
Local<Value> module_value = modules_vector[i].Get(isolate);
670662
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
671663
context, requests->Get(context, i).As<ModuleRequest>());
672-
DCHECK(dependent->resolve_cache_.contains(module_cache_key));
673-
674-
Local<Value> module_i;
675-
Local<Value> module_cache_i;
676-
uint32_t coalesced_index = dependent->resolve_cache_[module_cache_key];
677-
if (!modules->Get(context, i).ToLocal(&module_i) ||
678-
!modules->Get(context, coalesced_index).ToLocal(&module_cache_i) ||
679-
!module_i->StrictEquals(module_cache_i)) {
680-
// If the module is different from the one of the same request, throw an
681-
// error.
682-
THROW_ERR_MODULE_LINK_MISMATCH(
683-
realm->env(),
684-
"Module request '%s' at index %d must be linked "
685-
"to the same module requested at index %d",
686-
module_cache_key.ToString(),
687-
i,
688-
coalesced_index);
689-
return;
664+
auto it = module_request_map.find(module_cache_key);
665+
if (it == module_request_map.end()) {
666+
// This is the first request with this identity, record it - any mismatch
667+
// for this would only be found in subsequent requests, so no need to
668+
// check here.
669+
module_request_map[module_cache_key] = i;
670+
} else { // This identity has been seen before, check for mismatch.
671+
size_t first_seen_index = it->second;
672+
// Check that the module is the same as the one resolved by the first
673+
// request with this identity.
674+
Local<Value> first_seen_value =
675+
modules_vector[first_seen_index].Get(isolate);
676+
if (!module_value->StrictEquals(first_seen_value)) {
677+
// If the module is different from the one of the same request, throw an
678+
// error.
679+
THROW_ERR_MODULE_LINK_MISMATCH(
680+
realm->env(),
681+
"Module request '%s' at index %d must be linked "
682+
"to the same module requested at index %d",
683+
module_cache_key.ToString(),
684+
i,
685+
first_seen_index);
686+
return;
687+
}
690688
}
689+
690+
CHECK(module_value->IsObject()); // Guaranteed by link methods in JS land.
691+
ModuleWrap* resolved =
692+
BaseObject::Unwrap<ModuleWrap>(module_value.As<Object>());
693+
CHECK_NOT_NULL(resolved); // Guaranteed by link methods in JS land.
694+
linked_module_wraps[i] = resolved;
691695
}
692696

693697
args.This()->SetInternalField(kLinkedRequestsSlot, modules);
698+
std::swap(dependent->linked_module_wraps_, linked_module_wraps);
694699
dependent->linked_ = true;
695700
}
696701

@@ -1012,11 +1017,10 @@ void ModuleWrap::HasAsyncGraph(Local<Name> property,
10121017
// static
10131018
MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
10141019
Local<Context> context,
1015-
Local<String> specifier,
1016-
Local<FixedArray> import_attributes,
1020+
size_t module_request_index,
10171021
Local<Module> referrer) {
10181022
ModuleWrap* resolved_module;
1019-
if (!ResolveModule(context, specifier, import_attributes, referrer)
1023+
if (!ResolveModule(context, module_request_index, referrer)
10201024
.To(&resolved_module)) {
10211025
return {};
10221026
}
@@ -1027,11 +1031,10 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
10271031
// static
10281032
MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
10291033
Local<Context> context,
1030-
Local<String> specifier,
1031-
Local<FixedArray> import_attributes,
1034+
size_t module_request_index,
10321035
Local<Module> referrer) {
10331036
ModuleWrap* resolved_module;
1034-
if (!ResolveModule(context, specifier, import_attributes, referrer)
1037+
if (!ResolveModule(context, module_request_index, referrer)
10351038
.To(&resolved_module)) {
10361039
return {};
10371040
}
@@ -1050,12 +1053,22 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
10501053
return module_source_object.As<Object>();
10511054
}
10521055

1056+
static std::string GetSpecifierFromModuleRequest(Local<Context> context,
1057+
Local<Module> referrer,
1058+
size_t module_request_index) {
1059+
Local<ModuleRequest> raw_request =
1060+
referrer->GetModuleRequests()
1061+
->Get(context, static_cast<int>(module_request_index))
1062+
.As<ModuleRequest>();
1063+
Local<String> specifier = raw_request->GetSpecifier();
1064+
Utf8Value specifier_utf8(Isolate::GetCurrent(), specifier);
1065+
return specifier_utf8.ToString();
1066+
}
1067+
10531068
// static
1054-
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
1055-
Local<Context> context,
1056-
Local<String> specifier,
1057-
Local<FixedArray> import_attributes,
1058-
Local<Module> referrer) {
1069+
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(Local<Context> context,
1070+
size_t module_request_index,
1071+
Local<Module> referrer) {
10591072
Isolate* isolate = Isolate::GetCurrent();
10601073
Environment* env = Environment::GetCurrent(context);
10611074
if (env == nullptr) {
@@ -1065,37 +1078,34 @@ Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
10651078
// Check that the referrer is not yet been instantiated.
10661079
DCHECK(referrer->GetStatus() <= Module::kInstantiated);
10671080

1068-
ModuleCacheKey cache_key =
1069-
ModuleCacheKey::From(context, specifier, import_attributes);
1070-
10711081
ModuleWrap* dependent = ModuleWrap::GetFromModule(env, referrer);
10721082
if (dependent == nullptr) {
1083+
std::string specifier =
1084+
GetSpecifierFromModuleRequest(context, referrer, module_request_index);
10731085
THROW_ERR_VM_MODULE_LINK_FAILURE(
1074-
env, "request for '%s' is from invalid module", cache_key.specifier);
1086+
env, "request for '%s' is from invalid module", specifier);
10751087
return Nothing<ModuleWrap*>();
10761088
}
10771089
if (!dependent->IsLinked()) {
1090+
std::string specifier =
1091+
GetSpecifierFromModuleRequest(context, referrer, module_request_index);
10781092
THROW_ERR_VM_MODULE_LINK_FAILURE(env,
10791093
"request for '%s' can not be resolved on "
10801094
"module '%s' that is not linked",
1081-
cache_key.specifier,
1095+
specifier,
10821096
dependent->url_);
10831097
return Nothing<ModuleWrap*>();
10841098
}
10851099

1086-
auto it = dependent->resolve_cache_.find(cache_key);
1087-
if (it == dependent->resolve_cache_.end()) {
1088-
THROW_ERR_VM_MODULE_LINK_FAILURE(
1089-
env,
1090-
"request for '%s' is not cached on module '%s'",
1091-
cache_key.specifier,
1092-
dependent->url_);
1093-
return Nothing<ModuleWrap*>();
1100+
size_t linked_module_count = dependent->linked_module_wraps_.size();
1101+
if (linked_module_count > 0) {
1102+
CHECK_LT(module_request_index, linked_module_count);
1103+
} else {
1104+
UNREACHABLE("Module resolution callback invoked for a module"
1105+
" without linked requests");
10941106
}
10951107

1096-
ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second);
1097-
CHECK_NOT_NULL(module_wrap);
1098-
return Just(module_wrap);
1108+
return Just(dependent->linked_module_wraps_[module_request_index]);
10991109
}
11001110

11011111
static MaybeLocal<Promise> ImportModuleDynamicallyWithPhase(

‎src/module_wrap.h‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,14 @@ struct ModuleCacheKey : public MemoryRetainer {
9393
};
9494

9595
class ModuleWrap : public BaseObject {
96-
using ResolveCache =
97-
std::unordered_map<ModuleCacheKey, uint32_t, ModuleCacheKey::Hash>;
98-
9996
public:
10097
enum InternalFields {
10198
kModuleSlot = BaseObject::kInternalFieldCount,
10299
kModuleSourceObjectSlot,
103100
kSyntheticEvaluationStepsSlot,
104101
kContextObjectSlot, // Object whose creation context is the target Context
105-
kLinkedRequestsSlot, // Array of linked requests
102+
kLinkedRequestsSlot, // Array of linked requests, each is a ModuleWrap JS
103+
// wrapper object.
106104
kInternalFieldCount
107105
};
108106

@@ -134,8 +132,6 @@ class ModuleWrap : public BaseObject {
134132

135133
bool IsLinked() const { return linked_; }
136134

137-
ModuleWrap* GetLinkedRequest(uint32_t index);
138-
139135
static v8::Local<v8::PrimitiveArray> GetHostDefinedOptions(
140136
v8::Isolate* isolate, v8::Local<v8::Symbol> symbol);
141137

@@ -196,34 +192,34 @@ class ModuleWrap : public BaseObject {
196192

197193
static v8::MaybeLocal<v8::Module> ResolveModuleCallback(
198194
v8::Local<v8::Context> context,
199-
v8::Local<v8::String> specifier,
200-
v8::Local<v8::FixedArray> import_attributes,
195+
size_t module_request_index,
201196
v8::Local<v8::Module> referrer);
202197
static v8::MaybeLocal<v8::Object> ResolveSourceCallback(
203198
v8::Local<v8::Context> context,
204-
v8::Local<v8::String> specifier,
205-
v8::Local<v8::FixedArray> import_attributes,
199+
size_t module_request_index,
206200
v8::Local<v8::Module> referrer);
207201
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
208202

209203
// This method may throw a JavaScript exception, so the return type is
210204
// wrapped in a Maybe.
211-
static v8::Maybe<ModuleWrap*> ResolveModule(
212-
v8::Local<v8::Context> context,
213-
v8::Local<v8::String> specifier,
214-
v8::Local<v8::FixedArray> import_attributes,
215-
v8::Local<v8::Module> referrer);
205+
static v8::Maybe<ModuleWrap*> ResolveModule(v8::Local<v8::Context> context,
206+
size_t module_request_index,
207+
v8::Local<v8::Module> referrer);
216208

217209
std::string url_;
218210
v8::Global<v8::Module> module_;
219-
ResolveCache resolve_cache_;
220211
contextify::ContextifyContext* contextify_context_ = nullptr;
221212
bool synthetic_ = false;
222213
bool linked_ = false;
223214
// This depends on the module to be instantiated so it begins with a
224215
// nullopt value.
225216
std::optional<bool> has_async_graph_ = std::nullopt;
226217
int module_hash_;
218+
// Corresponds to the ModuleWrap* of the wrappers in kLinkedRequestsSlot.
219+
// These are populated during Link(), and are only valid after that as
220+
// convenient shortcuts, but do not hold the ModuleWraps alive. The actual
221+
// strong references come from the array in kLinkedRequestsSlot.
222+
std::vector<ModuleWrap*> linked_module_wraps_;
227223
};
228224

229225
} // namespace loader

‎test/parallel/test-internal-module-wrap.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ function testLinkMismatch() {
9191
}, {
9292
code: 'ERR_MODULE_LINK_MISMATCH',
9393
// Test that ModuleCacheKey::ToString() is used in the error message.
94-
message: `Module request 'ModuleCacheKey("bar")' at index 0 must be linked to the same module requested at index 1`
94+
message: `Module request 'ModuleCacheKey("bar")' at index 1 must be linked to the same module requested at index 0`
9595
});
9696
}
9797

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL