diff --git a/src/workerd/jsg/modules.c++ b/src/workerd/jsg/modules.c++ index 6bfb78eb332..2c2cd036e3f 100644 --- a/src/workerd/jsg/modules.c++ +++ b/src/workerd/jsg/modules.c++ @@ -9,47 +9,8 @@ #include namespace workerd::jsg { - namespace { -// 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" -}; - -} // namespace - -void logIfNodeSpecifier(kj::StringPtr specifier) { - if (NODEJS_BUILTINS.contains(specifier)) { - LOG_NOSENTRY(WARNING, "Using require() to import a module with a known Node.js built-in name", - specifier); - } -} -namespace { // The CompileCache is used to hold cached compilation data for built-in JavaScript modules. // // Importantly, this is a process-lifetime in-memory cache that is only appropriate for @@ -97,7 +58,6 @@ v8::MaybeLocal resolveCallback(v8::Local context, "referrer passed to resolveCallback isn't in modules table"); auto spec = kj::str(specifier); - logIfNodeSpecifier(spec); // If the referrer module is a built-in, it is only permitted to resolve // internal modules. If the worker bundle provided an override for a builtin, @@ -294,8 +254,6 @@ v8::Local CommonJsModuleContext::require(jsg::Lock& js, kj::String sp auto modulesForResolveCallback = getModulesForResolveCallback(js.v8Isolate); KJ_REQUIRE(modulesForResolveCallback != nullptr, "didn't expect resolveCallback() now"); - logIfNodeSpecifier(specifier); - kj::Path targetPath = ([&] { // If the specifier begins with one of our known prefixes, let's not resolve // it against the referrer. @@ -598,11 +556,39 @@ NodeJsModuleContext::NodeJsModuleContext(jsg::Lock& js, kj::Path path) exports(js.v8Ref(module->getExports(js))) {} v8::Local NodeJsModuleContext::require(jsg::Lock& js, kj::String specifier) { + // 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; - logIfNodeSpecifier(specifier); if (NODEJS_BUILTINS.contains(specifier)) { specifier = kj::str("node:", specifier); isNodeBuiltin = true; diff --git a/src/workerd/jsg/modules.h b/src/workerd/jsg/modules.h index e85d239587e..be4bf30283d 100644 --- a/src/workerd/jsg/modules.h +++ b/src/workerd/jsg/modules.h @@ -14,8 +14,6 @@ namespace workerd::jsg { -void logIfNodeSpecifier(kj::StringPtr specifier); - class CommonJsModuleContext; class CommonJsModuleObject: public jsg::Object { @@ -436,12 +434,6 @@ class ModuleRegistryImpl final: public ModuleRegistry { void add(kj::Path& specifier, ModuleInfo&& info) { entries.insert(kj::heap(specifier, Type::BUNDLE, kj::fwd(info))); - - if (entries.size() > 1) { - // This is just to give us some sense of how many workers make use of multiple modules - // in the bundle... - LOG_PERIODICALLY(WARNING, "NOSENTRY Worker using multiple modules in a single bundle."); - } } void addBuiltinModule(Module::Reader module) { @@ -835,9 +827,6 @@ v8::MaybeLocal dynamicImportCallback(v8::Local context // If the specifier begins with one of our known prefixes, let's not resolve // it against the referrer. auto spec = kj::str(specifier); - - logIfNodeSpecifier(spec); - if (spec.startsWith("node:") || spec.startsWith("cloudflare:") || spec.startsWith("workerd:")) {