fix: improved support for cjs and cts modules (#26558)

* cts support
* better cjs/cts type checking
* deno compile cjs/cts support
* More efficient detect cjs (going towards stabilization)
* Determination of whether .js, .ts, .jsx, or .tsx is cjs or esm is only
done after loading
* Support `import x = require(...);`

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

4 files changed, 83 insertions, 97 deletions

diff --git a/ext/node/lib.rs b/ext/node/lib.rs
index b34bea815..db6d08e11 100644
--- a/ext/node/lib.rs
+++ b/ext/node/lib.rs
@@ -9,15 +9,11 @@ use std::path::Path;
 use std::path::PathBuf;
 
 use deno_core::error::AnyError;
-use deno_core::located_script_name;
 use deno_core::op2;
 use deno_core::url::Url;
 #[allow(unused_imports)]
 use deno_core::v8;
 use deno_core::v8::ExternalReference;
-use deno_core::JsRuntime;
-use deno_fs::sync::MaybeSend;
-use deno_fs::sync::MaybeSync;
 use node_resolver::NpmResolverRc;
 use once_cell::sync::Lazy;
 
@@ -125,16 +121,17 @@ impl NodePermissions for deno_permissions::PermissionsContainer {
 }
 
 #[allow(clippy::disallowed_types)]
-pub type NodeRequireResolverRc =
-  deno_fs::sync::MaybeArc<dyn NodeRequireResolver>;
+pub type NodeRequireLoaderRc = std::rc::Rc<dyn NodeRequireLoader>;
 
-pub trait NodeRequireResolver: std::fmt::Debug + MaybeSend + MaybeSync {
+pub trait NodeRequireLoader {
   #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
   fn ensure_read_permission<'a>(
     &self,
     permissions: &mut dyn NodePermissions,
     path: &'a Path,
   ) -> Result<Cow<'a, Path>, AnyError>;
+
+  fn load_text_file_lossy(&self, path: &Path) -> Result<String, AnyError>;
 }
 
 pub static NODE_ENV_VAR_ALLOWLIST: Lazy<HashSet<String>> = Lazy::new(|| {
@@ -152,10 +149,12 @@ fn op_node_build_os() -> String {
   env!("TARGET").split('-').nth(2).unwrap().to_string()
 }
 
+#[derive(Clone)]
 pub struct NodeExtInitServices {
-  pub node_require_resolver: NodeRequireResolverRc,
+  pub node_require_loader: NodeRequireLoaderRc,
   pub node_resolver: NodeResolverRc,
   pub npm_resolver: NpmResolverRc,
+  pub pkg_json_resolver: PackageJsonResolverRc,
 }
 
 deno_core::extension!(deno_node,
@@ -639,9 +638,10 @@ deno_core::extension!(deno_node,
     state.put(options.fs.clone());
 
     if let Some(init) = &options.maybe_init {
-      state.put(init.node_require_resolver.clone());
+      state.put(init.node_require_loader.clone());
       state.put(init.node_resolver.clone());
       state.put(init.npm_resolver.clone());
+      state.put(init.pkg_json_resolver.clone());
     }
   },
   global_template_middleware = global_template_middleware,
@@ -761,33 +761,16 @@ deno_core::extension!(deno_node,
   },
 );
 
-pub fn load_cjs_module(
-  js_runtime: &mut JsRuntime,
-  module: &str,
-  main: bool,
-  inspect_brk: bool,
-) -> Result<(), AnyError> {
-  fn escape_for_single_quote_string(text: &str) -> String {
-    text.replace('\\', r"\\").replace('\'', r"\'")
-  }
-
-  let source_code = format!(
-    r#"(function loadCjsModule(moduleName, isMain, inspectBrk) {{
-      Deno[Deno.internal].node.loadCjsModule(moduleName, isMain, inspectBrk);
-    }})('{module}', {main}, {inspect_brk});"#,
-    main = main,
-    module = escape_for_single_quote_string(module),
-    inspect_brk = inspect_brk,
-  );
-
-  js_runtime.execute_script(located_script_name!(), source_code)?;
-  Ok(())
-}
-
 pub type NodeResolver = node_resolver::NodeResolver<DenoFsNodeResolverEnv>;
 #[allow(clippy::disallowed_types)]
 pub type NodeResolverRc =
   deno_fs::sync::MaybeArc<node_resolver::NodeResolver<DenoFsNodeResolverEnv>>;
