Skip to content

Commit

Permalink
src: set ModuleWrap internal fields only once
Browse files Browse the repository at this point in the history
There is no need to initialize the internal fields to undefined
and then initialize them to something else in the caller. Simply
pass the internal fields into the constructor to initialize
them just once.

PR-URL: #49391
Backport-PR-URL: #51004
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
  • Loading branch information
joyeecheung authored and richardlau committed Mar 15, 2024
1 parent 425e011 commit 2c2892b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 18 deletions.
40 changes: 23 additions & 17 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,22 @@ using v8::Value;
ModuleWrap::ModuleWrap(Environment* env,
Local<Object> object,
Local<Module> module,
Local<String> url)
: BaseObject(env, object),
module_(env->isolate(), module),
id_(env->get_next_module_id()) {
Local<String> url,
Local<Object> context_object,
Local<Value> synthetic_evaluation_step)
: BaseObject(env, object),
module_(env->isolate(), module),
id_(env->get_next_module_id()) {
env->id_to_module_map.emplace(id_, this);

Local<Value> undefined = Undefined(env->isolate());
object->SetInternalField(kURLSlot, url);
object->SetInternalField(kSyntheticEvaluationStepsSlot, undefined);
object->SetInternalField(kContextObjectSlot, undefined);
object->SetInternalField(kSyntheticEvaluationStepsSlot,
synthetic_evaluation_step);
object->SetInternalField(kContextObjectSlot, context_object);

if (!synthetic_evaluation_step->IsUndefined()) {
synthetic_ = true;
}
}

ModuleWrap::~ModuleWrap() {
Expand All @@ -79,7 +85,9 @@ ModuleWrap::~ModuleWrap() {

Local<Context> ModuleWrap::context() const {
Local<Value> obj = object()->GetInternalField(kContextObjectSlot).As<Value>();
if (obj.IsEmpty()) return {};
// If this fails, there is likely a bug e.g. ModuleWrap::context() is accessed
// before the ModuleWrap constructor completes.
CHECK(obj->IsObject());
return obj.As<Object>()->GetCreationContext().ToLocalChecked();
}

Expand Down Expand Up @@ -227,18 +235,16 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
return;
}

ModuleWrap* obj = new ModuleWrap(env, that, module, url);

if (synthetic) {
obj->synthetic_ = true;
obj->object()->SetInternalField(kSyntheticEvaluationStepsSlot, args[3]);
}

// Use the extras object as an object whose GetCreationContext() will be the
// original `context`, since the `Context` itself strictly speaking cannot
// be stored in an internal field.
obj->object()->SetInternalField(kContextObjectSlot,
context->GetExtrasBindingObject());
Local<Object> context_object = context->GetExtrasBindingObject();
Local<Value> synthetic_evaluation_step =
synthetic ? args[3] : Undefined(env->isolate()).As<v8::Value>();

ModuleWrap* obj = new ModuleWrap(
env, that, module, url, context_object, synthetic_evaluation_step);

obj->contextify_context_ = contextify_context;

env->hash_to_module_map.emplace(module->GetIdentityHash(), obj);
Expand Down
4 changes: 3 additions & 1 deletion src/module_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ class ModuleWrap : public BaseObject {
ModuleWrap(Environment* env,
v8::Local<v8::Object> object,
v8::Local<v8::Module> module,
v8::Local<v8::String> url);
v8::Local<v8::String> url,
v8::Local<v8::Object> context_object,
v8::Local<v8::Value> synthetic_evaluation_step);
~ModuleWrap() override;

static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down

0 comments on commit 2c2892b

Please sign in to comment.