Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,19 +220,19 @@ impl LspScopedResolver {
}
CliNpmResolver::Managed(managed_npm_resolver) => {
CliNpmResolverCreateOptions::Managed({
let sys = CliSys::default();
let sys = &factory.sys;
let npmrc = self
.config_data
.as_ref()
.and_then(|d| d.npmrc.clone())
.unwrap_or_else(|| Arc::new(create_default_npmrc(&sys)));
.unwrap_or_else(|| Arc::new(create_default_npmrc(sys)));
let npm_cache_dir = Arc::new(NpmCacheDir::new(
&sys,
sys,
managed_npm_resolver.global_cache_root_path().to_path_buf(),
npmrc.get_all_known_registries_urls(),
));
ManagedNpmResolverCreateOptions {
sys,
sys: factory.node_resolution_sys.clone(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As part of this change, I was worried it would slow things down, so I updated the global resolver to use the NodeResolutionSys, which has a built-in cache. It should be faster than before now, but I didn't measure it yet.

npm_cache_dir,
maybe_node_modules_path: managed_npm_resolver
.root_node_modules_path()
Expand Down Expand Up @@ -992,7 +992,7 @@ impl<'a> ResolverFactory<'a> {
}

CliNpmResolverCreateOptions::Managed(ManagedNpmResolverCreateOptions {
sys: CliSys::default(),
sys: self.node_resolution_sys.clone(),
npm_cache_dir,
maybe_node_modules_path,
npmrc,
Expand Down
4 changes: 2 additions & 2 deletions cli/rt/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ pub async fn run(
NpmResolverCreateOptions::Managed(ManagedNpmResolverCreateOptions {
npm_resolution,
npm_cache_dir,
sys: sys.clone(),
sys: node_resolution_sys.clone(),
maybe_node_modules_path,
npm_system_info: Default::default(),
npmrc,
Expand Down Expand Up @@ -854,7 +854,7 @@ pub async fn run(
let npm_resolver = NpmResolver::<DenoRtSys>::new::<DenoRtSys>(
NpmResolverCreateOptions::Managed(ManagedNpmResolverCreateOptions {
npm_resolution,
sys: sys.clone(),
sys: node_resolution_sys.clone(),
npm_cache_dir,
maybe_node_modules_path: None,
npm_system_info: Default::default(),
Expand Down
4 changes: 0 additions & 4 deletions libs/cache_dir/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,6 @@ impl NpmCacheDir {
copy_index,
})
}

pub fn get_cache_location(&self) -> PathBuf {
self.root_dir.clone()
}
}

pub fn mixed_case_package_name_encode(name: &str) -> String {
Expand Down
24 changes: 16 additions & 8 deletions libs/node_resolver/cache.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright 2018-2026 the Deno authors. MIT license.

use std::borrow::Cow;
use std::cell::RefCell;
use std::collections::HashMap;
use std::path::Path;
Expand Down Expand Up @@ -111,28 +112,35 @@ impl<TSys: FsMetadata> NodeResolutionSys<TSys> {
Self { sys, cache: store }
}

pub fn is_file(&self, path: &Path) -> bool {
pub fn has_cache(&self) -> bool {
self.cache.is_some()
}

pub fn is_file(&self, path: Cow<'_, Path>) -> bool {
match self.get_file_type(path) {
Ok(file_type) => file_type.is_file(),
Err(_) => false,
}
}

pub fn is_dir(&self, path: &Path) -> bool {
pub fn is_dir(&self, path: Cow<'_, Path>) -> bool {
match self.get_file_type(path) {
Ok(file_type) => file_type.is_dir(),
Err(_) => false,
}
}

pub fn exists_(&self, path: &Path) -> bool {
pub fn exists_(&self, path: Cow<'_, Path>) -> bool {
self.get_file_type(path).is_ok()
}

pub fn get_file_type(&self, path: &Path) -> std::io::Result<FileType> {
pub fn get_file_type(
&self,
path: Cow<'_, Path>,
) -> std::io::Result<FileType> {
{
if let Some(maybe_value) =
self.cache.as_ref().and_then(|c| c.get_file_type(path))
self.cache.as_ref().and_then(|c| c.get_file_type(&path))
{
return match maybe_value {
Some(value) => Ok(value),
Expand All @@ -143,16 +151,16 @@ impl<TSys: FsMetadata> NodeResolutionSys<TSys> {
};
}
}
match self.sys.fs_metadata(path) {
match self.sys.fs_metadata(&path) {
Ok(metadata) => {
if let Some(cache) = &self.cache {
cache.set_file_type(path.to_path_buf(), Some(metadata.file_type()));
cache.set_file_type(path.into_owned(), Some(metadata.file_type()));
}
Ok(metadata.file_type())
}
Err(err) => {
if let Some(cache) = &self.cache {
cache.set_file_type(path.to_path_buf(), None);
cache.set_file_type(path.into_owned(), None);
}
Err(err)
}
Expand Down
36 changes: 19 additions & 17 deletions libs/node_resolver/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ impl<
None => Cow::Owned(path),
};

let maybe_file_type = self.sys.get_file_type(&path);
let maybe_file_type = self.sys.get_file_type(Cow::Borrowed(&path));
match maybe_file_type {
Ok(FileType::Dir) => {
if resolution_mode == ResolutionMode::Import
Expand All @@ -590,7 +590,7 @@ impl<
} else {
// prefer the file over the directory
let path_with_ext = with_known_extension(&path, "js");
if self.sys.is_file(&path_with_ext) {
if self.sys.is_file(Cow::Borrowed(&path_with_ext)) {
Ok(UrlOrPath::Path(path_with_ext))
} else {
let (resolved_url, resolved_method) = self
Expand Down Expand Up @@ -628,7 +628,7 @@ impl<
&& e.kind() == std::io::ErrorKind::NotFound
{
let file_with_ext = with_known_extension(&path, "js");
if self.sys.is_file(&file_with_ext) {
if self.sys.is_file(Cow::Borrowed(&file_with_ext)) {
return Ok(UrlOrPath::Path(file_with_ext));
}
}
Expand Down Expand Up @@ -666,9 +666,11 @@ impl<
}

if should_probe(path, resolved_method) {
["js", "mjs", "cjs"]
.into_iter()
.find(|ext| self.sys.is_file(&with_known_extension(path, ext)))
["js", "mjs", "cjs"].into_iter().find(|ext| {
self
.sys
.is_file(Cow::Owned(with_known_extension(path, ext)))
})
} else {
None
}
Expand Down Expand Up @@ -702,7 +704,7 @@ impl<
.flatten(),
)
.map(|p| deno_path_util::normalize_path(Cow::Owned(p)))
.find(|p| self.sys.is_file(p))
.find(|p| self.sys.is_file(Cow::Borrowed(p.as_ref())))
.and_then(|suggested_file_path| {
let pkg_json = self
.pkg_json_resolver
Expand Down Expand Up @@ -920,20 +922,20 @@ impl<
let mut searched_for_d_cts = false;
if media_type == MediaType::Mjs {
let d_mts_path = with_known_extension(path, "d.mts");
if sys.exists_(&d_mts_path) {
if sys.exists_(Cow::Borrowed(&d_mts_path)) {
return Some(d_mts_path);
}
searched_for_d_mts = true;
} else if media_type == MediaType::Cjs {
let d_cts_path = with_known_extension(path, "d.cts");
if sys.exists_(&d_cts_path) {
if sys.exists_(Cow::Borrowed(&d_cts_path)) {
return Some(d_cts_path);
}
searched_for_d_cts = true;
}

let dts_path = with_known_extension(path, "d.ts");
if sys.exists_(&dts_path) {
if sys.exists_(Cow::Borrowed(&dts_path)) {
return Some(dts_path);
}

Expand All @@ -947,12 +949,12 @@ impl<
_ => None, // already searched above
};
if let Some(specific_dts_path) = specific_dts_path
&& sys.exists_(&specific_dts_path)
&& sys.exists_(Cow::Borrowed(&specific_dts_path))
{
return Some(specific_dts_path);
}
let ts_path = with_known_extension(path, "ts");
if sys.is_file(&ts_path) {
if sys.is_file(Cow::Borrowed(&ts_path)) {
return Some(ts_path);
}
None
Expand All @@ -970,7 +972,7 @@ impl<
known_exists: true,
})));
}
if self.sys.is_dir(&local_path.path) {
if self.sys.is_dir(Cow::Borrowed(&local_path.path)) {
let resolution_result = self.resolve_package_dir_subpath(
&local_path.path,
/* sub path */ ".",
Expand Down Expand Up @@ -1968,7 +1970,7 @@ impl<

if let Some(main) = maybe_main.as_deref() {
let guess = package_json.path.parent().unwrap().join(main).clean();
if self.sys.is_file(&guess) {
if self.sys.is_file(Cow::Borrowed(&guess)) {
return Ok(self.maybe_resolve_types(
LocalUrlOrPath::Path(LocalPath {
path: guess,
Expand Down Expand Up @@ -2006,7 +2008,7 @@ impl<
.unwrap()
.join(format!("{main}{ending}"))
.clean();
if self.sys.is_file(&guess) {
if self.sys.is_file(Cow::Borrowed(&guess)) {
// TODO(bartlomieju): emitLegacyIndexDeprecation()
return Ok(MaybeTypesResolvedUrl(LocalUrlOrPath::Path(LocalPath {
path: guess,
Expand Down Expand Up @@ -2045,7 +2047,7 @@ impl<
};
for index_file_name in index_file_names {
let guess = directory.join(index_file_name).clean();
if self.sys.is_file(&guess) {
if self.sys.is_file(Cow::Borrowed(&guess)) {
// TODO(bartlomieju): emitLegacyIndexDeprecation()
return Ok(MaybeTypesResolvedUrl(LocalUrlOrPath::Path(LocalPath {
path: guess,
Expand Down Expand Up @@ -2633,7 +2635,7 @@ impl<'a, TSys: FsMetadata> TypesVersions<'a, TSys> {
Cow::Borrowed(value)
};
let path = self.dir_path.join(value.as_ref());
if self.sys.is_file(&path) {
if self.sys.is_file(Cow::Owned(path)) {
return Some(value);
}
}
Expand Down
24 changes: 24 additions & 0 deletions libs/npm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,22 @@ impl NpmSystemInfo {
}
}

/// Extracts the package name from a specifier that may contain a subpath.
/// For example, "@denotest/add2/sub.js" -> "@denotest/add2", "foo/bar" -> "foo".
pub fn package_name_without_subpath(name: &str) -> &str {
let mut search_start_index = 0;
if name.starts_with('@')
&& let Some(slash_index) = name.find('/')
{
search_start_index = slash_index + 1;
}
if let Some(slash_index) = name[search_start_index..].find('/') {
&name[..search_start_index + slash_index]
} else {
name
}
}

fn matches_os_or_cpu_vec(items: &[SmallStackString], target: &str) -> bool {
if items.is_empty() {
return true;
Expand Down Expand Up @@ -573,4 +589,12 @@ mod test {
"x64"
));
}

#[test]
fn test_package_name_without_subpath() {
assert_eq!(package_name_without_subpath("foo"), "foo");
assert_eq!(package_name_without_subpath("@foo/bar"), "@foo/bar");
assert_eq!(package_name_without_subpath("@foo/bar/baz"), "@foo/bar");
assert_eq!(package_name_without_subpath("@hello"), "@hello");
}
}
25 changes: 1 addition & 24 deletions libs/npm/resolution/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ impl NpmResolutionSnapshot {
Box::new(PackageNotFoundFromReferrerError::Referrer(referrer.clone()))
})?;

let name = name_without_path(name);
let name = crate::package_name_without_subpath(name);
if let Some(id) = referrer_package.dependencies.get(name) {
return Ok(self.packages.get(id).unwrap());
}
Expand Down Expand Up @@ -863,21 +863,6 @@ impl SnapshotPackageCopyIndexResolver {
}
}

fn name_without_path(name: &str) -> &str {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed and consolidated.

let mut search_start_index = 0;
if name.starts_with('@')
&& let Some(slash_index) = name.find('/')
{
search_start_index = slash_index + 1;
}
if let Some(slash_index) = &name[search_start_index..].find('/') {
// get the name up until the path slash
&name[0..search_start_index + slash_index]
} else {
name
}
}

#[derive(Debug, Error, Clone, JsError)]
pub enum SnapshotFromLockfileError {
#[error("Could not find '{}' specified in the lockfile.", .source.0)]
Expand Down Expand Up @@ -1070,14 +1055,6 @@ mod tests {
use super::*;
use crate::registry::TestNpmRegistryApi;

#[test]
fn test_name_without_path() {
assert_eq!(name_without_path("foo"), "foo");
assert_eq!(name_without_path("@foo/bar"), "@foo/bar");
assert_eq!(name_without_path("@foo/bar/baz"), "@foo/bar");
assert_eq!(name_without_path("@hello"), "@hello");
}

#[test]
fn test_copy_index_resolver() {
let mut copy_index_resolver =
Expand Down
2 changes: 1 addition & 1 deletion libs/resolver/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ impl<TSys: WorkspaceFactorySys> ResolverFactory<TSys> {
})
} else {
NpmResolverCreateOptions::Managed(ManagedNpmResolverCreateOptions {
sys: self.workspace_factory.sys.clone(),
sys: self.sys.clone(),
npm_resolution: self.npm_resolution().clone(),
npm_cache_dir: self.workspace_factory.npm_cache_dir()?.clone(),
maybe_node_modules_path: self
Expand Down
Loading
Loading