Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6f3b16d

Browse filesBrowse files
joyeecheungaduh95
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
Copy full SHA for 6f3b16d

File tree

Expand file treeCollapse file tree

3 files changed

+104
-98
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+104
-98
lines changed
Open diff view settings
Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+91-81Lines 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(
Collapse file

‎src/module_wrap.h‎

Copy file name to clipboardExpand all lines: src/module_wrap.h
+12-16Lines 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
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-internal-module-wrap.js
+1-1Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.