feat: support bare specifier resolution with package.json (#17864)

This commit enables resolution of "bare specifiers" (eg. "import express
from 'express';") if a "package.json" file is discovered. 

It's a step towards being able to run projects authored for Node.js 
without any changes.

With this commit we are able to successfully run Vite projects without
any changes to the user code.

---------

Co-authored-by: David Sherret <dsherret@gmail.com>

cli/cache/mod.rs

@@ -3,6 +3,7 @@
 use crate::errors::get_error_class_name;
 use crate::file_fetcher::FileFetcher;
 
+use deno_core::futures;
 use deno_core::futures::FutureExt;
 use deno_core::ModuleSpecifier;
 use deno_graph::source::CacheInfo;
@@ -44,6 +45,7 @@ pub struct FetchCacher {
   file_fetcher: Arc<FileFetcher>,
   root_permissions: PermissionsContainer,
   cache_info_enabled: bool,
+  maybe_local_node_modules_url: Option<ModuleSpecifier>,
 }
 
 impl FetchCacher {
@@ -52,6 +54,7 @@ impl FetchCacher {
     file_fetcher: Arc<FileFetcher>,
     root_permissions: PermissionsContainer,
     dynamic_permissions: PermissionsContainer,
+    maybe_local_node_modules_url: Option<ModuleSpecifier>,
   ) -> Self {
     Self {
       emit_cache,
@@ -59,6 +62,7 @@ impl FetchCacher {
       file_fetcher,
       root_permissions,
       cache_info_enabled: false,
+      maybe_local_node_modules_url,
     }
   }
 
@@ -96,6 +100,16 @@ impl Loader for FetchCacher {
     specifier: &ModuleSpecifier,
     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()) {
+        return Box::pin(futures::future::ready(Ok(Some(
+          LoadResponse::External {
+            specifier: specifier.clone(),
+          },
+        ))));
+      }
+    }
+
     let permissions = if is_dynamic {
       self.dynamic_permissions.clone()
     } else {

cli/graph_util.rs

@@ -14,6 +14,7 @@ use crate::resolver::CliGraphResolver;
 use crate::tools::check;
 
 use deno_core::anyhow::bail;
+use deno_core::anyhow::Context;
 use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::ModuleSpecifier;
@@ -152,6 +153,10 @@ pub async fn create_graph_and_maybe_check(
     ps.file_fetcher.clone(),
     PermissionsContainer::allow_all(),
     PermissionsContainer::allow_all(),
+    ps.options
+      .resolve_local_node_modules_folder()
+      .with_context(|| "Resolving local node_modules folder.")?
+      .map(|path| ModuleSpecifier::from_file_path(path).unwrap()),
   );
   let maybe_imports = ps.options.to_maybe_imports()?;
   let maybe_package_json_deps = ps.options.maybe_package_json_deps()?;

cli/lsp/language_server.rs

@@ -168,7 +168,7 @@ impl LanguageServer {
         .map(|d| (d.specifier().clone(), d))
         .collect::<HashMap<_, _>>();
       let ps = ProcState::from_options(Arc::new(cli_options)).await?;
-      let mut inner_loader = ps.create_graph_loader();
+      let mut inner_loader = ps.create_graph_loader()?;
       let mut loader = crate::lsp::documents::OpenDocumentsGraphLoader {
         inner_loader: &mut inner_loader,
         open_docs: &open_docs,

cli/proc_state.rs

@@ -329,6 +329,11 @@ impl ProcState {
       self.file_fetcher.clone(),
       root_permissions,
       dynamic_permissions,
+      self
+        .options
+        .resolve_local_node_modules_folder()
+        .with_context(|| "Resolving local node_modules folder.")?
+        .map(|path| ModuleSpecifier::from_file_path(path).unwrap()),
     );
     let maybe_imports = self.options.to_maybe_imports()?;
     let graph_resolver = self.resolver.as_graph_resolver();
@@ -627,20 +632,25 @@ impl ProcState {
   }
 
   /// Creates the default loader used for creating a graph.
-  pub fn create_graph_loader(&self) -> cache::FetchCacher {
-    cache::FetchCacher::new(
+  pub fn create_graph_loader(&self) -> Result<cache::FetchCacher, AnyError> {
+    Ok(cache::FetchCacher::new(
       self.emit_cache.clone(),
       self.file_fetcher.clone(),
       PermissionsContainer::allow_all(),
       PermissionsContainer::allow_all(),
-    )
+      self
+        .options
+        .resolve_local_node_modules_folder()
+        .with_context(|| "Resolving local node_modules folder.")?
+        .map(|path| ModuleSpecifier::from_file_path(path).unwrap()),
+    ))
   }
 
   pub async fn create_graph(
     &self,
     roots: Vec<ModuleSpecifier>,
   ) -> Result<deno_graph::ModuleGraph, AnyError> {
-    let mut cache = self.create_graph_loader();
+    let mut cache = self.create_graph_loader()?;
     self.create_graph_with_loader(roots, &mut cache).await
   }
 

cli/resolver.rs

@@ -26,9 +26,6 @@ use crate::npm::NpmResolution;
 #[derive(Debug, Clone)]
 pub struct CliGraphResolver {
   maybe_import_map: Option<Arc<ImportMap>>,
-  // TODO(bartlomieju): actually use in `resolver`, once
-  // deno_graph refactors and upgrades land.
-  #[allow(dead_code)]
   maybe_package_json_deps: Option<HashMap<String, NpmPackageReq>>,
   maybe_default_jsx_import_source: Option<String>,
   maybe_jsx_import_source_module: Option<String>,
@@ -116,15 +113,19 @@ impl Resolver for CliGraphResolver {
     specifier: &str,
     referrer: &ModuleSpecifier,
   ) -> Result<ModuleSpecifier, AnyError> {
-    // TODO(bartlomieju): actually use `maybe_package_json_deps` here, once
-    // deno_graph refactors and upgrades land.
     if let Some(import_map) = &self.maybe_import_map {
-      import_map
+      return import_map
         .resolve(specifier, referrer)
-        .map_err(|err| err.into())
-    } else {
-      deno_graph::resolve_import(specifier, referrer).map_err(|err| err.into())
+        .map_err(|err| err.into());
     }
+
+    if let Some(deps) = self.maybe_package_json_deps.as_ref() {
+      if let Some(req) = deps.get(specifier) {
+        return Ok(ModuleSpecifier::parse(&format!("npm:{req}")).unwrap());
+      }
+    }
+
+    deno_graph::resolve_import(specifier, referrer).map_err(|err| err.into())
   }
 }
 

cli/tests/integration/run_tests.rs

@@ -2750,7 +2750,7 @@ itest!(config_not_auto_discovered_for_remote_script {
 });
 
 itest!(package_json_auto_discovered_for_local_script_log {
-  args: "run -L debug no_deno_json/main.ts",
+  args: "run -L debug -A no_deno_json/main.ts",
   output: "run/with_package_json/no_deno_json/main.out",
   maybe_cwd: Some("run/with_package_json/"),
   envs: env_vars_for_npm_tests_no_sync_download(),
@@ -2766,6 +2766,7 @@ itest!(
     maybe_cwd: Some("run/with_package_json/"),
     envs: env_vars_for_npm_tests_no_sync_download(),
     http_server: true,
+    exit_code: 1,
   }
 );
 

cli/tests/testdata/run/with_package_json/no_deno_json/main.out

@@ -1,3 +1,9 @@
 [WILDCARD]package.json file found at '[WILDCARD]with_package_json[WILDCARD]package.json'
 [WILDCARD]
 ok
+[Chalk] {
+  constructor: [Function],
+  Instance: [Class: ChalkClass],
+  supportsColor: false,
+  stderr: [Chalk] { constructor: [Function], Instance: [Class: ChalkClass], supportsColor: false }
+}

cli/tests/testdata/run/with_package_json/no_deno_json/main.ts

@@ -1,6 +1,4 @@
-// TODO(bartlomieju): currently we don't support actual bare specifier
-// imports; this will be done in a follow up PR.
-// import express from "express";
+import chalk from "chalk";
 
-// console.log(express);
 console.log("ok");
+console.log(chalk);

cli/tests/testdata/run/with_package_json/no_deno_json/package.json

@@ -1,6 +1,7 @@
 {
   "dependencies": {
-    "@denotest/check-error": "1.0.0"
+    "@denotest/check-error": "1.0.0",
+    "chalk": "4"
   },
   "devDependencies": {
     "@denotest/cjs-default-export": "1.0.0"

cli/tests/testdata/run/with_package_json/with_stop/main.out

@@ -1,4 +1,5 @@
 [WILDCARD]Config file found at '[WILDCARD]with_package_json[WILDCARD]with_stop[WILDCARD]some[WILDCARD]nested[WILDCARD]deno.json'
 [WILDCARD]No package.json file found
 [WILDCARD]
-ok
+error: Relative import path "chalk" not prefixed with / or ./ or ../
+    at file:///[WILDCARD]with_package_json/with_stop/some/nested/dir/main.ts:3:19

cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts

@@ -1,6 +1,6 @@
-// TODO(bartlomieju): currently we don't support actual bare specifier
-// imports; this will be done in a follow up PR.
-// import express from "express";
+// This import should fail, because `package.json` is not discovered, as we're
+// stopping the discovery when encountering `deno.json`.
+import chalk from "chalk";
 
-// console.log(express);
 console.log("ok");
+console.log(chalk);

cli/tools/info.rs

@@ -34,7 +34,7 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> {
   let ps = ProcState::build(flags).await?;
   if let Some(specifier) = info_flags.file {
     let specifier = resolve_url_or_path(&specifier)?;
-    let mut loader = ps.create_graph_loader();
+    let mut loader = ps.create_graph_loader()?;
     loader.enable_loading_cache_info(); // for displaying the cache information
     let graph = ps
       .create_graph_with_loader(vec![specifier], &mut loader)