Move NonModuleScript out of modules.h/modules.c++ (#3296)

Author: jasnell

src/workerd/api/unsafe.c++

@@ -1,5 +1,7 @@
 #include "unsafe.h"
 
+#include <workerd/jsg/script.h>
+
 namespace workerd::api {
 
 namespace {
@@ -18,8 +20,8 @@ inline kj::StringPtr getName(jsg::Optional<kj::String>& name, kj::StringPtr def)
 jsg::JsValue UnsafeEval::eval(jsg::Lock& js, kj::String script, jsg::Optional<kj::String> name) {
   js.setAllowEval(true);
   KJ_DEFER(js.setAllowEval(false));
-  auto compiled = jsg::NonModuleScript::compile(script, js, getName(name, EVAL_STR));
-  return jsg::JsValue(compiled.runAndReturn(js.v8Context()));
+  auto compiled = jsg::NonModuleScript::compile(js, script, getName(name, EVAL_STR));
+  return compiled.runAndReturn(js);
 }
 
 UnsafeEval::UnsafeEvalFunction UnsafeEval::newFunction(jsg::Lock& js,

src/workerd/io/worker.c++

@@ -18,6 +18,7 @@
 #include <workerd/jsg/inspector.h>
 #include <workerd/jsg/jsg.h>
 #include <workerd/jsg/modules-new.h>
+#include <workerd/jsg/script.h>
 #include <workerd/jsg/util.h>
 #include <workerd/util/batch-queue.h>
 #include <workerd/util/color-util.h>
@@ -1344,7 +1345,7 @@ Worker::Script::Script(kj::Own<const Isolate> isolateParam,
                   auto limitScope =
                       isolate->getLimitEnforcer().enterStartupJs(lock, limitErrorOrTime);
                   impl->unboundScriptOrMainModule =
-                      jsg::NonModuleScript::compile(script.mainScript, lock, script.mainScriptName);
+                      jsg::NonModuleScript::compile(lock, script.mainScript, script.mainScriptName);
                 }
 
                 break;
@@ -1661,7 +1662,7 @@ Worker::Worker(kj::Own<const Script> scriptParam,
               KJ_CASE_ONEOF(unboundScript, jsg::NonModuleScript) {
                 auto limitScope =
                     script->isolate->getLimitEnforcer().enterStartupJs(lock, limitErrorOrTime);
-                unboundScript.run(lock.v8Context());
+                unboundScript.run(lock);
               }
               KJ_CASE_ONEOF(mainModule, kj::Path) {
                 KJ_IF_SOME(ns,

src/workerd/jsg/BUILD.bazel

@@ -21,6 +21,7 @@ wd_cc_library(
         "modules-new.c++",
         "promise.c++",
         "resource.c++",
+        "script.c++",
         "ser.c++",
         "setup.c++",
         "util.c++",
@@ -44,6 +45,7 @@ wd_cc_library(
         "modules-new.h",
         "promise.h",
         "resource.h",
+        "script.h",
         "ser.h",
         "setup.h",
         "struct.h",

src/workerd/jsg/modules.c++

@@ -280,26 +280,6 @@ v8::Local<v8::Value> CommonJsModuleContext::require(jsg::Lock& js, kj::String sp
   return ModuleRegistry::requireImpl(js, info, options);
 }
 
-v8::Local<v8::Value> NonModuleScript::runAndReturn(v8::Local<v8::Context> context) const {
-  auto isolate = context->GetIsolate();
-  auto boundScript = unboundScript.Get(isolate)->BindToCurrentContext();
-  return check(boundScript->Run(context));
-}
-
-void NonModuleScript::run(v8::Local<v8::Context> context) const {
-  auto isolate = context->GetIsolate();
-  auto boundScript = unboundScript.Get(isolate)->BindToCurrentContext();
-  check(boundScript->Run(context));
-}
-
-NonModuleScript NonModuleScript::compile(kj::StringPtr code, jsg::Lock& js, kj::StringPtr name) {
-  // Create a dummy script origin for it to appear in Sources panel.
-  auto isolate = js.v8Isolate;
-  v8::ScriptOrigin origin(v8StrIntern(isolate, name));
-  v8::ScriptCompiler::Source source(v8Str(isolate, code), origin);
-  return NonModuleScript(js, check(v8::ScriptCompiler::CompileUnboundScript(isolate, &source)));
-}
-
 void instantiateModule(
     jsg::Lock& js, v8::Local<v8::Module>& module, InstantiateModuleOptions options) {
   KJ_ASSERT(!module.IsEmpty());

src/workerd/jsg/modules.h

@@ -171,28 +171,6 @@ class NodeJsModuleContext: public jsg::Object {
   jsg::Value exports;
 };
 
-// jsg::NonModuleScript wraps a v8::UnboundScript.
-class NonModuleScript {
- public:
-  NonModuleScript(jsg::Lock& js, v8::Local<v8::UnboundScript> script)
-      : unboundScript(js.v8Isolate, script) {}
-
-  NonModuleScript(NonModuleScript&&) = default;
-  NonModuleScript& operator=(NonModuleScript&&) = default;
-
-  // Running the script will create a v8::Script instance bound to the given
-  // context then will run it to completion.
-  void run(v8::Local<v8::Context> context) const;
-
-  v8::Local<v8::Value> runAndReturn(v8::Local<v8::Context> context) const;
-
-  static jsg::NonModuleScript compile(
-      kj::StringPtr code, jsg::Lock& js, kj::StringPtr name = "worker.js");
-
- private:
-  v8::Global<v8::UnboundScript> unboundScript;
-};
-
 enum class InstantiateModuleOptions {
   // Allows pending top-level await in the module when evaluated. Will cause
   // the microtask queue to be drained once in an attempt to resolve those.

src/workerd/jsg/script.c++

@@ -0,0 +1,23 @@
+#include "script.h"
+
+namespace workerd::jsg {
+
+jsg::JsValue NonModuleScript::runAndReturn(jsg::Lock& js) const {
+  auto boundScript = unboundScript.Get(js.v8Isolate)->BindToCurrentContext();
+  return jsg::JsValue(check(boundScript->Run(js.v8Context())));
+}
+
+void NonModuleScript::run(jsg::Lock& js) const {
+  auto boundScript = unboundScript.Get(js.v8Isolate)->BindToCurrentContext();
+  check(boundScript->Run(js.v8Context()));
+}
+
+NonModuleScript NonModuleScript::compile(jsg::Lock& js, kj::StringPtr code, kj::StringPtr name) {
+  // Create a dummy script origin for it to appear in Sources panel.
+  auto isolate = js.v8Isolate;
+  v8::ScriptOrigin origin(js.str(name));
+  v8::ScriptCompiler::Source source(js.str(code), origin);
+  return NonModuleScript(js, check(v8::ScriptCompiler::CompileUnboundScript(isolate, &source)));
+}
+
+}  // namespace workerd::jsg

src/workerd/jsg/script.h

@@ -0,0 +1,32 @@
+#pragma once
+
+#include <workerd/jsg/jsg.h>
+
+namespace workerd::jsg {
+
+// jsg::NonModuleScript wraps a v8::UnboundScript.
+// An unbound script is a script that has been compiled but is not
+// yet bound to a specific context.
+class NonModuleScript final {
+ public:
+  NonModuleScript(jsg::Lock& js, v8::Local<v8::UnboundScript> script)
+      : unboundScript(js.v8Isolate, script) {}
+
+  NonModuleScript(NonModuleScript&&) = default;
+  NonModuleScript& operator=(NonModuleScript&&) = default;
+  KJ_DISALLOW_COPY(NonModuleScript);
+
+  // Running the script will create a v8::Script instance bound to the given
+  // context then will run it to completion.
+  void run(jsg::Lock& js) const;
+
+  jsg::JsValue runAndReturn(jsg::Lock& js) const;
+
+  static jsg::NonModuleScript compile(
+      jsg::Lock& js, kj::StringPtr code, kj::StringPtr name = "worker.js");
+
+ private:
+  v8::Global<v8::UnboundScript> unboundScript;
+};
+
+}  // namespace workerd::jsg