From 214cebe21424837ea2a86525f8a19ab764e97211 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Apr 2023 07:20:00 -0700 Subject: [PATCH] fixup! Implement NodeJsCompatModule type --- src/workerd/api/node/context.c++ | 170 ------------------------- src/workerd/api/node/context.h | 88 ------------- src/workerd/api/node/node.h | 3 - src/workerd/jsg/modules.c++ | 193 +++++++++++++++++++++++++++++ src/workerd/jsg/modules.h | 80 ++++++++++++ src/workerd/server/workerd-api.c++ | 4 +- 6 files changed, 276 insertions(+), 262 deletions(-) delete mode 100644 src/workerd/api/node/context.c++ delete mode 100644 src/workerd/api/node/context.h diff --git a/src/workerd/api/node/context.c++ b/src/workerd/api/node/context.c++ deleted file mode 100644 index cec8ec1194b2..000000000000 --- a/src/workerd/api/node/context.c++ +++ /dev/null @@ -1,170 +0,0 @@ -#include "context.h" -#include -#include -#include -#include -#include - -namespace workerd::jsg { -jsg::Ref jsg::ModuleRegistry::NodeJsModuleInfo::initModuleContext( - jsg::Lock& js, - kj::StringPtr name) { - return jsg::alloc(js.v8Isolate, kj::Path::parse(name)); -} - -v8::MaybeLocal jsg::ModuleRegistry::NodeJsModuleInfo::evaluate( - v8::Isolate* isolate, - jsg::ModuleRegistry::NodeJsModuleInfo& info, - v8::Local module) { - - const auto makeResolvedPromise = [&]() { - v8::Local resolver; - auto context = isolate->GetCurrentContext(); - if (!v8::Promise::Resolver::New(context).ToLocal(&resolver)) { - // Return empty local and allow error to propagate. - return v8::Local(); - } - if (!resolver->Resolve(context, v8::Undefined(isolate)).IsJust()) { - // Return empty local and allow error to propagate. - return v8::Local(); - } - return resolver->GetPromise(); - }; - - v8::MaybeLocal result; - v8::TryCatch catcher(isolate); - auto& js = jsg::Lock::from(isolate); - try { - info.evalFunc(js); - } catch (const JsExceptionThrown&) { - if (catcher.CanContinue()) catcher.ReThrow(); - // leave `result` empty to propagate the JS exception - return v8::MaybeLocal(); - } - - auto ctx = static_cast(info.moduleContext.get()); - - if (module->SetSyntheticModuleExport( - isolate, - v8StrIntern(isolate, "default"_kj), - ctx->module->getExports(js.v8Isolate)).IsJust()) { - result = makeResolvedPromise(); - } else { - // leave `result` empty to propagate the JS exception - } - return result; -} - -} // namespace workerd::jsg - -namespace workerd::api::node { - -v8::Local NodeJsModuleContext::require(kj::String specifier, v8::Isolate* isolate) { - static const std::set NODEJS_BUILTINS { - "_http_agent", "_http_client", "_http_common", - "_http_incoming", "_http_outgoing", "_http_server", - "_stream_duplex", "_stream_passthrough", "_stream_readable", - "_stream_transform", "_stream_wrap", "_stream_writable", - "_tls_common", "_tls_wrap", "assert", - "assert/strict", "async_hooks", "buffer", - "child_process", "cluster", "console", - "constants", "crypto", "dgram", - "diagnostics_channel", "dns", "dns/promises", - "domain", "events", "fs", - "fs/promises", "http", "http2", - "https", "inspector", "inspector/promises", - "module", "net", "os", - "path", "path/posix", "path/win32", - "perf_hooks", "process", "punycode", - "querystring", "readline", "readline/promises", - "repl", "stream", "stream/consumers", - "stream/promises", "stream/web", "string_decoder", - "sys", "timers", "timers/promises", - "tls", "trace_events", "tty", - "url", "util", "util/types", - "v8", "vm", "worker_threads", - "zlib" - }; - - // If it is a bare specifier known to be a Node.js built-in, then prefix the - // specifier with node: - bool isNodeBuiltin = false; - auto resolveOption = jsg::ModuleRegistry::ResolveOption::DEFAULT; - if (NODEJS_BUILTINS.contains(specifier)) { - specifier = kj::str("node:", specifier); - isNodeBuiltin = true; - resolveOption = jsg::ModuleRegistry::ResolveOption::BUILTIN_ONLY; - } else if (specifier.startsWith("node:")) { - isNodeBuiltin = true; - resolveOption = jsg::ModuleRegistry::ResolveOption::BUILTIN_ONLY; - } - - // TODO(cleanup): This implementation from here on is identical to the - // CommonJsModuleContext::require. We should consolidate these as the - // next step. - - auto modulesForResolveCallback = jsg::getModulesForResolveCallback(isolate); - KJ_REQUIRE(modulesForResolveCallback != nullptr, "didn't expect resolveCallback() now"); - - kj::Path targetPath = path.parent().eval(specifier); - auto& js = jsg::Lock::from(isolate); - - // require() is only exposed to worker bundle modules so the resolve here is only - // permitted to require worker bundle or built-in modules. Internal modules are - // excluded. - auto& info = JSG_REQUIRE_NONNULL( - modulesForResolveCallback->resolve(js, targetPath, resolveOption), - Error, "No such module \"", targetPath.toString(), "\"."); - // Adding imported from suffix here not necessary like it is for resolveCallback, since we have a - // js stack that will include the parent module's name and location of the failed require(). - - if (!isNodeBuiltin) { - JSG_REQUIRE_NONNULL(info.maybeSynthetic, TypeError, - "Cannot use require() to import an ES Module."); - } - - auto module = info.module.getHandle(js); - auto context = isolate->GetCurrentContext(); - - jsg::instantiateModule(js, module); - - auto handle = jsg::check(module->Evaluate(context)); - KJ_ASSERT(handle->IsPromise()); - auto prom = handle.As(); - - // This assert should always pass since evaluateSyntheticModuleCallback() for CommonJS - // modules (below) always returns an already-resolved promise. - KJ_ASSERT(prom->State() != v8::Promise::PromiseState::kPending); - - if (module->GetStatus() == v8::Module::kErrored) { - jsg::throwTunneledException(isolate, module->GetException()); - } - - return jsg::check(module->GetModuleNamespace().As() - ->Get(context, jsg::v8StrIntern(isolate, "default"))); -} - -v8::Local NodeJsModuleContext::getBuffer(jsg::Lock& js) { - auto value = require(kj::str("node:buffer"), js.v8Isolate); - JSG_REQUIRE(value->IsObject(), TypeError, "Invalid node:buffer implementation"); - auto module = value.As(); - auto buffer = jsg::check(module->Get(js.v8Context(), jsg::v8Str(js.v8Isolate, "Buffer"))); - JSG_REQUIRE(buffer->IsFunction(), TypeError, "Invalid node:buffer implementation"); - return buffer; -} - -v8::Local NodeJsModuleContext::getProcess(jsg::Lock& js) { - auto value = require(kj::str("node:process"), js.v8Isolate); - JSG_REQUIRE(value->IsObject(), TypeError, "Invalid node:process implementation"); - return value; -} - -kj::String NodeJsModuleContext::getFilename() { - return path.toString(true); -} - -kj::String NodeJsModuleContext::getDirname() { - return path.parent().toString(true); -} - -} // namespace workerd::api::node diff --git a/src/workerd/api/node/context.h b/src/workerd/api/node/context.h deleted file mode 100644 index 47a8ef359a58..000000000000 --- a/src/workerd/api/node/context.h +++ /dev/null @@ -1,88 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -namespace workerd::api::node { - -// The NodeJsModuleContext is used in support of the NodeJsCompatModule type. -// It adds additional extensions to the global context that would normally be -// expected within the global scope of a Node.js compatible module (such as -// Buffer and process). - -class NodeJsModuleObject: public jsg::Object { -public: - NodeJsModuleObject(v8::Isolate* isolate, kj::String path) - : exports(isolate, v8::Object::New(isolate)), - path(kj::mv(path)) {} - - v8::Local getExports(v8::Isolate* isolate) { return exports.getHandle(isolate); } - void setExports(jsg::Value value) { exports = kj::mv(value); } - kj::StringPtr getPath() { return path; } - - // TODO(soon): Additional properties... We can likely get by without implementing most - // of these (if any). - // * children https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulechildren - // * filename https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulefilename - // * id https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleid - // * isPreloading https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleispreloading - // * loaded https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleloaded - // * parent https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleparent - // * paths https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulepaths - // * require https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulerequireid - - JSG_RESOURCE_TYPE(NodeJsModuleObject) { - JSG_INSTANCE_PROPERTY(exports, getExports, setExports); - JSG_READONLY_INSTANCE_PROPERTY(path, getPath); - } -private: - jsg::Value exports; - kj::String path; -}; - -class NodeJsModuleContext: public jsg::Object { - // The NodeJsModuleContext is similar in structure to CommonJsModuleContext - // with the exception that: - // (a) Node.js-compat built-in modules can be required without the `node:` specifier-prefix - // (meaning that worker-bundle modules whose names conflict with the Node.js built-ins - // are ignored), and - // (b) The common Node.js globals that we implement are exposed. For instance, `process` - // and `Buffer` will be found at the global scope. -public: - NodeJsModuleContext(v8::Isolate* isolate, kj::Path path) - : module(jsg::alloc(isolate, path.toString(true))), path(kj::mv(path)), - exports(isolate, module->getExports(isolate)) {} - - v8::Local require(kj::String specifier, v8::Isolate* isolate); - v8::Local getBuffer(jsg::Lock& js); - v8::Local getProcess(jsg::Lock& js); - - // TODO(soon): Implement setImmediate/clearImmediate - - jsg::Ref getModule(v8::Isolate* isolate) { return module.addRef(); } - - v8::Local getExports(v8::Isolate* isolate) { return exports.getHandle(isolate); } - void setExports(jsg::Value value) { exports = kj::mv(value); } - - kj::String getFilename(); - kj::String getDirname(); - - JSG_RESOURCE_TYPE(NodeJsModuleContext) { - JSG_METHOD(require); - JSG_READONLY_INSTANCE_PROPERTY(module, getModule); - JSG_INSTANCE_PROPERTY(exports, getExports, setExports); - JSG_LAZY_INSTANCE_PROPERTY(Buffer, getBuffer); - JSG_LAZY_INSTANCE_PROPERTY(process, getProcess); - JSG_LAZY_INSTANCE_PROPERTY(__filename, getFilename); - JSG_LAZY_INSTANCE_PROPERTY(__dirname, getDirname); - } - - jsg::Ref module; -private: - kj::Path path; - jsg::Value exports; -}; - -} // namespace workerd::api::node diff --git a/src/workerd/api/node/node.h b/src/workerd/api/node/node.h index 02ba48236365..a95459be44aa 100644 --- a/src/workerd/api/node/node.h +++ b/src/workerd/api/node/node.h @@ -2,7 +2,6 @@ #include "async-hooks.h" #include "buffer.h" -#include "context.h" #include "crypto.h" #include #include @@ -61,8 +60,6 @@ void registerNodeJsCompatModules( } #define EW_NODE_ISOLATE_TYPES \ - api::node::NodeJsModuleObject, \ - api::node::NodeJsModuleContext, \ api::node::CompatibilityFlags, \ EW_NODE_BUFFER_ISOLATE_TYPES, \ EW_NODE_CRYPTO_ISOLATE_TYPES, \ diff --git a/src/workerd/jsg/modules.c++ b/src/workerd/jsg/modules.c++ index 5b5f7d64e150..63a7e6e6736b 100644 --- a/src/workerd/jsg/modules.c++ +++ b/src/workerd/jsg/modules.c++ @@ -453,4 +453,197 @@ v8::Local compileWasmModule(jsg::Lock& js, js.v8Isolate, v8::MemorySpan(code.begin(), code.size()))); } + +// ====================================================================================== + +jsg::Ref ModuleRegistry::NodeJsModuleInfo::initModuleContext( + jsg::Lock& js, + kj::StringPtr name) { + return jsg::alloc(js.v8Isolate, kj::Path::parse(name)); +} + +v8::MaybeLocal ModuleRegistry::NodeJsModuleInfo::evaluate( + v8::Isolate* isolate, + ModuleRegistry::NodeJsModuleInfo& info, + v8::Local module) { + + const auto makeResolvedPromise = [&]() { + v8::Local resolver; + auto context = isolate->GetCurrentContext(); + if (!v8::Promise::Resolver::New(context).ToLocal(&resolver)) { + // Return empty local and allow error to propagate. + return v8::Local(); + } + if (!resolver->Resolve(context, v8::Undefined(isolate)).IsJust()) { + // Return empty local and allow error to propagate. + return v8::Local(); + } + return resolver->GetPromise(); + }; + + v8::MaybeLocal result; + v8::TryCatch catcher(isolate); + auto& js = jsg::Lock::from(isolate); + try { + info.evalFunc(js); + } catch (const JsExceptionThrown&) { + if (catcher.CanContinue()) catcher.ReThrow(); + // leave `result` empty to propagate the JS exception + return v8::MaybeLocal(); + } + + auto ctx = static_cast(info.moduleContext.get()); + + if (module->SetSyntheticModuleExport( + isolate, + v8StrIntern(isolate, "default"_kj), + ctx->module->getExports(js.v8Isolate)).IsJust()) { + result = makeResolvedPromise(); + } else { + // leave `result` empty to propagate the JS exception + } + return result; +} + +NodeJsModuleContext::NodeJsModuleContext(v8::Isolate* isolate, kj::Path path) + : module(jsg::alloc(isolate, path.toString(true))), path(kj::mv(path)), + exports(isolate, module->getExports(isolate)) {} + +v8::Local NodeJsModuleContext::require(kj::String specifier, v8::Isolate* isolate) { + // This list must be kept in sync with the list of builtins from Node.js. + // It should be unlikely that anything is ever removed from this list, and + // adding items to it is considered a semver-major change in Node.js. + static const std::set NODEJS_BUILTINS { + "_http_agent", "_http_client", "_http_common", + "_http_incoming", "_http_outgoing", "_http_server", + "_stream_duplex", "_stream_passthrough", "_stream_readable", + "_stream_transform", "_stream_wrap", "_stream_writable", + "_tls_common", "_tls_wrap", "assert", + "assert/strict", "async_hooks", "buffer", + "child_process", "cluster", "console", + "constants", "crypto", "dgram", + "diagnostics_channel", "dns", "dns/promises", + "domain", "events", "fs", + "fs/promises", "http", "http2", + "https", "inspector", "inspector/promises", + "module", "net", "os", + "path", "path/posix", "path/win32", + "perf_hooks", "process", "punycode", + "querystring", "readline", "readline/promises", + "repl", "stream", "stream/consumers", + "stream/promises", "stream/web", "string_decoder", + "sys", "timers", "timers/promises", + "tls", "trace_events", "tty", + "url", "util", "util/types", + "v8", "vm", "worker_threads", + "zlib" + }; + + // If it is a bare specifier known to be a Node.js built-in, then prefix the + // specifier with node: + bool isNodeBuiltin = false; + auto resolveOption = jsg::ModuleRegistry::ResolveOption::DEFAULT; + if (NODEJS_BUILTINS.contains(specifier)) { + specifier = kj::str("node:", specifier); + isNodeBuiltin = true; + resolveOption = jsg::ModuleRegistry::ResolveOption::BUILTIN_ONLY; + } else if (specifier.startsWith("node:")) { + isNodeBuiltin = true; + resolveOption = jsg::ModuleRegistry::ResolveOption::BUILTIN_ONLY; + } + + // TODO(cleanup): This implementation from here on is identical to the + // CommonJsModuleContext::require. We should consolidate these as the + // next step. + + auto modulesForResolveCallback = jsg::getModulesForResolveCallback(isolate); + KJ_REQUIRE(modulesForResolveCallback != nullptr, "didn't expect resolveCallback() now"); + + kj::Path targetPath = path.parent().eval(specifier); + auto& js = jsg::Lock::from(isolate); + + // require() is only exposed to worker bundle modules so the resolve here is only + // permitted to require worker bundle or built-in modules. Internal modules are + // excluded. + auto& info = JSG_REQUIRE_NONNULL( + modulesForResolveCallback->resolve(js, targetPath, resolveOption), + Error, "No such module \"", targetPath.toString(), "\"."); + // Adding imported from suffix here not necessary like it is for resolveCallback, since we have a + // js stack that will include the parent module's name and location of the failed require(). + + if (!isNodeBuiltin) { + JSG_REQUIRE_NONNULL(info.maybeSynthetic, TypeError, + "Cannot use require() to import an ES Module."); + } + + auto module = info.module.getHandle(js); + auto context = isolate->GetCurrentContext(); + + jsg::instantiateModule(js, module); + + auto handle = jsg::check(module->Evaluate(context)); + KJ_ASSERT(handle->IsPromise()); + auto prom = handle.As(); + + // This assert should always pass since evaluateSyntheticModuleCallback() for CommonJS + // modules (below) always returns an already-resolved promise. + KJ_ASSERT(prom->State() != v8::Promise::PromiseState::kPending); + + if (module->GetStatus() == v8::Module::kErrored) { + jsg::throwTunneledException(isolate, module->GetException()); + } + + return jsg::check(module->GetModuleNamespace().As() + ->Get(context, jsg::v8StrIntern(isolate, "default"))); +} + +v8::Local NodeJsModuleContext::getBuffer(jsg::Lock& js) { + auto value = require(kj::str("node:buffer"), js.v8Isolate); + JSG_REQUIRE(value->IsObject(), TypeError, "Invalid node:buffer implementation"); + auto module = value.As(); + auto buffer = jsg::check(module->Get(js.v8Context(), jsg::v8Str(js.v8Isolate, "Buffer"))); + JSG_REQUIRE(buffer->IsFunction(), TypeError, "Invalid node:buffer implementation"); + return buffer; +} + +v8::Local NodeJsModuleContext::getProcess(jsg::Lock& js) { + auto value = require(kj::str("node:process"), js.v8Isolate); + JSG_REQUIRE(value->IsObject(), TypeError, "Invalid node:process implementation"); + return value; +} + +kj::String NodeJsModuleContext::getFilename() { + return path.toString(true); +} + +kj::String NodeJsModuleContext::getDirname() { + return path.parent().toString(true); +} + +jsg::Ref NodeJsModuleContext::getModule(v8::Isolate* isolate) { + return module.addRef(); +} + +v8::Local NodeJsModuleContext::getExports(v8::Isolate* isolate) { + return exports.getHandle(isolate); +} + +void NodeJsModuleContext::setExports(jsg::Value value) { + exports = kj::mv(value); +} + +NodeJsModuleObject::NodeJsModuleObject(v8::Isolate* isolate, kj::String path) + : exports(isolate, v8::Object::New(isolate)), + path(kj::mv(path)) {} + +v8::Local NodeJsModuleObject::getExports(v8::Isolate* isolate) { + return exports.getHandle(isolate); +} + +void NodeJsModuleObject::setExports(jsg::Value value) { + exports = kj::mv(value); +} + +kj::StringPtr NodeJsModuleObject::getPath() { return path; } + } // namespace workerd::jsg diff --git a/src/workerd/jsg/modules.h b/src/workerd/jsg/modules.h index 1a9de2ff27e0..8b18a3965c9c 100644 --- a/src/workerd/jsg/modules.h +++ b/src/workerd/jsg/modules.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace workerd::jsg { @@ -52,6 +53,85 @@ class CommonJsModuleContext: public jsg::Object { jsg::Value exports; }; +// TODO(cleanup): Ideally these would exist over with the rest of the Node.js +// compat related stuff in workerd/api/node but there's a dependency cycle issue +// to work through there. Specifically, these are needed in jsg but jsg cannot +// depend on workerd/api. We should revisit to see if we can get these moved over. + +// The NodeJsModuleContext is used in support of the NodeJsCompatModule type. +// It adds additional extensions to the global context that would normally be +// expected within the global scope of a Node.js compatible module (such as +// Buffer and process). + +class NodeJsModuleObject: public jsg::Object { +public: + NodeJsModuleObject(v8::Isolate* isolate, kj::String path); + + v8::Local getExports(v8::Isolate* isolate); + void setExports(jsg::Value value); + kj::StringPtr getPath(); + + // TODO(soon): Additional properties... We can likely get by without implementing most + // of these (if any). + // * children https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulechildren + // * filename https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulefilename + // * id https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleid + // * isPreloading https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleispreloading + // * loaded https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleloaded + // * parent https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#moduleparent + // * paths https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulepaths + // * require https://nodejs.org/dist/latest-v20.x/docs/api/modules.html#modulerequireid + + JSG_RESOURCE_TYPE(NodeJsModuleObject) { + JSG_INSTANCE_PROPERTY(exports, getExports, setExports); + JSG_READONLY_INSTANCE_PROPERTY(path, getPath); + } +private: + jsg::Value exports; + kj::String path; +}; + +class NodeJsModuleContext: public jsg::Object { + // The NodeJsModuleContext is similar in structure to CommonJsModuleContext + // with the exception that: + // (a) Node.js-compat built-in modules can be required without the `node:` specifier-prefix + // (meaning that worker-bundle modules whose names conflict with the Node.js built-ins + // are ignored), and + // (b) The common Node.js globals that we implement are exposed. For instance, `process` + // and `Buffer` will be found at the global scope. +public: + NodeJsModuleContext(v8::Isolate* isolate, kj::Path path); + + v8::Local require(kj::String specifier, v8::Isolate* isolate); + v8::Local getBuffer(jsg::Lock& js); + v8::Local getProcess(jsg::Lock& js); + + // TODO(soon): Implement setImmediate/clearImmediate + + jsg::Ref getModule(v8::Isolate* isolate); + + v8::Local getExports(v8::Isolate* isolate); + void setExports(jsg::Value value); + + kj::String getFilename(); + kj::String getDirname(); + + JSG_RESOURCE_TYPE(NodeJsModuleContext) { + JSG_METHOD(require); + JSG_READONLY_INSTANCE_PROPERTY(module, getModule); + JSG_INSTANCE_PROPERTY(exports, getExports, setExports); + JSG_LAZY_INSTANCE_PROPERTY(Buffer, getBuffer); + JSG_LAZY_INSTANCE_PROPERTY(process, getProcess); + JSG_LAZY_INSTANCE_PROPERTY(__filename, getFilename); + JSG_LAZY_INSTANCE_PROPERTY(__dirname, getDirname); + } + + jsg::Ref module; +private: + kj::Path path; + jsg::Value exports; +}; + class NonModuleScript { // jsg::NonModuleScript wraps a v8::UnboundScript. public: diff --git a/src/workerd/server/workerd-api.c++ b/src/workerd/server/workerd-api.c++ index 08f71bfbf629..b13bdbf1d558 100644 --- a/src/workerd/server/workerd-api.c++ +++ b/src/workerd/server/workerd-api.c++ @@ -76,7 +76,9 @@ JSG_DECLARE_ISOLATE_TYPE(JsgWorkerdIsolate, jsg::InjectConfiguration, Worker::ApiIsolate::ErrorInterface, jsg::CommonJsModuleObject, - jsg::CommonJsModuleContext); + jsg::CommonJsModuleContext, + jsg::NodeJsModuleObject, + jsg::NodeJsModuleContext); struct WorkerdApiIsolate::Impl { kj::Own features;