Skip to content

Commit dd9382d

Browse files
committed
Cleanup: get/setWorkerEnv() should expect an Object, not a Value.
In fact they were already doing a runtime check for this, but by changing the types we can enforce it at compile time.
1 parent a1d4728 commit dd9382d

File tree

5 files changed

+10
-17
lines changed

5 files changed

+10
-17
lines changed

src/workerd/api/modules.c++

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,8 @@ kj::Maybe<jsg::JsObject> EnvModule::getCurrent(jsg::Lock& js) {
2020
if (FeatureFlags::get(js).getDisableImportableEnv()) return kj::none;
2121

2222
// Otherwise, fallback to provide the stored environment.
23-
return js.getWorkerEnv().map([&](const jsg::Value& val) -> jsg::JsObject {
24-
auto handle = val.getHandle(js);
25-
JSG_REQUIRE(handle->IsObject(), TypeError, "Expected environment to be an object.");
26-
return jsg::JsObject(handle.As<v8::Object>());
23+
return js.getWorkerEnv().map([&](const jsg::V8Ref<v8::Object>& val) -> jsg::JsObject {
24+
return jsg::JsObject(val.getHandle(js));
2725
});
2826
}
2927

src/workerd/api/node/util.c++

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,7 @@ jsg::JsValue UtilModule::getBuiltinModule(jsg::Lock& js, kj::String specifier) {
252252
jsg::JsObject UtilModule::getEnvObject(jsg::Lock& js) {
253253
if (FeatureFlags::get(js).getPopulateProcessEnv()) {
254254
KJ_IF_SOME(env, js.getWorkerEnv()) {
255-
jsg::JsValue val(env.getHandle(js));
256-
KJ_IF_SOME(obj, val.tryCast<jsg::JsObject>()) {
257-
return obj;
258-
}
255+
return jsg::JsObject(env.getHandle(js));
259256
}
260257
}
261258

src/workerd/io/worker.c++

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1618,7 +1618,7 @@ Worker::Worker(kj::Own<const Script> scriptParam,
16181618
// Use `env` variable.
16191619
bindingsScope = v8::Object::New(lock.v8Isolate);
16201620
if (!FeatureFlags::get(js).getDisableImportableEnv()) {
1621-
lock.setWorkerEnv(lock.v8Ref(bindingsScope.As<v8::Value>()));
1621+
lock.setWorkerEnv(lock.v8Ref(bindingsScope));
16221622
}
16231623
} else {
16241624
// Use global-scope bindings.

src/workerd/jsg/jsg.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2706,10 +2706,10 @@ class Lock {
27062706
virtual void reportError(const JsValue& value) = 0;
27072707

27082708
// Store the worker environment.
2709-
virtual void setWorkerEnv(Value value) = 0;
2709+
virtual void setWorkerEnv(V8Ref<v8::Object> value) = 0;
27102710

27112711
// Retrieve the worker environment.
2712-
virtual kj::Maybe<Value> getWorkerEnv() = 0;
2712+
virtual kj::Maybe<V8Ref<v8::Object>> getWorkerEnv() = 0;
27132713

27142714
// Resolve an internal module namespace from the given specifier.
27152715
// This variation can be used only for internal built-ins.

src/workerd/jsg/setup.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -697,15 +697,13 @@ class Isolate: public IsolateBase {
697697
}
698698
}
699699

700-
void setWorkerEnv(Value value) override {
701-
auto handle = value.getHandle(*this);
702-
KJ_ASSERT(handle->IsObject());
703-
jsgIsolate.workerEnvObj.Reset(v8Isolate, handle.template As<v8::Object>());
700+
void setWorkerEnv(V8Ref<v8::Object> value) override {
701+
jsgIsolate.workerEnvObj.Reset(v8Isolate, value.getHandle(*this));
704702
}
705703

706-
kj::Maybe<Value> getWorkerEnv() override {
704+
kj::Maybe<V8Ref<v8::Object>> getWorkerEnv() override {
707705
if (jsgIsolate.workerEnvObj.IsEmpty()) return kj::none;
708-
return v8Ref<v8::Value>(jsgIsolate.workerEnvObj.Get(v8Isolate));
706+
return v8Ref<v8::Object>(jsgIsolate.workerEnvObj.Get(v8Isolate));
709707
}
710708

711709
private:

0 commit comments

Comments
 (0)