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,7 +179,7 @@ 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>,
 }
 
@@ -187,7 +187,7 @@ 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
+        .node_resolver
-        .resolve_pkg_folder_from_deno_module_req(npm_req_ref.req(), referrer)
+        .resolve_req_reference(
-        .ok()
+          npm_req_ref,
-        .and_then(|package_folder| {
+          &PermissionsContainer::allow_all(),
-          npm
+          referrer,
-            .node_resolver
+          NodeResolutionMode::Types,
-            .resolve_package_subpath_from_deno_module(
+        )
-              &package_folder,
+        .ok(),
-              npm_req_ref.sub_path(),
-              referrer,
-              NodeResolutionMode::Types,
-              &PermissionsContainer::allow_all(),
-            )
-            .ok()
-            .flatten()
-        }),
     )
   })
 }

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 =
+        let node_resolver = Arc::new(NodeResolver::new(
-          Arc::new(NodeResolver::new(fs, resolver.clone().into_npm_resolver()));
+          fs.clone(),
-        StateNpmSnapshot {
+          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(
+          return self
-            &reference,
+            .shared
-            permissions,
+            .node_resolver
-            referrer,
+            .resolve_req_reference(
-          );
+              &reference,
+              permissions,
+              referrer,
+              NodeResolutionMode::Execution,
+            )
+            .map(|res| res.into_url());
         }
       }
     }

cli/resolver.rs

@@ -3,7 +3,6 @@
 use deno_ast::MediaType;
 use deno_core::anyhow::anyhow;
 use deno_core::anyhow::Context;
-use deno_core::error::generic_error;
 use deno_core::error::AnyError;
 use deno_core::futures::future;
 use deno_core::futures::future::LocalBoxFuture;
@@ -22,10 +21,12 @@ use deno_runtime::deno_fs;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::is_builtin_node_module;
 use deno_runtime::deno_node::parse_npm_pkg_name;
+use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NodeResolution;
 use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_node::NpmResolver as DenoNodeNpmResolver;
+use deno_runtime::deno_node::PackageJson;
 use deno_runtime::permissions::PermissionsContainer;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageReq;
@@ -63,20 +64,26 @@ pub struct ModuleCodeStringSource {
   pub media_type: MediaType,
 }
 
