feat(npm): support bare specifiers from package.json in more subcommands and language server (#17891)

cli/args/config_file.rs

@@ -2,7 +2,6 @@
 
 use crate::args::ConfigFlag;
 use crate::args::Flags;
-use crate::args::TaskFlags;
 use crate::util::fs::canonicalize_path;
 use crate::util::path::specifier_parent;
 use crate::util::path::specifier_to_file_path;
@@ -508,18 +507,6 @@ impl ConfigFile {
               return Ok(Some(cf));
             }
           }
-          // attempt to resolve the config file from the task subcommand's
-          // `--cwd` when specified
-          if let crate::args::DenoSubcommand::Task(TaskFlags {
-            cwd: Some(path),
-            ..
-          }) = &flags.subcommand
-          {
-            let task_cwd = canonicalize_path(&PathBuf::from(path))?;
-            if let Some(path) = Self::discover_from(&task_cwd, &mut checked)? {
-              return Ok(Some(path));
-            }
-          };
           // From CWD walk up to root looking for deno.json or deno.jsonc
           Self::discover_from(cwd, &mut checked)
         } else {

cli/args/flags.rs

@@ -19,6 +19,8 @@ use std::num::NonZeroUsize;
 use std::path::PathBuf;
 use std::str::FromStr;
 
+use crate::util::fs::canonicalize_path;
+
 use super::flags_allow_net;
 
 static LONG_VERSION: Lazy<String> = Lazy::new(|| {
@@ -499,6 +501,16 @@ impl Flags {
           Some(vec![])
         }
       }
+      Task(TaskFlags {
+        cwd: Some(path), ..
+      }) => {
+        // attempt to resolve the config file from the task subcommand's
+        // `--cwd` when specified
+        match canonicalize_path(&PathBuf::from(path)) {
+          Ok(path) => Some(vec![path]),
+          Err(_) => Some(vec![]),
+        }
+      }
       _ => Some(vec![]),
     }
   }
@@ -533,7 +545,8 @@ impl Flags {
           .to_file_path()
           .ok()
       }
-      Task(TaskFlags { cwd: None, .. }) => std::env::current_dir().ok(),
+      Task(_) | Check(_) | Coverage(_) | Cache(_) | Info(_) | Eval(_)
+      | Test(_) | Bench(_) => std::env::current_dir().ok(),
       _ => None,
     }
   }

cli/args/mod.rs

@@ -391,49 +391,13 @@ fn discover_package_json(
   flags: &Flags,
   maybe_stop_at: Option<PathBuf>,
 ) -> Result<Option<PackageJson>, AnyError> {
-  fn discover_from(
-    start: &Path,
-    maybe_stop_at: Option<PathBuf>,
-  ) -> Result<Option<PackageJson>, AnyError> {
-    const PACKAGE_JSON_NAME: &str = "package.json";
-
-    // note: ancestors() includes the `start` path
-    for ancestor in start.ancestors() {
-      let path = ancestor.join(PACKAGE_JSON_NAME);
-
-      let source = match std::fs::read_to_string(&path) {
-        Ok(source) => source,
-        Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
-          if let Some(stop_at) = maybe_stop_at.as_ref() {
-            if ancestor == stop_at {
-              break;
-            }
-          }
-          continue;
-        }
-        Err(err) => bail!(
-          "Error loading package.json at {}. {:#}",
-          path.display(),
-          err
-        ),
-      };
-
-      let package_json = PackageJson::load_from_string(path.clone(), source)?;
-      log::debug!("package.json file found at '{}'", path.display());
-      return Ok(Some(package_json));
-    }
-    // No config file found.
-    log::debug!("No package.json file found");
-    Ok(None)
-  }
-
   // TODO(bartlomieju): discover for all subcommands, but print warnings that
   // `package.json` is ignored in bundle/compile/etc.
 
   if let Some(package_json_dir) = flags.package_json_search_dir() {
     let package_json_dir =
       canonicalize_path_maybe_not_exists(&package_json_dir)?;
-    return discover_from(&package_json_dir, maybe_stop_at);
+    return package_json::discover_from(&package_json_dir, maybe_stop_at);
   }
 
   log::debug!("No package.json file found");

cli/args/package_json.rs

@@ -1,6 +1,8 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
 use std::collections::HashMap;
+use std::path::Path;
+use std::path::PathBuf;
 
 use deno_core::anyhow::anyhow;
 use deno_core::anyhow::bail;
@@ -83,6 +85,44 @@ pub fn get_local_package_json_version_reqs(
   Ok(result)
 }
 
