From dcb00bb9b8bf39f0caa6bb87b6b11e1344803bb9 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 26 Sep 2023 16:42:39 -0400 Subject: [PATCH] chore: slight cleanup in npm resolvers (#20692) --- cli/lsp/analysis.rs | 1 + cli/lsp/diagnostics.rs | 1 + cli/lsp/tsc.rs | 1 + cli/npm/resolvers/mod.rs | 28 +++++----------------------- ext/node/lib.rs | 2 +- ext/node/resolution.rs | 9 +++++---- 6 files changed, 14 insertions(+), 28 deletions(-) diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 0d987163b2..7a0c11212a 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -21,6 +21,7 @@ use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::ModuleSpecifier; use deno_lint::rules::LintRule; +use deno_runtime::deno_node::NpmResolver; use deno_runtime::deno_node::PackageJson; use deno_runtime::deno_node::PathClean; use deno_semver::package::PackageReq; diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 2ab9656a42..43ae35f54b 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -38,6 +38,7 @@ use deno_graph::ResolutionError; use deno_graph::SpecifierError; use deno_lint::rules::LintRule; use deno_runtime::deno_node; +use deno_runtime::deno_node::NpmResolver; use deno_runtime::tokio_util::create_basic_runtime; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 77bfd20780..f2b9f56341 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -49,6 +49,7 @@ use deno_core::JsRuntime; use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::RuntimeOptions; +use deno_runtime::deno_node::NpmResolver; use deno_runtime::tokio_util::create_basic_runtime; use lazy_regex::lazy_regex; use log::error; diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 9f714256d0..19dd726f57 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -10,7 +10,6 @@ use std::path::PathBuf; use std::sync::Arc; use deno_ast::ModuleSpecifier; -use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; use deno_core::serde_json; @@ -24,7 +23,6 @@ use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::NpmResolver; -use deno_runtime::deno_node::PathClean; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use global::GlobalNpmPackageResolver; @@ -174,13 +172,6 @@ impl CliNpmResolver { Ok(crate::util::fs::dir_size(&package_folder)?) } - /// Gets if the provided specifier is in an npm package. - pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { - let root_dir_url = self.fs_resolver.root_dir_url(); - debug_assert!(root_dir_url.as_str().ends_with('/')); - specifier.as_ref().starts_with(root_dir_url.as_str()) - } - /// Adds package requirements to the resolver and ensures everything is setup. pub async fn add_package_reqs( &self, @@ -273,10 +264,9 @@ impl NpmResolver for CliNpmResolver { fn resolve_package_folder_from_path( &self, - path: &Path, + specifier: &ModuleSpecifier, ) -> Result, AnyError> { - let specifier = path_to_specifier(path)?; - self.resolve_package_folder_from_specifier(&specifier) + self.resolve_package_folder_from_specifier(specifier) } fn resolve_package_folder_from_deno_module( @@ -295,10 +285,9 @@ impl NpmResolver for CliNpmResolver { } fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { - self - .resolve_package_folder_from_specifier(specifier) - .map(|p| p.is_some()) - .unwrap_or(false) + let root_dir_url = self.fs_resolver.root_dir_url(); + debug_assert!(root_dir_url.as_str().ends_with('/')); + specifier.as_ref().starts_with(root_dir_url.as_str()) } fn ensure_read_permission( @@ -338,10 +327,3 @@ pub fn create_npm_fs_resolver( )), } } - -fn path_to_specifier(path: &Path) -> Result { - match ModuleSpecifier::from_file_path(path.to_path_buf().clean()) { - Ok(specifier) => Ok(specifier), - Err(()) => bail!("Could not convert '{}' to url.", path.display()), - } -} diff --git a/ext/node/lib.rs b/ext/node/lib.rs index c7ca2ca72c..d3e0d07dbe 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -87,7 +87,7 @@ pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync { /// Resolves the npm package folder path from the specified path. fn resolve_package_folder_from_path( &self, - path: &Path, + specifier: &ModuleSpecifier, ) -> Result, AnyError>; /// Resolves an npm package folder path from a Deno module. diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs index 7a88356d88..dc247ddceb 100644 --- a/ext/node/resolution.rs +++ b/ext/node/resolution.rs @@ -1133,7 +1133,7 @@ impl NodeResolver { ) -> Result, AnyError> { let Some(root_folder) = self .npm_resolver - .resolve_package_folder_from_path(&referrer.to_file_path().unwrap())? + .resolve_package_folder_from_path(referrer)? else { return Ok(None); }; @@ -1170,9 +1170,10 @@ impl NodeResolver { if self.fs.exists_sync(&package_json_path) { return Ok(Some(package_json_path)); } - let Some(root_pkg_folder) = self - .npm_resolver - .resolve_package_folder_from_path(current_dir)? + let Some(root_pkg_folder) = + self.npm_resolver.resolve_package_folder_from_path( + &ModuleSpecifier::from_directory_path(current_dir).unwrap(), + )? else { return Ok(None); };