fix(node): improve cjs tracking (#22673)

We were missing saying that a file is CJS when some Deno code imported
from the node_modules directory at runtime.

cli/cache/mod.rs

@@ -196,7 +196,9 @@ impl Loader for FetchCacher {
   ) -> LoadFuture {
     use deno_graph::source::CacheSetting as LoaderCacheSetting;
 
-    if specifier.path().contains("/node_modules/") {
+    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

cli/factory.rs

@@ -483,14 +483,12 @@ impl CliFactory {
       .get_or_try_init_async(
         async {
           Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
-            fs: self.fs().clone(),
-            cjs_resolutions: Some(self.cjs_resolutions().clone()),
             sloppy_imports_resolver: if self.options.unstable_sloppy_imports() {
               Some(SloppyImportsResolver::new(self.fs().clone()))
             } else {
               None
             },
-            node_resolver: Some(self.node_resolver().await?.clone()),
+            node_resolver: Some(self.cli_node_resolver().await?.clone()),
             npm_resolver: if self.options.no_npm() {
               None
             } else {
@@ -714,7 +712,8 @@ impl CliFactory {
       .cli_node_resolver
       .get_or_try_init_async(async {
         Ok(Arc::new(CliNodeResolver::new(
-          self.cjs_resolutions().clone(),
+          Some(self.cjs_resolutions().clone()),
+          self.fs().clone(),
           self.node_resolver().await?.clone(),
           self.npm_resolver().await?.clone(),
         )))

cli/lsp/analysis.rs

@@ -8,6 +8,7 @@ use super::tsc;
 
 use crate::args::jsr_url;
 use crate::npm::CliNpmResolver;
+use crate::resolver::CliNodeResolver;
 use crate::tools::lint::create_linter;
 use crate::util::path::specifier_to_file_path;
 
@@ -23,7 +24,6 @@ use deno_core::serde_json::json;
 use deno_core::ModuleSpecifier;
 use deno_lint::diagnostic::LintDiagnostic;
 use deno_lint::rules::LintRule;
-use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_node::NpmResolver;
 use deno_runtime::deno_node::PathClean;
 use deno_runtime::permissions::PermissionsContainer;
@@ -179,15 +179,15 @@ fn code_as_string(code: &Option<lsp::NumberOrString>) -> String {
 pub struct TsResponseImportMapper<'a> {
   documents: &'a Documents,
   maybe_import_map: Option<&'a ImportMap>,
-  node_resolver: Option<&'a NodeResolver>,
+  node_resolver: Option<&'a CliNodeResolver>,
   npm_resolver: Option<&'a dyn CliNpmResolver>,
 }
 
 impl<'a> TsResponseImportMapper<'a> {
   pub fn new(
     documents: &'a Documents,
     maybe_import_map: Option<&'a ImportMap>,
-    node_resolver: Option<&'a NodeResolver>,
+    node_resolver: Option<&'a CliNodeResolver>,
     npm_resolver: Option<&'a dyn CliNpmResolver>,
   ) -> Self {
     Self {

cli/lsp/documents.rs

@@ -19,6 +19,7 @@ use crate::lsp::logging::lsp_warn;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
+use crate::resolver::CliNodeResolver;
 use crate::resolver::SloppyImportsFsEntry;
 use crate::resolver::SloppyImportsResolution;
 use crate::resolver::SloppyImportsResolver;
@@ -40,11 +41,9 @@ use deno_graph::source::ResolutionMode;
 use deno_graph::GraphImport;
 use deno_graph::Resolution;
 use deno_lockfile::Lockfile;
-use deno_runtime::deno_fs::RealFs;
 use deno_runtime::deno_node;
 use deno_runtime::deno_node::NodeResolution;
 use deno_runtime::deno_node::NodeResolutionMode;
-use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_node::PackageJson;
 use deno_runtime::permissions::PermissionsContainer;
 use deno_semver::jsr::JsrPackageReqReference;
@@ -835,7 +834,7 @@ pub struct UpdateDocumentConfigOptions<'a> {
   pub maybe_config_file: Option<&'a ConfigFile>,
   pub maybe_package_json: Option<&'a PackageJson>,
   pub maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
-  pub node_resolver: Option<Arc<NodeResolver>>,
+  pub node_resolver: Option<Arc<CliNodeResolver>>,
   pub npm_resolver: Option<Arc<dyn CliNpmResolver>>,
 }
 
@@ -897,10 +896,8 @@ impl Documents {
       resolver_config_hash: 0,
       imports: Default::default(),
       resolver: Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
-        fs: Arc::new(RealFs),
         node_resolver: None,
         npm_resolver: None,
-        cjs_resolutions: None,
         package_json_deps_provider: Arc::new(PackageJsonDepsProvider::default()),
         maybe_jsx_import_source_config: None,
         maybe_import_map: None,
@@ -1273,7 +1270,7 @@ impl Documents {
         NpmPackageReqReference::from_str(&specifier)
       {
         results.push(node_resolve_npm_req_ref(
-          npm_req_ref,
+          &npm_req_ref,
           maybe_npm,
           referrer,
         ));
@@ -1408,12 +1405,9 @@ impl Documents {
     );
     let deps_provider =
       Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps));
-    let fs = Arc::new(RealFs);
     self.resolver = Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
-      fs: fs.clone(),
       node_resolver: options.node_resolver,
       npm_resolver: options.npm_resolver,
-      cjs_resolutions: None, // only used for runtime
       package_json_deps_provider: deps_provider,
       maybe_jsx_import_source_config: maybe_jsx_config,
       maybe_import_map: options.maybe_import_map,
@@ -1693,7 +1687,7 @@ impl Documents {
     }
 
     if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) {
-      return node_resolve_npm_req_ref(npm_ref, maybe_npm, referrer);
+      return node_resolve_npm_req_ref(&npm_ref, maybe_npm, referrer);
     }
     let doc = self.get(specifier)?;
     let maybe_module = doc.maybe_js_module().and_then(|r| r.as_ref().ok());
@@ -1724,29 +1718,21 @@ impl Documents {
 }
 
 fn node_resolve_npm_req_ref(
-  npm_req_ref: NpmPackageReqReference,
+  npm_req_ref: &NpmPackageReqReference,
   maybe_npm: Option<&StateNpmSnapshot>,
   referrer: &ModuleSpecifier,
 ) -> Option<(ModuleSpecifier, MediaType)> {
   maybe_npm.map(|npm| {
     NodeResolution::into_specifier_and_media_type(
       npm
-        .npm_resolver
-        .resolve_pkg_folder_from_deno_module_req(npm_req_ref.req(), referrer)
-        .ok()
-        .and_then(|package_folder| {
-          npm
-            .node_resolver
-            .resolve_package_subpath_from_deno_module(
-              &package_folder,
-              npm_req_ref.sub_path(),
-              referrer,
-              NodeResolutionMode::Types,
-              &PermissionsContainer::allow_all(),
-            )
-            .ok()
-            .flatten()
-        }),
+        .node_resolver
+        .resolve_req_reference(
+          npm_req_ref,
+          &PermissionsContainer::allow_all(),
+          referrer,
+          NodeResolutionMode::Types,
+        )
+        .ok(),
     )
   })
 }

cli/lsp/language_server.rs

@@ -121,6 +121,7 @@ use crate::npm::CliNpmResolverCreateOptions;
 use crate::npm::CliNpmResolverManagedCreateOptions;
 use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption;
 use crate::npm::CliNpmResolverManagedSnapshotOption;
+use crate::resolver::CliNodeResolver;
 use crate::tools::fmt::format_file;
 use crate::tools::fmt::format_parsed_source;
 use crate::tools::upgrade::check_for_upgrades_for_lsp;
@@ -146,7 +147,7 @@ struct LspNpmServices {
   /// Npm's search api.
   search_api: CliNpmSearchApi,
   /// Node resolver.
-  node_resolver: Option<Arc<NodeResolver>>,
+  node_resolver: Option<Arc<CliNodeResolver>>,
   /// Resolver for npm packages.
   resolver: Option<Arc<dyn CliNpmResolver>>,
 }
@@ -171,7 +172,7 @@ pub struct LanguageServer(Arc<tokio::sync::RwLock<Inner>>, CancellationToken);
 
 #[derive(Clone, Debug)]
 pub struct StateNpmSnapshot {
-  pub node_resolver: Arc<NodeResolver>,
+  pub node_resolver: Arc<CliNodeResolver>,
   pub npm_resolver: Arc<dyn CliNpmResolver>,
 }
 
@@ -760,10 +761,18 @@ impl Inner {
       .map(|resolver| resolver.clone_snapshotted())
       .map(|resolver| {
         let fs = Arc::new(deno_fs::RealFs);
-        let node_resolver =
-          Arc::new(NodeResolver::new(fs, resolver.clone().into_npm_resolver()));
-        StateNpmSnapshot {
+        let node_resolver = Arc::new(NodeResolver::new(
+          fs.clone(),
+          resolver.clone().into_npm_resolver(),
+        ));
+        let cli_node_resolver = Arc::new(CliNodeResolver::new(
+          None,
+          fs,
           node_resolver,
+          resolver.clone(),
+        ));
+        StateNpmSnapshot {
+          node_resolver: cli_node_resolver,
           npm_resolver: resolver,
         }
       });
@@ -907,9 +916,15 @@ impl Inner {
       self.config.maybe_node_modules_dir_path().cloned(),
     )
     .await;
-    self.npm.node_resolver = Some(Arc::new(NodeResolver::new(
+    let node_resolver = Arc::new(NodeResolver::new(
       Arc::new(deno_fs::RealFs),
       npm_resolver.clone().into_npm_resolver(),
+    ));
+    self.npm.node_resolver = Some(Arc::new(CliNodeResolver::new(
+      None,
+      Arc::new(deno_fs::RealFs),
+      node_resolver,
+      npm_resolver.clone(),
     )));
     self.npm.resolver = Some(npm_resolver);
 

cli/module_loader.rs

@@ -52,6 +52,7 @@ use deno_graph::Module;
 use deno_graph::Resolution;
 use deno_lockfile::Lockfile;
 use deno_runtime::deno_fs;
+use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::permissions::PermissionsContainer;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_terminal::colors;
@@ -513,9 +514,13 @@ impl CliModuleLoader {
     if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package(
       specifier,
       referrer,
+      NodeResolutionMode::Execution,
       permissions,
     ) {
-      return result;
+      return match result? {
+        Some(res) => Ok(res.into_url()),
+        None => Err(generic_error("not found")),
+      };
     }
 
     let graph = self.shared.graph_container.graph();
@@ -538,18 +543,23 @@ impl CliModuleLoader {
               .as_managed()
               .unwrap() // byonm won't create a Module::Npm
               .resolve_pkg_folder_from_deno_module(module.nv_reference.nv())?;
-            self
+            let maybe_resolution = self
               .shared
               .node_resolver
-              .resolve_package_sub_path(
+              .resolve_package_sub_path_from_deno_module(
                 &package_folder,
                 module.nv_reference.sub_path(),
                 referrer,
+                NodeResolutionMode::Execution,
                 permissions,
               )
               .with_context(|| {
                 format!("Could not resolve '{}'.", module.nv_reference)
-              })?
+              })?;
+            match maybe_resolution {
+              Some(res) => res.into_url(),
+              None => return Err(generic_error("not found")),
+            }
           }
           Some(Module::Node(module)) => module.specifier.clone(),
           Some(Module::Js(module)) => module.specifier.clone(),
@@ -592,11 +602,16 @@ impl CliModuleLoader {
         if let Ok(reference) =
           NpmPackageReqReference::from_specifier(&specifier)
         {
-          return self.shared.node_resolver.resolve_req_reference(
-            &reference,
-            permissions,
-            referrer,
-          );
+          return self
+            .shared
+            .node_resolver
+            .resolve_req_reference(
+              &reference,
+              permissions,
+              referrer,
+              NodeResolutionMode::Execution,
+            )
+            .map(|res| res.into_url());
         }
       }
     }