+/// Attempts to discover the package.json file, maybe stopping when it
+/// reaches the specified `maybe_stop_at` directory.
+pub fn discover_from(
+  start: &Path,
+  maybe_stop_at: Option<PathBuf>,
+) -> Result<Option<PackageJson>, AnyError> {
+  const PACKAGE_JSON_NAME: &str = "package.json";
+
+  // note: ancestors() includes the `start` path
+  for ancestor in start.ancestors() {
+    let path = ancestor.join(PACKAGE_JSON_NAME);
+
+    let source = match std::fs::read_to_string(&path) {
+      Ok(source) => source,
+      Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
+        if let Some(stop_at) = maybe_stop_at.as_ref() {
+          if ancestor == stop_at {
+            break;
+          }
+        }
+        continue;
+      }
+      Err(err) => bail!(
+        "Error loading package.json at {}. {:#}",
+        path.display(),
+        err
+      ),
+    };
+
+    let package_json = PackageJson::load_from_string(path.clone(), source)?;
+    log::debug!("package.json file found at '{}'", path.display());
+    return Ok(Some(package_json));
+  }
+
+  log::debug!("No package.json file found");
+  Ok(None)
+}
+
 #[cfg(test)]
 mod test {
   use pretty_assertions::assert_eq;

cli/cache/mod.rs

@@ -2,6 +2,7 @@
 
 use crate::errors::get_error_class_name;
 use crate::file_fetcher::FileFetcher;
+use crate::util::fs::canonicalize_path;
 
 use deno_core::futures;
 use deno_core::futures::FutureExt;
@@ -101,12 +102,23 @@ impl Loader for FetchCacher {
     is_dynamic: bool,
   ) -> LoadFuture {
     if let Some(node_modules_url) = self.maybe_local_node_modules_url.as_ref() {
-      if specifier.as_str().starts_with(node_modules_url.as_str()) {
+      // The specifier might be in a completely different symlinked tree than
-        return Box::pin(futures::future::ready(Ok(Some(
+      // what the resolved node_modules_url is in (ex. `/my-project-1/node_modules`
-          LoadResponse::External {
+      // symlinked to `/my-project-2/node_modules`), so first check if the path
-            specifier: specifier.clone(),
+      // is in a node_modules dir to avoid needlessly canonicalizing, then compare
-          },
+      // against the canonicalized specifier.
-        ))));
+      if specifier.path().contains("/node_modules/") {
+        let specifier = specifier
+          .to_file_path()
+          .ok()
+          .and_then(|path| canonicalize_path(&path).ok())
+          .and_then(|path| ModuleSpecifier::from_file_path(path).ok())
+          .unwrap_or_else(|| specifier.clone());
+        if specifier.as_str().starts_with(node_modules_url.as_str()) {
+          return Box::pin(futures::future::ready(Ok(Some(
+            LoadResponse::External { specifier },
+          ))));
+        }
       }
     }
 

cli/graph_util.rs

@@ -156,6 +156,9 @@ pub async fn create_graph_and_maybe_check(
   );
   let maybe_imports = ps.options.to_maybe_imports()?;
   let maybe_package_json_deps = ps.options.maybe_package_json_deps()?;
+  ps.npm_resolver
+    .add_package_json_deps(maybe_package_json_deps.as_ref())
+    .await?;
   let cli_resolver = CliGraphResolver::new(
     ps.options.to_maybe_jsx_import_source_config(),
     ps.maybe_import_map.clone(),

cli/lsp/documents.rs

@@ -5,6 +5,7 @@ use super::text::LineIndex;
 use super::tsc;
 use super::tsc::AssetDocument;
 
+use crate::args::package_json;
 use crate::args::ConfigFile;
 use crate::args::JsxImportSourceConfig;
 use crate::cache::CachedUrlMetadata;
@@ -13,6 +14,7 @@ use crate::cache::HttpCache;
 use crate::file_fetcher::get_source_from_bytes;
 use crate::file_fetcher::map_content_type;
 use crate::file_fetcher::SUPPORTED_SCHEMES;
+use crate::lsp::logging::lsp_log;
 use crate::node;
 use crate::node::node_resolve_npm_reference;
 use crate::node::NodeResolution;
@@ -37,9 +39,11 @@ use deno_graph::npm::NpmPackageReqReference;
 use deno_graph::GraphImport;
 use deno_graph::Resolution;
 use deno_runtime::deno_node::NodeResolutionMode;
+use deno_runtime::deno_node::PackageJson;
 use deno_runtime::permissions::PermissionsContainer;
 use indexmap::IndexMap;
 use once_cell::sync::Lazy;
+use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::collections::VecDeque;
@@ -818,8 +822,10 @@ pub struct Documents {
   /// A resolver that takes into account currently loaded import map and JSX
   /// settings.
   resolver: CliGraphResolver,
-  /// The npm package requirements.
+  /// The npm package requirements found in a package.json file.
-  npm_reqs: Arc<HashSet<NpmPackageReq>>,
+  npm_package_json_reqs: Arc<Vec<NpmPackageReq>>,
+  /// The npm package requirements found in npm specifiers.
+  npm_specifier_reqs: Arc<Vec<NpmPackageReq>>,
   /// Gets if any document had a node: specifier such that a @types/node package
   /// should be injected.
   has_injected_types_node_package: bool,
@@ -838,7 +844,8 @@ impl Documents {
       resolver_config_hash: 0,
       imports: Default::default(),
       resolver: CliGraphResolver::default(),
-      npm_reqs: Default::default(),
+      npm_package_json_reqs: Default::default(),
+      npm_specifier_reqs: Default::default(),
       has_injected_types_node_package: false,
       specifier_resolver: Arc::new(SpecifierResolver::new(location)),
     }
@@ -970,9 +977,15 @@ impl Documents {
   }
 
   /// Returns a collection of npm package requirements.
-  pub fn npm_package_reqs(&mut self) -> HashSet<NpmPackageReq> {
+  pub fn npm_package_reqs(&mut self) -> Vec<NpmPackageReq> {
     self.calculate_dependents_if_dirty();
-    (*self.npm_reqs).clone()
+    let mut reqs = Vec::with_capacity(
+      self.npm_package_json_reqs.len() + self.npm_specifier_reqs.len(),
+    );
+    // resolve the package.json reqs first, then the npm specifiers
+    reqs.extend(self.npm_package_json_reqs.iter().cloned());
+    reqs.extend(self.npm_specifier_reqs.iter().cloned());
+    reqs
   }
 
   /// Returns if a @types/node package was injected into the npm
@@ -1150,12 +1163,14 @@ impl Documents {
     &mut self,
     maybe_import_map: Option<Arc<import_map::ImportMap>>,
     maybe_config_file: Option<&ConfigFile>,
+    maybe_package_json: Option<&PackageJson>,
     npm_registry_api: NpmRegistryApi,
     npm_resolution: NpmResolution,
   ) {
     fn calculate_resolver_config_hash(
       maybe_import_map: Option<&import_map::ImportMap>,
       maybe_jsx_config: Option<&JsxImportSourceConfig>,
+      maybe_package_json_deps: Option<&HashMap<String, NpmPackageReq>>,
     ) -> u64 {
       let mut hasher = FastInsecureHasher::default();
       if let Some(import_map) = maybe_import_map {
@@ -1165,23 +1180,46 @@ impl Documents {
       if let Some(jsx_config) = maybe_jsx_config {
         hasher.write_hashable(&jsx_config);
       }
+      if let Some(deps) = maybe_package_json_deps {
+        let deps = deps.iter().collect::<BTreeMap<_, _>>();
+        hasher.write_hashable(&deps);
+      }
       hasher.finish()
     }
 
+    let maybe_package_json_deps = maybe_package_json.and_then(|package_json| {
+      match package_json::get_local_package_json_version_reqs(package_json) {
+        Ok(deps) => Some(deps),
+        Err(err) => {
+          lsp_log!("Error parsing package.json deps: {err:#}");
+          None
+        }
+      }
+    });
     let maybe_jsx_config =
       maybe_config_file.and_then(|cf| cf.to_maybe_jsx_import_source_config());
     let new_resolver_config_hash = calculate_resolver_config_hash(
       maybe_import_map.as_deref(),
       maybe_jsx_config.as_ref(),
+      maybe_package_json_deps.as_ref(),
     );
-    // TODO(bartlomieju): handle package.json dependencies here
+    self.npm_package_json_reqs = Arc::new({
+      match &maybe_package_json_deps {
+        Some(deps) => {
+          let mut reqs = deps.values().cloned().collect::<Vec<_>>();
+          reqs.sort();
+          reqs
+        }
+        None => Vec::new(),
+      }
+    });
     self.resolver = CliGraphResolver::new(
       maybe_jsx_config,
       maybe_import_map,
       false,
       npm_registry_api,
       npm_resolution,
-      None,
+      maybe_package_json_deps,
     );
     self.imports = Arc::new(
       if let Some(Ok(imports)) =
@@ -1306,7 +1344,11 @@ impl Documents {
     }
 
     self.dependents_map = Arc::new(doc_analyzer.dependents_map);
-    self.npm_reqs = Arc::new(npm_reqs);
+    self.npm_specifier_reqs = Arc::new({
+      let mut reqs = npm_reqs.into_iter().collect::<Vec<_>>();
+      reqs.sort();
+      reqs
+    });
     self.dirty = false;
     file_system_docs.dirty = false;
   }
@@ -1589,6 +1631,7 @@ console.log(b, "hello deno");
       documents.update_config(
         Some(Arc::new(import_map)),
         None,
+        None,
         npm_registry_api.clone(),
         npm_resolution.clone(),
       );
@@ -1627,6 +1670,7 @@ console.log(b, "hello deno");
       documents.update_config(
         Some(Arc::new(import_map)),
         None,
+        None,
         npm_registry_api,
         npm_resolution,
       );

cli/lsp/language_server.rs

@@ -9,12 +9,14 @@ use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::serde_json::Value;
 use deno_core::ModuleSpecifier;
+use deno_runtime::deno_node::PackageJson;
 use deno_runtime::deno_web::BlobStore;
 use import_map::ImportMap;
 use log::error;
 use log::warn;
 use serde_json::from_value;
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::env;
 use std::fmt::Write as _;
 use std::path::PathBuf;
@@ -58,6 +60,7 @@ use super::tsc::AssetsSnapshot;
 use super::tsc::TsServer;
 use super::urls;
 use crate::args::get_root_cert_store;
+use crate::args::package_json;
 use crate::args::resolve_import_map_from_specifier;
 use crate::args::CaData;
 use crate::args::CacheSetting;
@@ -130,6 +133,8 @@ pub struct Inner {
   maybe_import_map: Option<Arc<ImportMap>>,
   /// The URL for the import map which is used to determine relative imports.
   maybe_import_map_uri: Option<Url>,
+  /// An optional package.json configuration file.
+  maybe_package_json: Option<PackageJson>,
   /// Configuration for formatter which has been taken from specified config file.
   fmt_options: FmtOptions,
   /// An optional configuration for linter which has been taken from specified config file.
@@ -376,6 +381,7 @@ impl Inner {
       maybe_config_file: None,
       maybe_import_map: None,
       maybe_import_map_uri: None,
+      maybe_package_json: None,
       fmt_options: Default::default(),
       lint_options: Default::default(),
       maybe_testing_server: None,
@@ -456,8 +462,6 @@ impl Inner {
     Ok(navigation_tree)
   }
 
-  /// Returns a tuple with parsed `ConfigFile` and `Url` pointing to that file.
-  /// If there's no config file specified in settings returns `None`.
   fn get_config_file(&self) -> Result<Option<ConfigFile>, AnyError> {
     let workspace_settings = self.config.get_workspace_settings();
     let maybe_config = workspace_settings.config;
@@ -501,6 +505,28 @@ impl Inner {
     }
   }
 
+  fn get_package_json(
+    &self,
+    maybe_config_file: Option<&ConfigFile>,
+  ) -> Result<Option<PackageJson>, AnyError> {
+    // It is possible that root_uri is not set, for example when having a single
+    // file open and not a workspace.  In those situations we can't
+    // automatically discover the configuration
+    if let Some(root_uri) = &self.config.root_uri {
+      let root_path = specifier_to_file_path(root_uri)?;
+      let maybe_package_json = package_json::discover_from(
+        &root_path,
+        maybe_config_file.and_then(|f| f.specifier.to_file_path().ok()),
+      )?;
+      Ok(maybe_package_json.map(|c| {
+        lsp_log!("  Auto-resolved package.json: \"{}\"", c.specifier());
+        c
+      }))
+    } else {
+      Ok(None)
+    }
+  }
+
   fn is_diagnosable(&self, specifier: &ModuleSpecifier) -> bool {
     if specifier.scheme() == "asset" {
       matches!(
@@ -785,6 +811,7 @@ impl Inner {
 
   fn update_config_file(&mut self) -> Result<(), AnyError> {
     self.maybe_config_file = None;
+    self.maybe_package_json = None;
     self.fmt_options = Default::default();
     self.lint_options = Default::default();
 
@@ -814,6 +841,15 @@ impl Inner {
     Ok(())
   }
 
+  /// Updates the package.json. Always ensure this is done after updating
+  /// the configuration file as the resolution of this depends on that.
+  fn update_package_json(&mut self) -> Result<(), AnyError> {
+    self.maybe_package_json = None;
+    self.maybe_package_json =
+      self.get_package_json(self.maybe_config_file.as_ref())?;
+    Ok(())
+  }
+
   async fn update_tsconfig(&mut self) -> Result<(), AnyError> {
     let mark = self.performance.mark("update_tsconfig", None::<()>);
     let mut tsconfig = TsConfig::new(json!({
@@ -923,6 +959,9 @@ impl Inner {
     if let Err(err) = self.update_config_file() {
       self.client.show_message(MessageType::WARNING, err).await;
     }
+    if let Err(err) = self.update_package_json() {
+      self.client.show_message(MessageType::WARNING, err).await;
+    }
     if let Err(err) = self.update_tsconfig().await {
       self.client.show_message(MessageType::WARNING, err).await;
     }
@@ -950,6 +989,7 @@ impl Inner {
     self.documents.update_config(
       self.maybe_import_map.clone(),
       self.maybe_config_file.as_ref(),
+      self.maybe_package_json.as_ref(),
       self.npm_resolver.api().clone(),
       self.npm_resolver.resolution().clone(),
     );
@@ -1129,6 +1169,9 @@ impl Inner {
     if let Err(err) = self.update_config_file() {
       self.client.show_message(MessageType::WARNING, err).await;
     }
+    if let Err(err) = self.update_package_json() {
+      self.client.show_message(MessageType::WARNING, err).await;
+    }
     if let Err(err) = self.update_import_map().await {
       self.client.show_message(MessageType::WARNING, err).await;
     }
@@ -1139,6 +1182,7 @@ impl Inner {
     self.documents.update_config(
       self.maybe_import_map.clone(),
       self.maybe_config_file.as_ref(),
+      self.maybe_package_json.as_ref(),
       self.npm_resolver.api().clone(),
       self.npm_resolver.resolution().clone(),
     );
@@ -1155,7 +1199,7 @@ impl Inner {
       .performance
       .mark("did_change_watched_files", Some(&params));
     let mut touched = false;
-    let changes: Vec<Url> = params
+    let changes: HashSet<Url> = params
       .changes
       .iter()
       .map(|f| self.url_map.normalize_url(&f.uri))
@@ -1163,7 +1207,7 @@ impl Inner {
 
     // if the current tsconfig has changed, we need to reload it
     if let Some(config_file) = &self.maybe_config_file {
-      if changes.iter().any(|uri| config_file.specifier == *uri) {
+      if changes.contains(&config_file.specifier) {
         if let Err(err) = self.update_config_file() {
           self.client.show_message(MessageType::WARNING, err).await;
         }
@@ -1173,10 +1217,18 @@ impl Inner {
         touched = true;
       }
     }
+    if let Some(package_json) = &self.maybe_package_json {
+      if changes.contains(&package_json.specifier()) {
+        if let Err(err) = self.update_package_json() {
+          self.client.show_message(MessageType::WARNING, err).await;
+        }
+        touched = true;
+      }
+    }
     // if the current import map, or config file has changed, we need to reload
     // reload the import map
     if let Some(import_map_uri) = &self.maybe_import_map_uri {
-      if changes.iter().any(|uri| import_map_uri == uri) || touched {
+      if changes.contains(import_map_uri) || touched {
         if let Err(err) = self.update_import_map().await {
           self.client.show_message(MessageType::WARNING, err).await;
         }
@@ -1187,6 +1239,7 @@ impl Inner {
       self.documents.update_config(
         self.maybe_import_map.clone(),
         self.maybe_config_file.as_ref(),
+        self.maybe_package_json.as_ref(),
         self.npm_resolver.api().clone(),
         self.npm_resolver.resolution().clone(),
       );

cli/npm/cache.rs

@@ -179,6 +179,10 @@ impl ReadonlyNpmCache {
     Self::new(dir.npm_folder_path())
   }
 
+  pub fn root_dir_url(&self) -> &Url {
+    &self.root_dir_url
+  }
+
   pub fn package_folder_for_id(
     &self,
     folder_id: &NpmPackageCacheFolderId,
@@ -345,6 +349,10 @@ impl NpmCache {
     &self.cache_setting
   }
 
+  pub fn root_dir_url(&self) -> &Url {
+    self.readonly.root_dir_url()
+  }
+
   /// Checks if the cache should be used for the provided name and version.
   /// NOTE: Subsequent calls for the same package will always return `true`
   /// to ensure a package is only downloaded once per run of the CLI. This

cli/npm/resolution/mod.rs

@@ -292,17 +292,18 @@ impl NpmResolution {
 
   pub async fn set_package_reqs(
     &self,
-    package_reqs: HashSet<NpmPackageReq>,
+    package_reqs: Vec<NpmPackageReq>,
   ) -> Result<(), AnyError> {
     let inner = &self.0;
     // only allow one thread in here at a time
     let _permit = inner.update_semaphore.acquire().await?;
     let snapshot = inner.snapshot.read().clone();
 
+    let reqs_set = package_reqs.iter().collect::<HashSet<_>>();
     let has_removed_package = !snapshot
       .package_reqs
       .keys()
-      .all(|req| package_reqs.contains(req));
+      .all(|req| reqs_set.contains(req));
     // if any packages were removed, we need to completely recreate the npm resolution snapshot
     let snapshot = if has_removed_package {
       NpmResolutionSnapshot::default()
@@ -311,7 +312,7 @@ impl NpmResolution {
     };
     let snapshot = add_package_reqs_to_snapshot(
       &inner.api,
-      package_reqs.into_iter().collect(),
+      package_reqs,
       snapshot,
       self.0.maybe_lockfile.clone(),
     )

cli/npm/resolvers/common.rs

@@ -20,6 +20,9 @@ use crate::npm::NpmResolutionPackage;
 /// Part of the resolution that interacts with the file system.
 #[async_trait]
 pub trait NpmPackageFsResolver: Send + Sync {
+  /// Specifier for the root directory.
+  fn root_dir_url(&self) -> &Url;
+
   fn resolve_package_folder_from_deno_module(
     &self,
     id: &NpmPackageId,

cli/npm/resolvers/global.rs

@@ -68,6 +68,10 @@ impl GlobalNpmPackageResolver {
 
 #[async_trait]
 impl NpmPackageFsResolver for GlobalNpmPackageResolver {
+  fn root_dir_url(&self) -> &Url {
+    self.cache.root_dir_url()
+  }
+
   fn resolve_package_folder_from_deno_module(
     &self,
     id: &NpmPackageId,

cli/npm/resolvers/local.rs

@@ -44,7 +44,7 @@ pub struct LocalNpmPackageResolver {
   resolution: NpmResolution,
   registry_url: Url,
   root_node_modules_path: PathBuf,
-  root_node_modules_specifier: ModuleSpecifier,
+  root_node_modules_url: Url,
 }
 
 impl LocalNpmPackageResolver {
@@ -58,10 +58,8 @@ impl LocalNpmPackageResolver {
       cache,
       resolution,
       registry_url,
-      root_node_modules_specifier: ModuleSpecifier::from_directory_path(
+      root_node_modules_url: Url::from_directory_path(&node_modules_folder)
-        &node_modules_folder,
+        .unwrap(),
-      )
-      .unwrap(),
       root_node_modules_path: node_modules_folder,
     }
   }
@@ -92,8 +90,7 @@ impl LocalNpmPackageResolver {
     &self,
     specifier: &ModuleSpecifier,
   ) -> Option<PathBuf> {
-    let relative_url =
+    let relative_url = self.root_node_modules_url.make_relative(specifier)?;
-      self.root_node_modules_specifier.make_relative(specifier)?;
     if relative_url.starts_with("../") {
       return None;
     }
@@ -126,6 +123,10 @@ impl LocalNpmPackageResolver {
 
 #[async_trait]
 impl NpmPackageFsResolver for LocalNpmPackageResolver {
+  fn root_dir_url(&self) -> &Url {
+    &self.root_node_modules_url
+  }
+
   fn resolve_package_folder_from_deno_module(
     &self,
     node_id: &NpmPackageId,

cli/npm/resolvers/mod.rs

@@ -19,7 +19,7 @@ use deno_runtime::deno_node::RequireNpmResolver;
 use global::GlobalNpmPackageResolver;
 use serde::Deserialize;
 use serde::Serialize;
-use std::collections::HashSet;
+use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
@@ -214,9 +214,9 @@ impl NpmPackageResolver {
 
   /// Gets if the provided specifier is in an npm package.
   pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool {
-    self
+    let root_dir_url = self.fs_resolver.root_dir_url();
-      .resolve_package_folder_from_specifier(specifier)
+    debug_assert!(root_dir_url.as_str().ends_with('/'));
-      .is_ok()
+    specifier.as_ref().starts_with(root_dir_url.as_str())
   }
 
   /// If the resolver has resolved any npm packages.
@@ -224,6 +224,19 @@ impl NpmPackageResolver {
     self.resolution.has_packages()
   }
 
+  /// Adds the package reqs from a package.json if they exist.
+  pub async fn add_package_json_deps(
+    &self,
+    maybe_package_json_deps: Option<&HashMap<String, NpmPackageReq>>,
+  ) -> Result<(), AnyError> {
+    if let Some(deps) = maybe_package_json_deps {
+      let mut package_reqs = deps.values().cloned().collect::<Vec<_>>();
+      package_reqs.sort(); // deterministic resolution
+      self.add_package_reqs(package_reqs).await?;
+    }
+    Ok(())
+  }
+
   /// Adds package requirements to the resolver and ensures everything is setup.
   pub async fn add_package_reqs(
     &self,
@@ -250,7 +263,7 @@ impl NpmPackageResolver {
   /// This will retrieve and resolve package information, but not cache any package files.
   pub async fn set_package_reqs(
     &self,
-    packages: HashSet<NpmPackageReq>,
+    packages: Vec<NpmPackageReq>,
   ) -> Result<(), AnyError> {
     self.resolution.set_package_reqs(packages).await
   }

cli/proc_state.rs

@@ -237,12 +237,10 @@ impl ProcState {
       cli_options.resolve_inspector_server().map(Arc::new);
 
     let maybe_package_json_deps = cli_options.maybe_package_json_deps()?;
-    if let Some(deps) = &maybe_package_json_deps {
+    // resolve the package.json npm requirements ahead of time
-      // resolve the package.json npm requirements ahead of time
+    npm_resolver
-      let mut package_reqs = deps.values().cloned().collect::<Vec<_>>();
+      .add_package_json_deps(maybe_package_json_deps.as_ref())
-      package_reqs.sort(); // deterministic resolution
+      .await?;
-      npm_resolver.add_package_reqs(package_reqs).await?;
-    }
     let resolver = Arc::new(CliGraphResolver::new(
       cli_options.to_maybe_jsx_import_source_config(),
       maybe_import_map.clone(),
@@ -639,14 +637,18 @@ impl ProcState {
   ) -> Result<deno_graph::ModuleGraph, AnyError> {
     let maybe_imports = self.options.to_maybe_imports()?;
 
+    let maybe_package_json_deps = self.options.maybe_package_json_deps()?;
+    self
+      .npm_resolver
+      .add_package_json_deps(maybe_package_json_deps.as_ref())
+      .await?;
     let cli_resolver = CliGraphResolver::new(
       self.options.to_maybe_jsx_import_source_config(),
       self.maybe_import_map.clone(),
       self.options.no_npm(),
       self.npm_resolver.api().clone(),
       self.npm_resolver.resolution().clone(),
-      // TODO(bartlomieju): this should use dependencies from `package.json`?
+      maybe_package_json_deps,
-      None,
     );
     let graph_resolver = cli_resolver.as_graph_resolver();
     let graph_npm_resolver = cli_resolver.as_graph_npm_resolver();

cli/tests/integration/bench_tests.rs

@@ -2,6 +2,7 @@
 
 use deno_core::url::Url;
 use test_util as util;
+use util::env_vars_for_npm_tests;
 
 itest!(overloads {
   args: "bench bench/overloads.ts",
@@ -216,3 +217,13 @@ fn file_protocol() {
   })
   .run();
 }
+
+itest!(package_json_basic {
+  args: "bench",
+  output: "package_json/basic/main.bench.out",
+  envs: env_vars_for_npm_tests(),
+  http_server: true,
+  cwd: Some("package_json/basic"),
+  copy_temp_dir: Some("package_json/basic"),
+  exit_code: 0,
+});

cli/tests/integration/cache_tests.rs

@@ -1,5 +1,7 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
+use test_util::env_vars_for_npm_tests;
+
 itest!(_036_import_map_fetch {
   args:
     "cache --quiet --reload --import-map=import_maps/import_map.json import_maps/test.ts",
@@ -95,3 +97,13 @@ itest!(json_import {
   // should not error
   args: "cache --quiet cache/json_import/main.ts",
 });
+
+itest!(package_json_basic {
+  args: "cache main.ts",
+  output: "package_json/basic/main.cache.out",
+  envs: env_vars_for_npm_tests(),
+  http_server: true,
+  cwd: Some("package_json/basic"),
+  copy_temp_dir: Some("package_json/basic"),
+  exit_code: 0,
+});

cli/tests/integration/check_tests.rs

@@ -3,6 +3,8 @@
 use std::process::Command;
 use std::process::Stdio;
 use test_util as util;
+use util::env_vars_for_npm_tests;
+use util::env_vars_for_npm_tests_no_sync_download;
 use util::TempDir;
 
 itest!(_095_check_with_bare_import {
@@ -229,3 +231,23 @@ fn ts_no_recheck_on_redirect() {
 
   assert!(std::str::from_utf8(&output.stderr).unwrap().is_empty());
 }
+
+itest!(package_json_basic {
+  args: "check main.ts",
+  output: "package_json/basic/main.check.out",
+  envs: env_vars_for_npm_tests(),
+  http_server: true,
+  cwd: Some("package_json/basic"),
+  copy_temp_dir: Some("package_json/basic"),
+  exit_code: 0,
+});
+
+itest!(package_json_fail_check {
+  args: "check --quiet fail_check.ts",
+  output: "package_json/basic/fail_check.check.out",
+  envs: env_vars_for_npm_tests_no_sync_download(),
+  http_server: true,
+  cwd: Some("package_json/basic"),
+  copy_temp_dir: Some("package_json/basic"),
+  exit_code: 1,
+});

cli/tests/integration/info_tests.rs

@@ -2,6 +2,7 @@
 
 use test_util as util;
 use test_util::TempDir;
+use util::env_vars_for_npm_tests_no_sync_download;
 
 #[test]
 fn info_with_compiled_source() {
@@ -127,3 +128,13 @@ itest!(with_config_override {
   args: "info info/with_config/test.ts --config info/with_config/deno-override.json --import-map info/with_config/import_map.json",
   output: "info/with_config/with_config.out",
 });
+
+itest!(package_json_basic {
+  args: "info --quiet main.ts",
+  output: "package_json/basic/main.info.out",
+  envs: env_vars_for_npm_tests_no_sync_download(),
+  http_server: true,
+  cwd: Some("package_json/basic"),
+  copy_temp_dir: Some("package_json/basic"),
+  exit_code: 0,
+});

cli/tests/integration/npm_tests.rs

@@ -1565,3 +1565,23 @@ itest!(create_require {
   envs: env_vars_for_npm_tests(),
   http_server: true,
 });
+
+itest!(node_modules_import_run {
+  args: "run --quiet main.ts",
+  output: "npm/node_modules_import/main.out",
+  envs: env_vars_for_npm_tests(),
+  http_server: true,
+  cwd: Some("npm/node_modules_import/"),
+  copy_temp_dir: Some("npm/node_modules_import/"),
+  exit_code: 0,
+});
+
+itest!(node_modules_import_check {
+  args: "check --quiet main.ts",
+  output: "npm/node_modules_import/main_check.out",
+  envs: env_vars_for_npm_tests(),
+  http_server: true,
+  cwd: Some("npm/node_modules_import/"),
+  copy_temp_dir: Some("npm/node_modules_import/"),
+  exit_code: 1,
+});

cli/tests/integration/test_tests.rs

@@ -2,6 +2,7 @@
 
 use deno_core::url::Url;
 use test_util as util;
+use util::env_vars_for_npm_tests;
 
 #[test]
 fn no_color() {
@@ -452,3 +453,13 @@ itest!(parallel_output {
   output: "test/parallel_output.out",
   exit_code: 1,
 });
+
+itest!(package_json_basic {
+  args: "test",
+  output: "package_json/basic/main.test.out",
+  envs: env_vars_for_npm_tests(),
+  http_server: true,
+  cwd: Some("package_json/basic"),
+  copy_temp_dir: Some("package_json/basic"),
+  exit_code: 0,
+});

cli/tests/testdata/npm/node_modules_import/main.out

@@ -0,0 +1,2 @@
+2
+2

cli/tests/testdata/npm/node_modules_import/main.ts

@@ -0,0 +1,13 @@
+import * as myImport1 from "@denotest/esm-basic";
+import * as myImport2 from "./node_modules/@denotest/esm-basic/main.mjs";
+
+myImport1.setValue(5);
+myImport2.setValue(2);
+
+// these should both give type errors
+const value1: string = myImport1.getValue();
+const value2: string = myImport2.getValue();
+
+// these should both be equal because it should be mutating the same module
+console.log(value1);
+console.log(value2);

cli/tests/testdata/npm/node_modules_import/main_check.out

@@ -0,0 +1,11 @@
+error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
+const value1: string = myImport1.getValue();
+      ~~~~~~
+    at file:///[WILDCARD]/npm/node_modules_import/main.ts:8:7
+
+TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
+const value2: string = myImport2.getValue();
+      ~~~~~~
+    at file:///[WILDCARD]/npm/node_modules_import/main.ts:9:7
+
+Found 2 errors.

cli/tests/testdata/npm/node_modules_import/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@denotest/esm-basic": "^1"
+  }
+}

cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.d.mts

@@ -0,0 +1,2 @@
+export declare function setValue(val: number): void;
+export declare function getValue(): number;

cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.mjs

@@ -0,0 +1,9 @@
+let value = 0;
+
+export function setValue(newValue) {
+  value = newValue;
+}
+
+export function getValue() {
+  return value;
+}

cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "@denotest/esm-basic",
+  "version": "1.0.0",
+  "type": "module",
+  "main": "main.mjs",
+  "types": "main.d.mts"
+}

cli/tests/testdata/package_json/basic/fail_check.check.out

@@ -0,0 +1,4 @@
+error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
+const _test: string = getValue();
+      ~~~~~
+    at file:///[WILDCARD]/fail_check.ts:3:7

cli/tests/testdata/package_json/basic/fail_check.ts

@@ -0,0 +1,3 @@
+import { getValue } from "./main.ts";
+
+const _test: string = getValue();

cli/tests/testdata/package_json/basic/main.bench.out

@@ -0,0 +1,11 @@
+Download http://localhost:4545/npm/registry/@denotest/esm-basic
+Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz
+Check file:///[WILDCARD]/main.bench.ts
+0
+cpu: [WILDCARD]
+runtime: [WILDCARD]
+
+file:///[WILDCARD]/main.bench.ts
+[WILDCARD]
+-------------------------------------------------- -----------------------------
+should add  [WILDCARD]

cli/tests/testdata/package_json/basic/main.bench.ts

@@ -0,0 +1,7 @@
+import { add } from "./main.ts";
+
+Deno.bench("should add", () => {
+  if (add(1, 2) !== 3) {
+    throw new Error("Fail");
+  }
+});

cli/tests/testdata/package_json/basic/main.cache.out

@@ -0,0 +1,2 @@
+Download http://localhost:4545/npm/registry/@denotest/esm-basic
+Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz

cli/tests/testdata/package_json/basic/main.check.out

@@ -0,0 +1,3 @@
+Download http://localhost:4545/npm/registry/@denotest/esm-basic
+Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz
+Check file://[WILDCARD]/main.ts

cli/tests/testdata/package_json/basic/main.info.out

@@ -0,0 +1,7 @@
+local: [WILDCARD]main.ts
+type: TypeScript
+dependencies: 1 unique
+size: [WILDCARD]
+
+file://[WILDCARD]/package_json/basic/main.ts ([WILDCARD])
+└── npm:@denotest/esm-basic@1.0.0 ([WILDCARD])

cli/tests/testdata/package_json/basic/main.test.out

@@ -0,0 +1,9 @@
+Download http://localhost:4545/npm/registry/@denotest/esm-basic
+Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz
+Check file://[WILDCARD]/main.test.ts
+0
+running 1 test from [WILDCARD]main.test.ts
+should add ... ok ([WILDCARD])
+
+ok | 1 passed | 0 failed ([WILDCARD])
+

cli/tests/testdata/package_json/basic/main.test.ts

@@ -0,0 +1,7 @@
+import { add } from "./main.ts";
+
+Deno.test("should add", () => {
+  if (add(1, 2) !== 3) {
+    throw new Error("Fail");
+  }
+});

cli/tests/testdata/package_json/basic/main.ts

@@ -0,0 +1,11 @@
+import * as test from "@denotest/esm-basic";
+
+console.log(test.getValue());
+
+export function add(a: number, b: number) {
+  return a + b;
+}
+
+export function getValue() {
+  return test.getValue();
+}

cli/tests/testdata/package_json/basic/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@denotest/esm-basic": "*"
+  }
+}

cli/tsc/mod.rs

@@ -556,7 +556,17 @@ fn op_load(state: &mut OpState, args: Value) -> Result<Value, AnyError> {
           media_type = MediaType::Json;
           Some(Cow::Borrowed(&*module.source))
         }
-        Module::External(_) | Module::Npm(_) | Module::Node(_) => None,
+        Module::Npm(_) | Module::Node(_) => None,
+        Module::External(module) => {
+          // means it's Deno code importing an npm module
+          media_type = MediaType::from(&module.specifier);
+          let file_path = specifier.to_file_path().unwrap();
+          let code =
+            std::fs::read_to_string(&file_path).with_context(|| {
+              format!("Unable to load {}", file_path.display())
+            })?;
+          Some(Cow::Owned(code))
+        }
       }
     } else if state
       .maybe_npm_resolver
@@ -718,7 +728,16 @@ fn resolve_graph_specifier_types(
         Ok(None)
       }
     }
-    Some(Module::External(_) | Module::Node(_)) | None => Ok(None),
+    Some(Module::External(module)) => {
+      // we currently only use "External" for when the module is in an npm package
+      Ok(state.maybe_npm_resolver.as_ref().map(|npm_resolver| {
+        NodeResolution::into_specifier_and_media_type(
+          node::url_to_node_resolution(module.specifier.clone(), npm_resolver)
+            .ok(),
+        )
+      }))
+    }
+    Some(Module::Node(_)) | None => Ok(None),
   }
 }
 

ext/node/package_json.rs

@@ -11,6 +11,7 @@ use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::serde_json::Map;
 use deno_core::serde_json::Value;
+use deno_core::ModuleSpecifier;
 use indexmap::IndexMap;
 use serde::Serialize;
 use std::cell::RefCell;
@@ -205,6 +206,10 @@ impl PackageJson {
       self.main.as_ref()
     }
   }
+
+  pub fn specifier(&self) -> ModuleSpecifier {
+    ModuleSpecifier::from_file_path(&self.path).unwrap()
+  }
 }
 
 fn is_conditional_exports_main_sugar(exports: &Value) -> bool {