+#[derive(Debug)]
 pub struct CliNodeResolver {
-  cjs_resolutions: Arc<CjsResolutionStore>,
+  // not used in the LSP
+  cjs_resolutions: Option<Arc<CjsResolutionStore>>,
+  fs: Arc<dyn deno_fs::FileSystem>,
   node_resolver: Arc<NodeResolver>,
+  // todo(dsherret): remove this pub(crate)
   pub(crate) npm_resolver: Arc<dyn CliNpmResolver>,
 }
 
 impl CliNodeResolver {
   pub fn new(
-    cjs_resolutions: Arc<CjsResolutionStore>,
+    cjs_resolutions: Option<Arc<CjsResolutionStore>>,
+    fs: Arc<dyn deno_fs::FileSystem>,
     node_resolver: Arc<NodeResolver>,
     npm_resolver: Arc<dyn CliNpmResolver>,
   ) -> Self {
     Self {
       cjs_resolutions,
+      fs,
       node_resolver,
       npm_resolver,
     }
@@ -86,81 +93,150 @@ impl CliNodeResolver {
     self.npm_resolver.in_npm_package(referrer)
   }
 
+  pub fn get_closest_package_json(
+    &self,
+    referrer: &ModuleSpecifier,
+    permissions: &dyn NodePermissions,
+  ) -> Result<Option<PackageJson>, AnyError> {
+    self
+      .node_resolver
+      .get_closest_package_json(referrer, permissions)
+  }
+
   pub fn resolve_if_in_npm_package(
     &self,
     specifier: &str,
     referrer: &ModuleSpecifier,
+    mode: NodeResolutionMode,
     permissions: &PermissionsContainer,
-  ) -> Option<Result<ModuleSpecifier, AnyError>> {
+  ) -> Option<Result<Option<NodeResolution>, AnyError>> {
     if self.in_npm_package(referrer) {
       // we're in an npm package, so use node resolution
-      Some(
+      Some(self.resolve(specifier, referrer, mode, permissions))
-        self
-          .handle_node_resolve_result(self.node_resolver.resolve(
-            specifier,
-            referrer,
-            NodeResolutionMode::Execution,
-            permissions,
-          ))
-          .with_context(|| {
-            format!("Could not resolve '{specifier}' from '{referrer}'.")
-          }),
-      )
     } else {
       None
     }
   }
 
+  pub fn resolve(
+    &self,
+    specifier: &str,
+    referrer: &ModuleSpecifier,
+    mode: NodeResolutionMode,
+    permissions: &PermissionsContainer,
+  ) -> Result<Option<NodeResolution>, AnyError> {
+    self.handle_node_resolve_result(self.node_resolver.resolve(
+      specifier,
+      referrer,
+      mode,
+      permissions,
+    ))
+  }
+
   pub fn resolve_req_reference(
     &self,
     req_ref: &NpmPackageReqReference,
     permissions: &PermissionsContainer,
     referrer: &ModuleSpecifier,
-  ) -> Result<ModuleSpecifier, AnyError> {
+    mode: NodeResolutionMode,
+  ) -> Result<NodeResolution, AnyError> {
     let package_folder = self
       .npm_resolver
       .resolve_pkg_folder_from_deno_module_req(req_ref.req(), referrer)?;
-    self
+    let maybe_resolution = self.resolve_package_sub_path_from_deno_module(
-      .resolve_package_sub_path(
+      &package_folder,
-        &package_folder,
+      req_ref.sub_path(),
-        req_ref.sub_path(),
+      referrer,
-        referrer,
+      mode,
-        permissions,
+      permissions,
-      )
+    )?;
-      .with_context(|| format!("Could not resolve '{}'.", req_ref))
+    match maybe_resolution {
+      Some(resolution) => Ok(resolution),
+      None => {
+        if self.npm_resolver.as_byonm().is_some() {
+          let package_json_path = package_folder.join("package.json");
+          if !self.fs.exists_sync(&package_json_path) {
+            return Err(anyhow!(
+              "Could not find '{}'. Deno expects the node_modules/ directory to be up to date. Did you forget to run `npm install`?",
+              package_json_path.display()
+            ));
+          }
+        }
+        Err(anyhow!(
+          "Failed resolving package subpath for '{}' in '{}'.",
+          req_ref,
+          package_folder.display()
+        ))
+      }
+    }
   }
 
-  pub fn resolve_package_sub_path(
+  pub fn resolve_package_sub_path_from_deno_module(
     &self,
     package_folder: &Path,
     sub_path: Option<&str>,
     referrer: &ModuleSpecifier,
+    mode: NodeResolutionMode,
     permissions: &PermissionsContainer,
-  ) -> Result<ModuleSpecifier, AnyError> {
+  ) -> Result<Option<NodeResolution>, AnyError> {
     self.handle_node_resolve_result(
       self.node_resolver.resolve_package_subpath_from_deno_module(
         package_folder,
         sub_path,
         referrer,
-        NodeResolutionMode::Execution,
+        mode,
         permissions,
       ),
     )
   }
 
+  pub fn handle_if_in_node_modules(
+    &self,
+    specifier: ModuleSpecifier,
+  ) -> Result<ModuleSpecifier, AnyError> {
+    // skip canonicalizing if we definitely know it's unnecessary
+    if specifier.scheme() == "file"
+      && specifier.path().contains("/node_modules/")
+    {
+      // Specifiers in the node_modules directory are canonicalized
+      // so canoncalize then check if it's in the node_modules directory.
+      // If so, check if we need to store this specifier as being a CJS
+      // resolution.
+      let specifier =
+        crate::node::resolve_specifier_into_node_modules(&specifier);
+      if self.in_npm_package(&specifier) {
+        if let Some(cjs_resolutions) = &self.cjs_resolutions {
+          let resolution =
+            self.node_resolver.url_to_node_resolution(specifier)?;
+          if let NodeResolution::CommonJs(specifier) = &resolution {
+            cjs_resolutions.insert(specifier.clone());
+          }
+          return Ok(resolution.into_url());
+        } else {
+          return Ok(specifier);
+        }
+      }
+    }
+
+    Ok(specifier)
+  }
+
   fn handle_node_resolve_result(
     &self,
     result: Result<Option<NodeResolution>, AnyError>,
-  ) -> Result<ModuleSpecifier, AnyError> {
+  ) -> Result<Option<NodeResolution>, AnyError> {
-    let response = match result? {
+    match result? {
-      Some(response) => response,
+      Some(response) => {
-      None => return Err(generic_error("not found")),
+        if let NodeResolution::CommonJs(specifier) = &response {
-    };
+          // remember that this was a common js resolution
-    if let NodeResolution::CommonJs(specifier) = &response {
+          if let Some(cjs_resolutions) = &self.cjs_resolutions {
-      // remember that this was a common js resolution
+            cjs_resolutions.insert(specifier.clone());
-      self.cjs_resolutions.insert(specifier.clone());
+          }
+        }
+        Ok(Some(response))
+      }
+      None => Ok(None),
     }
-    Ok(response.into_url())
   }
 }
 
@@ -359,24 +435,20 @@ impl MappedSpecifierResolver {
 /// import map, JSX settings.
 #[derive(Debug)]
 pub struct CliGraphResolver {
-  fs: Arc<dyn FileSystem>,
   sloppy_imports_resolver: Option<SloppyImportsResolver>,
   mapped_specifier_resolver: MappedSpecifierResolver,
   maybe_default_jsx_import_source: Option<String>,
   maybe_jsx_import_source_module: Option<String>,
   maybe_vendor_specifier: Option<ModuleSpecifier>,
-  cjs_resolutions: Option<Arc<CjsResolutionStore>>,
+  node_resolver: Option<Arc<CliNodeResolver>>,
-  node_resolver: Option<Arc<NodeResolver>>,
   npm_resolver: Option<Arc<dyn CliNpmResolver>>,
   found_package_json_dep_flag: Arc<AtomicFlag>,
   bare_node_builtins_enabled: bool,
 }
 
 pub struct CliGraphResolverOptions<'a> {
-  pub fs: Arc<dyn FileSystem>,
-  pub cjs_resolutions: Option<Arc<CjsResolutionStore>>,
   pub sloppy_imports_resolver: Option<SloppyImportsResolver>,
-  pub node_resolver: Option<Arc<NodeResolver>>,
+  pub node_resolver: Option<Arc<CliNodeResolver>>,
   pub npm_resolver: Option<Arc<dyn CliNpmResolver>>,
   pub package_json_deps_provider: Arc<PackageJsonDepsProvider>,
   pub maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
@@ -393,8 +465,6 @@ impl CliGraphResolver {
       .map(|n| n.as_byonm().is_some())
       .unwrap_or(false);
     Self {
-      fs: options.fs,
-      cjs_resolutions: options.cjs_resolutions,
       sloppy_imports_resolver: options.sloppy_imports_resolver,
       mapped_specifier_resolver: MappedSpecifierResolver::new(
         options.maybe_import_map,
@@ -496,7 +566,7 @@ impl Resolver for CliGraphResolver {
     }
 
     let referrer = &referrer_range.specifier;
-    let result = self
+    let result: Result<_, ResolveError> = self
       .mapped_specifier_resolver
       .resolve(specifier, referrer)
       .map_err(|err| err.into())
@@ -549,46 +619,16 @@ impl Resolver for CliGraphResolver {
           if let Ok(npm_req_ref) =
             NpmPackageReqReference::from_specifier(specifier)
           {
-            let package_folder = resolver
-              .resolve_pkg_folder_from_deno_module_req(
-                npm_req_ref.req(),
-                referrer,
-              )?;
             let node_resolver = self.node_resolver.as_ref().unwrap();
-            let package_json_path = package_folder.join("package.json");
+            return node_resolver
-            if !self.fs.exists_sync(&package_json_path) {
+              .resolve_req_reference(
-              return Err(ResolveError::Other(anyhow!(
+                &npm_req_ref,
-                "Could not find '{}'. Deno expects the node_modules/ directory to be up to date. Did you forget to run `npm install`?",
+                &PermissionsContainer::allow_all(),
-                package_json_path.display()
-              )));
-            }
-            let maybe_resolution = node_resolver
-              .resolve_package_subpath_from_deno_module(
-                &package_folder,
-                npm_req_ref.sub_path(),
                 referrer,
                 to_node_mode(mode),
-                &PermissionsContainer::allow_all(),
+              )
-              )?;
+              .map(|res| res.into_url())
-            match maybe_resolution {
+              .map_err(|err| err.into());
-              Some(resolution) => {
-                if let Some(cjs_resolutions) = &self.cjs_resolutions {
-                  if let NodeResolution::CommonJs(specifier) = &resolution {
-                    // remember that this was a common js resolution
-                    cjs_resolutions.insert(specifier.clone());
-                  }
-                }
-
-                return Ok(resolution.into_url());
-              }
-              None => {
-                return Err(ResolveError::Other(anyhow!(
-                  "Failed resolving package subpath for '{}' in '{}'.",
-                  npm_req_ref,
-                  package_folder.display()
-                )));
-              }
-            }
           }
         }
         Err(_) => {
@@ -601,14 +641,8 @@ impl Resolver for CliGraphResolver {
                 &PermissionsContainer::allow_all(),
               );
               match node_result {
-                Ok(Some(resolution)) => {
+                Ok(Some(res)) => {
-                  if let Some(cjs_resolutions) = &self.cjs_resolutions {
+                  return Ok(res.into_url());
-                    if let NodeResolution::CommonJs(specifier) = &resolution {
-                      // remember that this was a common js resolution
-                      cjs_resolutions.insert(specifier.clone());
-                    }
-                  }
-                  return Ok(resolution.into_url());
                 }
                 Ok(None) => {
                   self
@@ -639,7 +673,13 @@ impl Resolver for CliGraphResolver {
       }
     }
 
-    result
+    let specifier = result?;
+    match &self.node_resolver {
+      Some(node_resolver) => node_resolver
+        .handle_if_in_node_modules(specifier)
+        .map_err(|e| e.into()),
+      None => Ok(specifier),
+    }
   }
 }
 

cli/standalone/mod.rs

@@ -44,6 +44,7 @@ use deno_core::RequestedModuleType;
 use deno_core::ResolutionKind;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::analyze::NodeCodeTranslator;
+use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::NodeResolver;
 use deno_runtime::deno_tls::rustls::RootCertStore;
 use deno_runtime::deno_tls::RootCertStoreProvider;
@@ -111,9 +112,13 @@ impl ModuleLoader for EmbeddedModuleLoader {
     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 maybe_mapped = self
@@ -128,18 +133,26 @@ impl ModuleLoader for EmbeddedModuleLoader {
       .map(|r| r.as_str())
       .unwrap_or(specifier);
     if let Ok(reference) = NpmPackageReqReference::from_str(specifier_text) {
-      return self.shared.node_resolver.resolve_req_reference(
+      return self
-        &reference,
+        .shared
-        permissions,
+        .node_resolver
-        &referrer,
+        .resolve_req_reference(
-      );
+          &reference,
+          permissions,
+          &referrer,
+          NodeResolutionMode::Execution,
+        )
+        .map(|res| res.into_url());
     }
 
-    match maybe_mapped {
+    let specifier = match maybe_mapped {
-      Some(resolved) => Ok(resolved),
+      Some(resolved) => resolved,
-      None => deno_core::resolve_import(specifier, referrer.as_str())
+      None => deno_core::resolve_import(specifier, referrer.as_str())?,
-        .map_err(|err| err.into()),
+    };
-    }
+    self
+      .shared
+      .node_resolver
+      .handle_if_in_node_modules(specifier)
   }
 
   fn load(
@@ -452,7 +465,8 @@ pub async fn run(
     Arc::new(parse_from_json(&base, &source).unwrap().import_map)
   });
   let cli_node_resolver = Arc::new(CliNodeResolver::new(
-    cjs_resolutions.clone(),
+    Some(cjs_resolutions.clone()),
+    fs.clone(),
     node_resolver.clone(),
     npm_resolver.clone(),
   ));

cli/tools/vendor/test.rs

@@ -20,7 +20,6 @@ use deno_graph::source::Loader;
 use deno_graph::DefaultModuleAnalyzer;
 use deno_graph::GraphKind;
 use deno_graph::ModuleGraph;
-use deno_runtime::deno_fs::RealFs;
 use import_map::ImportMap;
 
 use crate::args::JsxImportSourceConfig;
@@ -295,10 +294,8 @@ fn build_resolver(
   original_import_map: Option<ImportMap>,
 ) -> CliGraphResolver {
   CliGraphResolver::new(CliGraphResolverOptions {
-    fs: Arc::new(RealFs),
     node_resolver: None,
     npm_resolver: None,
-    cjs_resolutions: None,
     sloppy_imports_resolver: None,
     package_json_deps_provider: Default::default(),
     maybe_jsx_import_source_config,

tests/integration/npm_tests.rs

@@ -2611,10 +2611,7 @@ fn cjs_rexport_analysis_json() {
   let dir = test_context.temp_dir();
   dir.write("deno.json", r#"{ "unstable": [ "byonm" ] }"#);
 
-  dir.write(
+  dir.write("package.json", r#"{ "name": "test" }"#);
-    "package.json",
-    r#"{ "name": "test", "packages": { "my-package": "1.0.0" } }"#,
-  );
   dir.write(
     "main.js",
     "import data from 'my-package';\nconsole.log(data);\n",
@@ -2670,6 +2667,28 @@ fn cjs_rexport_analysis_json() {
   );
 }
 
+#[test]
+fn cjs_export_analysis_import_cjs_directly_relative_import() {
+  let test_context = TestContextBuilder::for_npm().use_temp_cwd().build();
+  let dir = test_context.temp_dir();
+  dir.write("deno.json", r#"{ "unstable": [ "byonm" ] }"#);
+
+  dir.write(
+    "package.json",
+    r#"{ "name": "test", "dependencies": { "@denotest/cjs-default-export": "1.0.0" } }"#,
+  );
+  // previously it wasn't doing cjs export analysis on this file
+  dir.write(
+    "main.ts",
+    "import { named } from './node_modules/@denotest/cjs-default-export/index.js';\nconsole.log(named());\n",
+  );
+
+  test_context.run_npm("install");
+
+  let output = test_context.new_command().args("run main.ts").run();
+  output.assert_matches_text("2\n");
+}
+
 itest!(imports_package_json {
   args: "run --node-modules-dir=false npm/imports_package_json/main.js",
   output: "npm/imports_package_json/main.out",

tests/testdata/npm/imports_package_json/import_not_defined.out

@@ -1,6 +1,3 @@
 Download http://localhost:4545/npm/registry/@denotest/imports-package-json
 Download http://localhost:4545/npm/registry/@denotest/imports-package-json/1.0.0.tgz
-error: Could not resolve '#not-defined' from 'file:///[WILDCARD]/@denotest/imports-package-json/1.0.0/import_not_defined.js'.
+error: [ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier "#not-defined" is not defined in package [WILDCARD]package.json imported from [WILDCARD]import_not_defined.js
-
-Caused by:
-    [ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier "#not-defined" is not defined in package [WILDCARD]package.json imported from [WILDCARD]import_not_defined.js

tests/testdata/npm/imports_package_json/sub_path_import_not_defined.out

@@ -1,6 +1,3 @@
 Download http://localhost:4545/npm/registry/@denotest/imports-package-json
 Download http://localhost:4545/npm/registry/@denotest/imports-package-json/1.0.0.tgz
-error: Could not resolve '#hi' from 'file:///[WILDCARD]/@denotest/imports-package-json/1.0.0/sub_path/import_not_defined.js'.
+error: [ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier "#hi" is not defined in package [WILDCARD]sub_path[WILDCARD]package.json imported from [WILDCARD]import_not_defined.js
-
-Caused by:
-    [ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier "#hi" is not defined in package [WILDCARD]sub_path[WILDCARD]package.json imported from [WILDCARD]import_not_defined.js