fix: improved support for cjs and cts modules

.dprint.json

@@ -68,7 +68,7 @@
     "third_party"
   ],
   "plugins": [
-    "https://plugins.dprint.dev/typescript-0.93.0.wasm",
+    "https://plugins.dprint.dev/typescript-0.93.1.wasm",
     "https://plugins.dprint.dev/json-0.19.4.wasm",
     "https://plugins.dprint.dev/markdown-0.17.8.wasm",
     "https://plugins.dprint.dev/toml-0.6.3.wasm",

Cargo.toml

@@ -45,12 +45,12 @@ license = "MIT"
 repository = "https://github.com/denoland/deno"
 
 [workspace.dependencies]
-deno_ast = { version = "=0.42.2", features = ["transpiling"] }
+deno_ast = { version = "=0.43.1", features = ["transpiling"] }
 deno_core = { version = "0.314.2" }
 
 deno_bench_util = { version = "0.168.0", path = "./bench_util" }
 deno_lockfile = "=0.23.1"
-deno_media_type = { version = "0.1.4", features = ["module_specifier"] }
+deno_media_type = { version = "0.2.0", features = ["module_specifier"] }
 deno_npm = "=0.25.4"
 deno_path_util = "=0.2.1"
 deno_permissions = { version = "0.34.0", path = "./runtime/permissions" }
@@ -111,7 +111,7 @@ console_static_text = "=0.8.1"
 dashmap = "5.5.3"
 data-encoding = "2.3.3"
 data-url = "=0.3.0"
-deno_cache_dir = "=0.13.1"
+deno_cache_dir = "=0.13.2"
 deno_package_json = { version = "0.1.2", default-features = false }
 dlopen2 = "0.6.1"
 ecb = "=0.1.2"

cli/Cargo.toml

@@ -72,9 +72,9 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposa
 deno_cache_dir = { workspace = true }
 deno_config = { version = "=0.37.2", features = ["workspace", "sync"] }
 deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
-deno_doc = { version = "0.154.0", default-features = false, features = ["rust", "html", "syntect"] }
-deno_graph = { version = "=0.83.4" }
-deno_lint = { version = "=0.67.0", features = ["docs"] }
+deno_doc = { version = "0.156.0", default-features = false, features = ["rust", "html", "syntect"] }
+deno_graph = { version = "=0.84.0" }
+deno_lint = { version = "=0.68.0", features = ["docs"] }
 deno_lockfile.workspace = true
 deno_npm.workspace = true
 deno_package_json.workspace = true
@@ -107,7 +107,7 @@ dotenvy = "0.15.7"
 dprint-plugin-json = "=0.19.4"
 dprint-plugin-jupyter = "=0.1.5"
 dprint-plugin-markdown = "=0.17.8"
-dprint-plugin-typescript = "=0.93.0"
+dprint-plugin-typescript = "=0.93.1"
 env_logger = "=0.10.0"
 fancy-regex = "=0.10.0"
 faster-hex.workspace = true

cli/args/mod.rs

@@ -200,6 +200,8 @@ pub fn ts_config_to_transpile_and_emit_options(
       precompile_jsx_dynamic_props: None,
       transform_jsx,
       var_decl_imports: false,
+      // todo(dsherret): support verbatim_module_syntax here properly
+      verbatim_module_syntax: false,
     },
     deno_ast::EmitOptions {
       inline_sources: options.inline_sources,
@@ -1595,6 +1597,15 @@ impl CliOptions {
   }
 
   pub fn use_byonm(&self) -> bool {
+    if matches!(
+      self.sub_command(),
+      DenoSubcommand::Install(_)
+        | DenoSubcommand::Add(_)
+        | DenoSubcommand::Remove(_)
+    ) {
+      // For `deno install/add/remove` we want to force the managed resolver so it can set up `node_modules/` directory.
+      return false;
+    }
     if self.node_modules_dir().ok().flatten().is_none()
       && self.maybe_node_modules_folder.is_some()
       && self

cli/cache/cache_db.rs

@@ -57,7 +57,7 @@ impl rusqlite::types::FromSql for CacheDBHash {
 }
 
 /// What should the cache should do on failure?
-#[derive(Default)]
+#[derive(Debug, Default)]
 pub enum CacheFailure {
   /// Return errors if failure mode otherwise unspecified.
   #[default]
@@ -69,6 +69,7 @@ pub enum CacheFailure {
 }
 
 /// Configuration SQL and other parameters for a [`CacheDB`].
+#[derive(Debug)]
 pub struct CacheDBConfiguration {
   /// SQL to run for a new database.
   pub table_initializer: &'static str,
@@ -98,6 +99,7 @@ impl CacheDBConfiguration {
   }
 }
 
+#[derive(Debug)]
 enum ConnectionState {
   Connected(Connection),
   Blackhole,
@@ -106,7 +108,7 @@ enum ConnectionState {
 
 /// A cache database that eagerly initializes itself off-thread, preventing initialization operations
 /// from blocking the main thread.
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct CacheDB {
   // TODO(mmastrac): We can probably simplify our thread-safe implementation here
   conn: Arc<Mutex<OnceCell<ConnectionState>>>,

cli/cache/emit.rs

@@ -10,6 +10,7 @@ use deno_core::unsync::sync::AtomicFlag;
 use super::DiskCache;
 
 /// The cache that stores previously emitted files.
+#[derive(Debug)]
 pub struct EmitCache {
   disk_cache: DiskCache,
   emit_failed_flag: AtomicFlag,
@@ -91,6 +92,7 @@ impl EmitCache {
 
 const LAST_LINE_PREFIX: &str = "\n// denoCacheMetadata=";
 
+#[derive(Debug)]
 struct EmitFileSerializer {
   cli_version: &'static str,
 }

cli/cache/mod.rs

@@ -8,14 +8,9 @@ use crate::file_fetcher::FetchOptions;
 use crate::file_fetcher::FetchPermissionsOptionRef;
 use crate::file_fetcher::FileFetcher;
 use crate::file_fetcher::FileOrRedirect;
-use crate::npm::CliNpmResolver;
-use crate::resolver::CliNodeResolver;
 use crate::util::fs::atomic_write_file_with_retries;
 use crate::util::fs::atomic_write_file_with_retries_and_fs;
 use crate::util::fs::AtomicWriteFileFsAdapter;
-use crate::util::path::specifier_has_extension;
-use crate::util::text_encoding::arc_str_to_bytes;
-use crate::util::text_encoding::from_utf8_lossy_owned;
 
 use deno_ast::MediaType;
 use deno_core::futures;
@@ -25,7 +20,9 @@ use deno_graph::source::CacheInfo;
 use deno_graph::source::LoadFuture;
 use deno_graph::source::LoadResponse;
 use deno_graph::source::Loader;
+use deno_runtime::deno_fs;
 use deno_runtime::deno_permissions::PermissionsContainer;
+use node_resolver::InNpmPackageChecker;
 use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
@@ -60,7 +57,6 @@ pub use fast_check::FastCheckCache;
 pub use incremental::IncrementalCache;
 pub use module_info::ModuleInfoCache;
 pub use node::NodeAnalysisCache;
-pub use parsed_source::EsmOrCjsChecker;
 pub use parsed_source::LazyGraphSourceParser;
 pub use parsed_source::ParsedSourceCache;
 
@@ -181,46 +177,40 @@ pub struct FetchCacherOptions {
   pub permissions: PermissionsContainer,
   /// If we're publishing for `deno publish`.
   pub is_deno_publish: bool,
-  pub unstable_detect_cjs: bool,
 }
 
 /// A "wrapper" for the FileFetcher and DiskCache for the Deno CLI that provides
 /// a concise interface to the DENO_DIR when building module graphs.
 pub struct FetchCacher {
   pub file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
-  esm_or_cjs_checker: Arc<EsmOrCjsChecker>,
   file_fetcher: Arc<FileFetcher>,
+  fs: Arc<dyn deno_fs::FileSystem>,
   global_http_cache: Arc<GlobalHttpCache>,
-  node_resolver: Arc<CliNodeResolver>,
-  npm_resolver: Arc<dyn CliNpmResolver>,
+  in_npm_pkg_checker: Arc<dyn InNpmPackageChecker>,
   module_info_cache: Arc<ModuleInfoCache>,
   permissions: PermissionsContainer,
   is_deno_publish: bool,
-  unstable_detect_cjs: bool,
   cache_info_enabled: bool,
 }
 
 impl FetchCacher {
   pub fn new(
-    esm_or_cjs_checker: Arc<EsmOrCjsChecker>,
     file_fetcher: Arc<FileFetcher>,
+    fs: Arc<dyn deno_fs::FileSystem>,
     global_http_cache: Arc<GlobalHttpCache>,
-    node_resolver: Arc<CliNodeResolver>,
-    npm_resolver: Arc<dyn CliNpmResolver>,
+    in_npm_pkg_checker: Arc<dyn InNpmPackageChecker>,
     module_info_cache: Arc<ModuleInfoCache>,
     options: FetchCacherOptions,
   ) -> Self {
     Self {
       file_fetcher,
-      esm_or_cjs_checker,
+      fs,
       global_http_cache,
-      node_resolver,
-      npm_resolver,
+      in_npm_pkg_checker,
       module_info_cache,
       file_header_overrides: options.file_header_overrides,
       permissions: options.permissions,
       is_deno_publish: options.is_deno_publish,
-      unstable_detect_cjs: options.unstable_detect_cjs,
       cache_info_enabled: false,
     }
   }
@@ -271,70 +261,23 @@ impl Loader for FetchCacher {
   ) -> LoadFuture {
     use deno_graph::source::CacheSetting as LoaderCacheSetting;
 
-    if specifier.scheme() == "file" {
-      if specifier.path().contains("/node_modules/") {
-        // The specifier might be in a completely different symlinked tree than
-        // what the node_modules url is in (ex. `/my-project-1/node_modules`
-        // symlinked to `/my-project-2/node_modules`), so first we checked if the path
-        // is in a node_modules dir to avoid needlessly canonicalizing, then now compare
-        // against the canonicalized specifier.
-        let specifier =
-          crate::node::resolve_specifier_into_node_modules(specifier);
-        if self.npm_resolver.in_npm_package(&specifier) {
-          return Box::pin(futures::future::ready(Ok(Some(
-            LoadResponse::External { specifier },
-          ))));
-        }
-      }
-
-      // make local CJS modules external to the graph
-      if specifier_has_extension(specifier, "cjs") {
+    if specifier.scheme() == "file"
+      && specifier.path().contains("/node_modules/")
+    {
+      // The specifier might be in a completely different symlinked tree than
+      // what the node_modules url is in (ex. `/my-project-1/node_modules`
+      // symlinked to `/my-project-2/node_modules`), so first we checked if the path
+      // is in a node_modules dir to avoid needlessly canonicalizing, then now compare
+      // against the canonicalized specifier.
+      let specifier = crate::node::resolve_specifier_into_node_modules(
+        specifier,
+        self.fs.as_ref(),
+      );
+      if self.in_npm_pkg_checker.in_npm_package(&specifier) {
         return Box::pin(futures::future::ready(Ok(Some(
-          LoadResponse::External {
-            specifier: specifier.clone(),
-          },
+          LoadResponse::External { specifier },
         ))));
       }
-
-      if self.unstable_detect_cjs && specifier_has_extension(specifier, "js") {
-        if let Ok(Some(pkg_json)) =
-          self.node_resolver.get_closest_package_json(specifier)
-        {
-          if pkg_json.typ == "commonjs" {
-            if let Ok(path) = specifier.to_file_path() {
-              if let Ok(bytes) = std::fs::read(&path) {
-                let text: Arc<str> = from_utf8_lossy_owned(bytes).into();
-                let is_es_module = match self.esm_or_cjs_checker.is_esm(
-                  specifier,
-                  text.clone(),
-                  MediaType::JavaScript,
-                ) {
-                  Ok(value) => value,
-                  Err(err) => {
-                    return Box::pin(futures::future::ready(Err(err.into())));
-                  }
-                };
-                if !is_es_module {
-                  self.node_resolver.mark_cjs_resolution(specifier.clone());
-                  return Box::pin(futures::future::ready(Ok(Some(
-                    LoadResponse::External {
-                      specifier: specifier.clone(),
-                    },
-                  ))));
-                } else {
-                  return Box::pin(futures::future::ready(Ok(Some(
-                    LoadResponse::Module {
-                      specifier: specifier.clone(),
-                      content: arc_str_to_bytes(text),
-                      maybe_headers: None,
-                    },
-                  ))));
-                }
-              }
-            }
-          }
-        }
-      }
     }
 
     if self.is_deno_publish