feat(byonm): support deno run npm:<package> when package is not in …

…package.json (#25981) Closes #25905
denoland · Oct 2, 2024 · cac28b5 · cac28b5
1 parent bbd4ae1
commit cac28b5
Show file tree

Hide file tree

Showing 20 changed files with 178 additions and 66 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -580,6 +580,15 @@ pub struct UnstableConfig {
   pub features: Vec<String>, // --unstabe-kv --unstable-cron
 }
 
+#[derive(Clone, Debug, Eq, PartialEq, Default)]
+pub struct InternalFlags {
+  /// Used when the language server is configured with an
+  /// explicit cache option.
+  pub cache_path: Option<PathBuf>,
+  /// Only reads to the lockfile instead of writing to it.
+  pub lockfile_skip_write: bool,
+}
+
 #[derive(Clone, Debug, Eq, PartialEq, Default)]
 pub struct Flags {
   /// Vector of CLI arguments - these are user script arguments, all Deno
@@ -591,9 +600,6 @@ pub struct Flags {
   pub ca_stores: Option<Vec<String>>,
   pub ca_data: Option<CaData>,
   pub cache_blocklist: Vec<String>,
-  /// This is not exposed as an option in the CLI, it is used internally when
-  /// the language server is configured with an explicit cache option.
-  pub cache_path: Option<PathBuf>,
   pub cached_only: bool,
   pub type_check_mode: TypeCheckMode,
   pub config_flag: ConfigFlag,
@@ -602,6 +608,8 @@ pub struct Flags {
   pub enable_op_summary_metrics: bool,
   pub enable_testing_features: bool,
   pub ext: Option<String>,
+  /// Flags that aren't exposed in the CLI, but are used internally.
+  pub internal: InternalFlags,
   pub ignore: Vec<String>,
   pub import_map_path: Option<String>,
   pub env_file: Option<String>,

diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs
@@ -24,11 +24,20 @@ use crate::args::InstallKind;
 
 use deno_lockfile::Lockfile;
 
+#[derive(Debug)]
+pub struct CliLockfileReadFromPathOptions {
+  pub file_path: PathBuf,
+  pub frozen: bool,
+  /// Causes the lockfile to only be read from, but not written to.
+  pub skip_write: bool,
+}
+
 #[derive(Debug)]
 pub struct CliLockfile {
   lockfile: Mutex<Lockfile>,
   pub filename: PathBuf,
-  pub frozen: bool,
+  frozen: bool,
+  skip_write: bool,
 }
 
 pub struct Guard<'a, T> {
@@ -50,15 +59,6 @@ impl<'a, T> std::ops::DerefMut for Guard<'a, T> {
 }
 
 impl CliLockfile {
-  pub fn new(lockfile: Lockfile, frozen: bool) -> Self {
-    let filename = lockfile.filename.clone();
-    Self {
-      lockfile: Mutex::new(lockfile),
-      filename,
-      frozen,
-    }
-  }
-
   /// Get the inner deno_lockfile::Lockfile.
   pub fn lock(&self) -> Guard<Lockfile> {
     Guard {
@@ -78,6 +78,10 @@ impl CliLockfile {
   }
 
   pub fn write_if_changed(&self) -> Result<(), AnyError> {
+    if self.skip_write {
+      return Ok(());
+    }
+
     self.error_if_changed()?;
     let mut lockfile = self.lockfile.lock();
     let Some(bytes) = lockfile.resolve_write_bytes() else {
@@ -142,7 +146,7 @@ impl CliLockfile {
       return Ok(None);
     }
 
-    let filename = match flags.lock {
+    let file_path = match flags.lock {
       Some(ref lock) => PathBuf::from(lock),
       None => match workspace.resolve_lockfile_path()? {
         Some(path) => path,
@@ -160,7 +164,11 @@ impl CliLockfile {
         .unwrap_or(false)
     });
 
-    let lockfile = Self::read_from_path(filename, frozen)?;
+    let lockfile = Self::read_from_path(CliLockfileReadFromPathOptions {
+      file_path,
+      frozen,
+      skip_write: flags.internal.lockfile_skip_write,
+    })?;
 
     // initialize the lockfile with the workspace's configuration
     let root_url = workspace.root_dir();
@@ -212,25 +220,29 @@ impl CliLockfile {
   }
 
   pub fn read_from_path(
-    file_path: PathBuf,
-    frozen: bool,
+    opts: CliLockfileReadFromPathOptions,
   ) -> Result<CliLockfile, AnyError> {
-    match std::fs::read_to_string(&file_path) {
-      Ok(text) => Ok(CliLockfile::new(
-        Lockfile::new(deno_lockfile::NewLockfileOptions {
-          file_path,
-          content: &text,
-          overwrite: false,
-        })?,
-        frozen,
-      )),
-      Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(
-        CliLockfile::new(Lockfile::new_empty(file_path, false), frozen),
-      ),
-      Err(err) => Err(err).with_context(|| {
-        format!("Failed reading lockfile '{}'", file_path.display())
-      }),
-    }
+    let lockfile = match std::fs::read_to_string(&opts.file_path) {
+      Ok(text) => Lockfile::new(deno_lockfile::NewLockfileOptions {
+        file_path: opts.file_path,
+        content: &text,
+        overwrite: false,
+      })?,
+      Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
+        Lockfile::new_empty(opts.file_path, false)
+      }
+      Err(err) => {
+        return Err(err).with_context(|| {
+          format!("Failed reading lockfile '{}'", opts.file_path.display())
+        });
+      }
+    };
+    Ok(CliLockfile {
+      filename: lockfile.filename.clone(),
+      lockfile: Mutex::new(lockfile),
+      frozen: opts.frozen,
+      skip_write: opts.skip_write,
+    })
   }
 
   pub fn error_if_changed(&self) -> Result<(), AnyError> {

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -44,6 +44,7 @@ pub use deno_config::glob::FilePatterns;
 pub use deno_json::check_warn_tsconfig;
 pub use flags::*;
 pub use lockfile::CliLockfile;
+pub use lockfile::CliLockfileReadFromPathOptions;
 pub use package_json::NpmInstallDepsProvider;
 
 use deno_ast::ModuleSpecifier;
@@ -828,7 +829,7 @@ impl CliOptions {
 
     let maybe_lockfile = maybe_lockfile.filter(|_| !force_global_cache);
     let deno_dir_provider =
-      Arc::new(DenoDirProvider::new(flags.cache_path.clone()));
+      Arc::new(DenoDirProvider::new(flags.internal.cache_path.clone()));
     let maybe_node_modules_folder = resolve_node_modules_folder(
       &initial_cwd,
       &flags,

diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
@@ -53,6 +53,7 @@ use super::logging::lsp_log;
 use crate::args::discover_npmrc_from_workspace;
 use crate::args::has_flag_env_var;
 use crate::args::CliLockfile;
+use crate::args::CliLockfileReadFromPathOptions;
 use crate::args::ConfigFile;
 use crate::args::LintFlags;
 use crate::args::LintOptions;
@@ -1931,7 +1932,11 @@ fn resolve_lockfile_from_path(
   lockfile_path: PathBuf,
   frozen: bool,
 ) -> Option<CliLockfile> {
-  match CliLockfile::read_from_path(lockfile_path, frozen) {
+  match CliLockfile::read_from_path(CliLockfileReadFromPathOptions {
+    file_path: lockfile_path,
+    frozen,
+    skip_write: false,
+  }) {
     Ok(value) => {
       if value.filename.exists() {
         if let Ok(specifier) = ModuleSpecifier::from_file_path(&value.filename)

diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
@@ -96,6 +96,7 @@ use crate::args::CaData;
 use crate::args::CacheSetting;
 use crate::args::CliOptions;
 use crate::args::Flags;
+use crate::args::InternalFlags;
 use crate::args::UnstableFmtOptions;
 use crate::factory::CliFactory;
 use crate::file_fetcher::FileFetcher;
@@ -3605,7 +3606,10 @@ impl Inner {
     };
     let cli_options = CliOptions::new(
       Arc::new(Flags {
-        cache_path: Some(self.cache.deno_dir().root.clone()),
+        internal: InternalFlags {
+          cache_path: Some(self.cache.deno_dir().root.clone()),
+          ..Default::default()
+        },
         ca_stores: workspace_settings.certificate_stores.clone(),
         ca_data: workspace_settings.tls_certificate.clone().map(CaData::File),
         unsafely_ignore_certificate_errors: workspace_settings

diff --git a/cli/main.rs b/cli/main.rs
@@ -37,6 +37,7 @@ use crate::util::v8::get_v8_flags_from_env;
 use crate::util::v8::init_v8_flags;
 
 use args::TaskFlags;
+use deno_resolver::npm::ByonmResolvePkgFolderFromDenoReqError;
 use deno_runtime::WorkerExecutionMode;
 pub use deno_runtime::UNSTABLE_GRANULAR_FLAGS;
 
@@ -51,6 +52,7 @@ use deno_runtime::fmt_errors::FixSuggestion;
 use deno_runtime::tokio_util::create_and_run_current_thread_with_maybe_metrics;
 use deno_terminal::colors;
 use factory::CliFactory;
+use npm::ResolvePkgFolderFromDenoReqError;
 use standalone::MODULE_NOT_FOUND;
 use standalone::UNSUPPORTED_SCHEME;
 use std::env;
@@ -182,6 +184,21 @@ async fn run_subcommand(flags: Arc<Flags>) -> Result<i32, AnyError> {
         match result {
           Ok(v) => Ok(v),
           Err(script_err) => {
+            if let Some(ResolvePkgFolderFromDenoReqError::Byonm(ByonmResolvePkgFolderFromDenoReqError::UnmatchedReq(_))) = script_err.downcast_ref::<ResolvePkgFolderFromDenoReqError>() {
+              if flags.node_modules_dir.is_none() {
+                let mut flags = flags.deref().clone();
+                let watch = match &flags.subcommand {
+                  DenoSubcommand::Run(run_flags) => run_flags.watch.clone(),
+                  _ => unreachable!(),
+                };
+                flags.node_modules_dir = Some(deno_config::deno_json::NodeModulesDirMode::None);
+                // use the current lockfile, but don't write it out
+                if flags.frozen_lockfile.is_none() {
+                  flags.internal.lockfile_skip_write = true;
+                }
+                return tools::run::run_script(WorkerExecutionMode::Run, Arc::new(flags), watch).await;
+              }
+            }
             let script_err_msg = script_err.to_string();
             if script_err_msg.starts_with(MODULE_NOT_FOUND) || script_err_msg.starts_with(UNSUPPORTED_SCHEME) {
               if run_flags.bare {

diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
@@ -21,6 +21,7 @@ use crate::resolver::CliDenoResolverFs;
 
 use super::CliNpmResolver;
 use super::InnerCliNpmResolverRef;
+use super::ResolvePkgFolderFromDenoReqError;
 
 pub type CliByonmNpmResolverCreateOptions =
   ByonmNpmResolverCreateOptions<CliDenoResolverFs>;
@@ -90,10 +91,11 @@ impl CliNpmResolver for CliByonmNpmResolver {
     &self,
     req: &PackageReq,
     referrer: &Url,
-  ) -> Result<PathBuf, AnyError> {
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
     ByonmNpmResolver::resolve_pkg_folder_from_deno_module_req(
       self, req, referrer,
     )
+    .map_err(ResolvePkgFolderFromDenoReqError::Byonm)
   }
 
   fn check_state_hash(&self) -> Option<u64> {

diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
@@ -51,6 +51,7 @@ use self::resolvers::NpmPackageFsResolver;
 
 use super::CliNpmResolver;
 use super::InnerCliNpmResolverRef;
+use super::ResolvePkgFolderFromDenoReqError;
 
 mod cache;
 mod registry;
@@ -649,9 +650,13 @@ impl CliNpmResolver for ManagedCliNpmResolver {
     &self,
     req: &PackageReq,
     _referrer: &ModuleSpecifier,
-  ) -> Result<PathBuf, AnyError> {
-    let pkg_id = self.resolve_pkg_id_from_pkg_req(req)?;
-    self.resolve_pkg_folder_from_pkg_id(&pkg_id)
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
+    let pkg_id = self
+      .resolve_pkg_id_from_pkg_req(req)
+      .map_err(|err| ResolvePkgFolderFromDenoReqError::Managed(err.into()))?;
+    self
+      .resolve_pkg_folder_from_pkg_id(&pkg_id)
+      .map_err(ResolvePkgFolderFromDenoReqError::Managed)
   }
 
   fn check_state_hash(&self) -> Option<u64> {

diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs
@@ -14,11 +14,13 @@ use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_npm::registry::NpmPackageInfo;
 use deno_resolver::npm::ByonmNpmResolver;
+use deno_resolver::npm::ByonmResolvePkgFolderFromDenoReqError;
 use deno_runtime::deno_node::NodeRequireResolver;
 use deno_runtime::ops::process::NpmProcessStateProvider;
 use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
 use node_resolver::NpmResolver;
+use thiserror::Error;
 
 use crate::args::npm_registry_url;
 use crate::file_fetcher::FileFetcher;
@@ -29,6 +31,14 @@ pub use self::managed::CliNpmResolverManagedCreateOptions;
 pub use self::managed::CliNpmResolverManagedSnapshotOption;
 pub use self::managed::ManagedCliNpmResolver;
 
+#[derive(Debug, Error)]
+pub enum ResolvePkgFolderFromDenoReqError {
+  #[error(transparent)]
+  Managed(deno_core::error::AnyError),
+  #[error(transparent)]
+  Byonm(#[from] ByonmResolvePkgFolderFromDenoReqError),
+}
+
 pub enum CliNpmResolverCreateOptions {
   Managed(CliNpmResolverManagedCreateOptions),
   Byonm(CliByonmNpmResolverCreateOptions),
@@ -93,7 +103,7 @@ pub trait CliNpmResolver: NpmResolver {
     &self,
     req: &PackageReq,
     referrer: &ModuleSpecifier,
-  ) -> Result<PathBuf, AnyError>;
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError>;
 
   /// Returns a hash returning the state of the npm resolver
   /// or `None` if the state currently can't be determined.

diff --git a/resolvers/deno/Cargo.toml b/resolvers/deno/Cargo.toml
@@ -22,6 +22,7 @@ deno_package_json.features = ["sync"]
 deno_path_util.workspace = true
 deno_semver.workspace = true
 node_resolver.workspace = true
+thiserror.workspace = true
 url.workspace = true
 
 [dev-dependencies]