+pub type PackageJsonResolver =
+  node_resolver::PackageJsonResolver<DenoFsNodeResolverEnv>;
+#[allow(clippy::disallowed_types)]
+pub type PackageJsonResolverRc = deno_fs::sync::MaybeArc<
+  node_resolver::PackageJsonResolver<DenoFsNodeResolverEnv>,
+>;
 
 #[derive(Debug)]
 pub struct DenoFsNodeResolverEnv {
diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs
index 7524fb43c..30db8b629 100644
--- a/ext/node/ops/require.rs
+++ b/ext/node/ops/require.rs
@@ -9,6 +9,7 @@ use deno_core::OpState;
 use deno_fs::FileSystemRc;
 use deno_package_json::PackageJsonRc;
 use deno_path_util::normalize_path;
+use deno_path_util::url_to_file_path;
 use node_resolver::NodeModuleKind;
 use node_resolver::NodeResolutionMode;
 use node_resolver::REQUIRE_CONDITIONS;
@@ -19,9 +20,10 @@ use std::path::PathBuf;
 use std::rc::Rc;
 
 use crate::NodePermissions;
-use crate::NodeRequireResolverRc;
+use crate::NodeRequireLoaderRc;
 use crate::NodeResolverRc;
 use crate::NpmResolverRc;
+use crate::PackageJsonResolverRc;
 
 #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
 fn ensure_read_permission<'a, P>(
@@ -31,9 +33,9 @@ fn ensure_read_permission<'a, P>(
 where
   P: NodePermissions + 'static,
 {
-  let resolver = state.borrow::<NodeRequireResolverRc>().clone();
+  let loader = state.borrow::<NodeRequireLoaderRc>().clone();
   let permissions = state.borrow_mut::<P>();
-  resolver.ensure_read_permission(permissions, file_path)
+  loader.ensure_read_permission(permissions, file_path)
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -54,10 +56,14 @@ pub enum RequireError {
   PackageImportsResolve(
     #[from] node_resolver::errors::PackageImportsResolveError,
   ),
-  #[error("failed to convert '{0}' to file path")]
-  FilePathConversion(Url),
+  #[error(transparent)]
+  FilePathConversion(#[from] deno_path_util::UrlToFilePathError),
+  #[error(transparent)]
+  UrlConversion(#[from] deno_path_util::PathToUrlError),
   #[error(transparent)]
   Fs(#[from] deno_io::fs::FsError),
+  #[error(transparent)]
+  ReadModule(deno_core::error::AnyError),
   #[error("Unable to get CWD: {0}")]
   UnableToGetCwd(deno_io::fs::FsError),
 }
@@ -229,8 +235,11 @@ pub fn op_require_is_deno_dir_package(
   state: &mut OpState,
   #[string] path: String,
 ) -> bool {
-  let resolver = state.borrow::<NpmResolverRc>();
-  resolver.in_npm_package_at_file_path(&PathBuf::from(path))
+  let resolver = state.borrow::<NodeResolverRc>();
+  match deno_path_util::url_from_file_path(&PathBuf::from(path)) {
+    Ok(specifier) => resolver.in_npm_package(&specifier),
+    Err(_) => false,
+  }
 }
 
 #[op2]
@@ -411,8 +420,8 @@ where
     return Ok(None);
   }
 
-  let node_resolver = state.borrow::<NodeResolverRc>();
-  let pkg = node_resolver
+  let pkg_json_resolver = state.borrow::<PackageJsonResolverRc>();
+  let pkg = pkg_json_resolver
     .get_closest_package_json_from_path(&PathBuf::from(parent_path.unwrap()))
     .ok()
     .flatten();
@@ -441,6 +450,7 @@ where
 
   let referrer = deno_core::url::Url::from_file_path(&pkg.path).unwrap();
   if let Some(exports) = &pkg.exports {
+    let node_resolver = state.borrow::<NodeResolverRc>();
     let r = node_resolver.package_exports_resolve(
       &pkg.path,
       &expansion,
@@ -470,10 +480,13 @@ where
   P: NodePermissions + 'static,
 {
   let file_path = PathBuf::from(file_path);
+  // todo(dsherret): there's multiple borrows to NodeRequireLoaderRc here
   let file_path = ensure_read_permission::<P>(state, &file_path)
     .map_err(RequireError::Permission)?;
-  let fs = state.borrow::<FileSystemRc>();
-  Ok(fs.read_text_file_lossy_sync(&file_path, None)?)
+  let loader = state.borrow::<NodeRequireLoaderRc>();
+  loader
+    .load_text_file_lossy(&file_path)
+    .map_err(RequireError::ReadModule)
 }
 
 #[op2]
@@ -503,11 +516,12 @@ where
   P: NodePermissions + 'static,
 {
   let fs = state.borrow::<FileSystemRc>();
-  let npm_resolver = state.borrow::<NpmResolverRc>();
   let node_resolver = state.borrow::<NodeResolverRc>();
+  let pkg_json_resolver = state.borrow::<PackageJsonResolverRc>();
 
   let modules_path = PathBuf::from(&modules_path_str);
-  let pkg_path = if npm_resolver.in_npm_package_at_file_path(&modules_path)
+  let modules_specifier = deno_path_util::url_from_file_path(&modules_path)?;
+  let pkg_path = if node_resolver.in_npm_package(&modules_specifier)
     && !uses_local_node_modules_dir
   {
     modules_path
@@ -521,7 +535,7 @@ where
     }
   };
   let Some(pkg) =
-    node_resolver.load_package_json(&pkg_path.join("package.json"))?
+    pkg_json_resolver.load_package_json(&pkg_path.join("package.json"))?
   else {
     return Ok(None);
   };
@@ -561,8 +575,8 @@ where
 {
   let filename = PathBuf::from(filename);
   // permissions: allow reading the closest package.json files
-  let node_resolver = state.borrow::<NodeResolverRc>().clone();
-  node_resolver.get_closest_package_json_from_path(&filename)
+  let pkg_json_resolver = state.borrow::<PackageJsonResolverRc>();
+  pkg_json_resolver.get_closest_package_json_from_path(&filename)
 }
 
 #[op2]
@@ -574,13 +588,13 @@ pub fn op_require_read_package_scope<P>(
 where
   P: NodePermissions + 'static,
 {
-  let node_resolver = state.borrow::<NodeResolverRc>().clone();
+  let pkg_json_resolver = state.borrow::<PackageJsonResolverRc>();
   let package_json_path = PathBuf::from(package_json_path);
   if package_json_path.file_name() != Some("package.json".as_ref()) {
     // permissions: do not allow reading a non-package.json file
     return None;
   }
-  node_resolver
+  pkg_json_resolver
     .load_package_json(&package_json_path)
     .ok()
     .flatten()
@@ -599,14 +613,15 @@ where
   let referrer_path = PathBuf::from(&referrer_filename);
   let referrer_path = ensure_read_permission::<P>(state, &referrer_path)
     .map_err(RequireError::Permission)?;
-  let node_resolver = state.borrow::<NodeResolverRc>();
+  let pkg_json_resolver = state.borrow::<PackageJsonResolverRc>();
   let Some(pkg) =
-    node_resolver.get_closest_package_json_from_path(&referrer_path)?
+    pkg_json_resolver.get_closest_package_json_from_path(&referrer_path)?
   else {
     return Ok(None);
   };
 
   if pkg.imports.is_some() {
+    let node_resolver = state.borrow::<NodeResolverRc>();
     let referrer_url = Url::from_file_path(&referrer_filename).unwrap();
     let url = node_resolver.package_imports_resolve(
       &request,
@@ -637,13 +652,6 @@ fn url_to_file_path_string(url: &Url) -> Result<String, RequireError> {
   Ok(file_path.to_string_lossy().into_owned())
 }
 
-fn url_to_file_path(url: &Url) -> Result<PathBuf, RequireError> {
-  match url.to_file_path() {
-    Ok(file_path) => Ok(file_path),
-    Err(()) => Err(RequireError::FilePathConversion(url.clone())),
-  }
-}
-
 #[op2(fast)]
 pub fn op_require_can_parse_as_esm(
   scope: &mut v8::HandleScope,
diff --git a/ext/node/ops/worker_threads.rs b/ext/node/ops/worker_threads.rs
index e33cf9157..d2e575882 100644
--- a/ext/node/ops/worker_threads.rs
+++ b/ext/node/ops/worker_threads.rs
@@ -4,14 +4,12 @@ use deno_core::op2;
 use deno_core::url::Url;
 use deno_core::OpState;
 use deno_fs::FileSystemRc;
-use node_resolver::NodeResolution;
 use std::borrow::Cow;
 use std::path::Path;
 use std::path::PathBuf;
 
 use crate::NodePermissions;
-use crate::NodeRequireResolverRc;
-use crate::NodeResolverRc;
+use crate::NodeRequireLoaderRc;
 
 #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
 fn ensure_read_permission<'a, P>(
@@ -21,9 +19,9 @@ fn ensure_read_permission<'a, P>(
 where
   P: NodePermissions + 'static,
 {
-  let resolver = state.borrow::<NodeRequireResolverRc>().clone();
+  let loader = state.borrow::<NodeRequireLoaderRc>().clone();
   let permissions = state.borrow_mut::<P>();
-  resolver.ensure_read_permission(permissions, file_path)
+  loader.ensure_read_permission(permissions, file_path)
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -42,14 +40,11 @@ pub enum WorkerThreadsFilenameError {
   UrlToPath,
   #[error("File not found [{0:?}]")]
   FileNotFound(PathBuf),
-  #[error("Neither ESM nor CJS")]
-  NeitherEsmNorCjs,
-  #[error("{0}")]
-  UrlToNodeResolution(node_resolver::errors::UrlToNodeResolutionError),
   #[error(transparent)]
   Fs(#[from] deno_io::fs::FsError),
 }
 
+// todo(dsherret): we should remove this and do all this work inside op_create_worker
 #[op2]
 #[string]
 pub fn op_worker_threads_filename<P>(
@@ -88,30 +83,5 @@ where
       url_path.to_path_buf(),
     ));
   }
-  let node_resolver = state.borrow::<NodeResolverRc>();
-  match node_resolver
-    .url_to_node_resolution(url)
-    .map_err(WorkerThreadsFilenameError::UrlToNodeResolution)?
-  {
-    NodeResolution::Esm(u) => Ok(u.to_string()),
-    NodeResolution::CommonJs(u) => wrap_cjs(u),
-    NodeResolution::BuiltIn(_) => {
-      Err(WorkerThreadsFilenameError::NeitherEsmNorCjs)
-    }
-  }
-}
-
-///
-/// Wrap a CJS file-URL and the required setup in a stringified `data:`-URL
-///
-fn wrap_cjs(url: Url) -> Result<String, WorkerThreadsFilenameError> {
-  let path = url
-    .to_file_path()
-    .map_err(|_| WorkerThreadsFilenameError::UrlToPath)?;
-  let filename = path.file_name().unwrap().to_string_lossy();
-  Ok(format!(
-    "data:text/javascript,import {{ createRequire }} from \"node:module\";\
-    const require = createRequire(\"{}\"); require(\"./{}\");",
-    url, filename,
-  ))
+  Ok(url.to_string())
 }
diff --git a/ext/node/polyfills/01_require.js b/ext/node/polyfills/01_require.js
index 7935903a8..296d819aa 100644
--- a/ext/node/polyfills/01_require.js
+++ b/ext/node/polyfills/01_require.js
@@ -1071,13 +1071,35 @@ Module._extensions[".js"] = function (module, filename) {
     } else if (pkg?.type === "commonjs") {
       format = "commonjs";
     }
-  } else if (StringPrototypeEndsWith(filename, ".cjs")) {
-    format = "commonjs";
   }
 
   module._compile(content, filename, format);
 };
 
+Module._extensions[".ts"] =
+  Module._extensions[".jsx"] =
+  Module._extensions[".tsx"] =
+    function (module, filename) {
+      const content = op_require_read_file(filename);
+
+      let format;
+      const pkg = op_require_read_closest_package_json(filename);
+      if (pkg?.type === "module") {
+        format = "module";
+      } else if (pkg?.type === "commonjs") {
+        format = "commonjs";
+      }
+
+      module._compile(content, filename, format);
+    };
+
+Module._extensions[".cjs"] =
+  Module._extensions[".cts"] =
+    function (module, filename) {
+      const content = op_require_read_file(filename);
+      module._compile(content, filename, "commonjs");
+    };
+
 function loadESMFromCJS(module, filename, code) {
   const namespace = op_import_sync(
     url.pathToFileURL(filename).toString(),
@@ -1087,7 +1109,10 @@ function loadESMFromCJS(module, filename, code) {
   module.exports = namespace;
 }
 
-Module._extensions[".mjs"] = function (module, filename) {
+Module._extensions[".mjs"] = Module._extensions[".mts"] = function (
+  module,
+  filename,
+) {
   loadESMFromCJS(module, filename);
 };